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

Logger doesn't seem to work with errors #46

Closed
jensengar opened this issue Mar 21, 2018 · 11 comments
Closed

Logger doesn't seem to work with errors #46

jensengar opened this issue Mar 21, 2018 · 11 comments
Assignees

Comments

@jensengar
Copy link
Contributor

I'm not sure if this is a new thing because I thought it worked before, but it seems whenever I try and log/warn/error an error object it just logs an empty object. My config is the same as you have in the readme, other than a different api url. Here's what happens when I try and log various things in the js console.

this.logger.log({msg: 'here we go', custom: 'yup'})

gets logged as

2018-03-21T00:37:34.131Z [LOG] {
  "msg": "here we go",
  "custom": "yup"
}

However if I do

error = new Error('Here is my error message');
this.logger.log(error);

I get

2018-03-21T00:40:54.334Z [LOG] {}

Which I imagine is because that is the result of JSON.stringify(error). Is this a known issue? Are there plans to fix it?

@th3n3wguy
Copy link
Contributor

Yeah. This is definitely something that can be handled better. Here is the line of code that is causing your problem: https://github.com/dbfannin/ngx-logger/blob/master/src/logger.service.ts#L150

In my current projects that use this library, I am always passing a string as the first parameter and the Error or any other objects / arrays / etc. as the rest of the parameters. This also helps me determine where in my code the information is being logged, since the logging of file / line numbers issue hasn't been solved yet.

@th3n3wguy
Copy link
Contributor

th3n3wguy commented Mar 21, 2018

@jensengar => What about the error are you expecting to be logged whenever you log an Error object as the first "message" parameter?

If you are expecting the .stack to be logged, then we could always check for the .stack property if it is an object on that same line of code.

@jensengar
Copy link
Contributor Author

Yeah, I think it would be ideal to log the stack because when that error goes up to the server, it would be great to be able to see what and where, rather than just what. I think it would probably be sufficient, at least for me if you did something like:

if (message instanceof Error) {
 message = message.stack;
} else if (typeof message !== 'string') {
 message = JSON.stringify(message, null, 2);
}

Any concerns with that?

@jensengar
Copy link
Contributor Author

I'm happy to PR that if you wish.

@th3n3wguy
Copy link
Contributor

@jensengar => The instanceof check might not work because of all of the different TYPES of errors that Angular has within its platform, as well as custom errors that you might have created. Here are the class instances we will have to check on (that I am aware of currently). I will update this list as we list more that have to be handled.

The only other way to get around this is to check for the .stack property on the object and assume that it is an instance / extension of the Error object.

@jensengar
Copy link
Contributor Author

jensengar commented Mar 21, 2018

It's true .stack would definitely be a catch-all however if you have an object that is not an error but has a stack property, it will not log correctly. As far as custom errors go, if they inherit from the Error base class they will still return true with the instance of. See example:

class ExError extends Error { }
error = new ExError('here is my ex error'));
error instanceOf Error // returns true

For what I need right now though, I can't think of a case where .stack would cause any issues if you'd still rather go that route. BTW, thank you for putting this library together, it does everything I could have hoped for.

Edit: I forgot to address the HttpErrorResponse argument. Honestly, I wouldn't treat this as an error object because it's not. I'm pretty sure as it is now, HttpErrorResponse logs correctly.

@th3n3wguy
Copy link
Contributor

@jensengar => I'm glad that you like the library, but I am not the one who created it. I am just passionate about maintaining it (for now, that is). @dbfannin is the creator / primary maintainer.

If you want to submit a pull request, I will review it for you. Otherwise, I will get a pull request opened when I get some free time.

@dbfannin
Copy link
Owner

This issue is now fixed with @jensengar's PR. Version [email protected]. Thanks @th3n3wguy for helping guide this issue.

@jensengar
Copy link
Contributor Author

Has this update been published to npm yet? And if not, when is the planned time for that?

@dbfannin
Copy link
Owner

@jensengar, see comment above. version 2.0.4

@dbfannin
Copy link
Owner

I had an issue with the first time I published to 2.0.3, so I had to unpublish and bump the revision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants