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

feat: automatic error formatting in json layout #224

Open
EinfachHans opened this issue Nov 14, 2023 · 4 comments
Open

feat: automatic error formatting in json layout #224

EinfachHans opened this issue Nov 14, 2023 · 4 comments
Assignees

Comments

@EinfachHans
Copy link

Informations

Type  Version
Improvement 6.6.3

Description

I'm currently in process of understanding the logger and how it works. I have problems with a usefull error formatting:

For production i enabled the json layout. See the following comparison, between what i log and what the output is:

Log Response
this.logger.error('some error', exception) {"startTime":"2023-11-14T14:30:30.024Z","categoryName":"ExceptionsFilter","level":"ERROR","data":["some error"]}
this.logger.error(exception) {"startTime":"2023-11-14T14:31:21.042Z","categoryName":"ExceptionsFilter","level":"ERROR","data":[]}
this.logger.error({ message: 'some error', exception }) {"startTime":"2023-11-14T14:32:23.111Z","categoryName":"ExceptionsFilter","level":"ERROR","message":"some error","exception":{},"data":[]}

In all of this variants the information about the exception (new Error('test') in my case) is missing.

What would i expect? Good question. When a error ocours the default context logger logs something like this:

{"startTime":"2023-11-14T14:32:23.113Z","categoryName":"Default","level":"ERROR","method":"GET","url":"/healthz","route":"/healthz","headers":{...},"body":{},"query":{},"params":{},"reqId":"f3dc11e567e24389b9847d7321643533","time":"2023-11-14T14:32:23.113Z","duration":9,"event":"request.end","status":500,"status_code":"500","state":"KO","error_name":"Error","error_message":"test","error_stack":"...","data":[]}

This information is explicit added by the PlatformLogMiddleware of @tsed. Maybe it would make sense something similar to the logger itself? Maybe here? Where when an error is detected within the data it could be handled separately.

Or is this not a good solution for some reasons? Then i would have to implement something in my application.

@EinfachHans
Copy link
Author

Hey @Romakita , did you see this issue? 😃 Just noticed that you are not automatically assigned here

@Romakita
Copy link
Collaborator

Hello @EinfachHans I haven't found this issue.

The problem is around the error object itself (detecting will be a huge cost, because we need to introspect object and nested object), JSON.stringify isn't able to serialize the error instance because Error.prototype haven't a toJSON method. We can see that here:

Capture d’écran 2023-11-18 à 09 41 39

Reading this article explain how it's possible to serialize error: https://zirkelc.dev/posts/stringify-and-parse-errors-in-javascript.

The problem is how to support that correctly for all possible Error type (and custom Error) without exposing critical information of our application (Error.stack ??). This is actually why I haven't fixed this problem on logger level.

Note: @tsed/logger code base was originally based on log4js, and the team haven't fixed that ^^

@Romakita Romakita self-assigned this Nov 18, 2023
@Romakita Romakita added the bug label Nov 18, 2023
@Romakita Romakita added enhancement and removed bug labels Nov 18, 2023
@EinfachHans
Copy link
Author

Hey @Romakita,

thanks for the explanation! Yeah already thought it is going to be a larger thing 😩

For know i will write a custom logic in my app the works for me 👍🏼

@Romakita
Copy link
Collaborator

You can monkey patch the Exception.prototype.toJSON until we haven’t a good solution ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do
Development

No branches or pull requests

2 participants