Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: logger fails to print null objects #4679

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

rosen-vladimirov
Copy link
Contributor

In case you try printing the following:

const obj1 = null;
this.$logger.info("value %s", obj1);

Our implementation fails with error: Cannot read property hasOwnProperty of null.
The problem is in the way we check for passed logger options in the args passed to logger methods. The typeof null returns object, so next parts of the code fail.

Also passed logger options are handled by priority, i.e. the first passed option should be used. This allows callers of methods that specify some logger options internaly, to overwrite their values. Add unit tests for this behavior.
Fix the case when same logger option is passed multiple times in different objects - currently only one of the instances is removed and the other one is included in the message visible to the user. Now all such objects are removed.
Add unit tests for all of the described cases.

PR Checklist

What is the current behavior?

What is the new behavior?

Fixes/Implements/Closes #[Issue Number].

@rosen-vladimirov rosen-vladimirov added this to the 5.4.1 milestone Jun 5, 2019
@rosen-vladimirov rosen-vladimirov self-assigned this Jun 5, 2019
@cla-bot cla-bot bot added the cla: yes label Jun 5, 2019
In case you try printing the following:
```
const obj1 = null;
this.$logger.info("value %s", obj1);
```
Our implementation fails with error: `Cannot read property hasOwnProperty of null`.
The problem is in the way we check for passed logger options in the args passed to `logger` methods. The `typeof null` returns object, so next parts of the code fail.

Also passed logger options are handled by priority, i.e. the first passed option should be used. This allows callers of methods that specify some logger options internaly, to overwrite their values. Add unit tests for this behavior.
Fix the case when same logger option is passed multiple times in different objects - currently only one of the instances is removed and the other one is included in the message visible to the user. Now all such objects are removed.
Add unit tests for all of the described cases.
@rosen-vladimirov rosen-vladimirov force-pushed the vladimirov/fix-logger-again branch from 23f3af9 to 6256958 Compare June 5, 2019 14:32
@rosen-vladimirov
Copy link
Contributor Author

test cli-smoke

@rosen-vladimirov rosen-vladimirov merged commit 8e92c4e into release Jun 5, 2019
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-logger-again branch June 5, 2019 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants