-
Notifications
You must be signed in to change notification settings - Fork 26.2k
fix(aio): constrain error logging to improve reporting #22713
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(aio): constrain error logging to improve reporting #22713
Conversation
You can preview 4ddfea2 at https://pr22713-4ddfea2.ngbuilds.io/. |
logger.error('param1', 'param2', 'param3'); | ||
expect(errorHandler.handleError).toHaveBeenCalledWith('param1 param2 param3'); | ||
let err: Error; | ||
try { throw new Error('some error message'); } catch (e) { err = e; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??? 😕
Why not logger.error(new Error('some error message'))
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! I think I was thinking about checking the stack or something. I'll remove as it doesn't make this test any better.
@@ -90,7 +90,7 @@ export class DocumentService { | |||
} | |||
|
|||
private getErrorDoc(id: string, error: HttpErrorResponse): Observable<DocumentContents> { | |||
this.logger.error('Error fetching document', error); | |||
this.logger.error(new Error(`Error fetching document '${id}': (${error.message})`)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC, why the parenthesis around error.message
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I parenthesized the original error because it is going to have some implementation specific error information, which not the main message of the error.
4ddfea2
to
f3c37b0
Compare
You can preview f3c37b0 at https://pr22713-f3c37b0.ngbuilds.io/. |
The `Logger.error()` method now only accepts a single `Error` parameter and passes this through to the error handler. This allows the error handler to serialize the error more accurately. The various places that use `Logger.error()` have been updated. See angular#21943#issuecomment-370230047
f3c37b0
to
abaa31b
Compare
You can preview abaa31b at https://pr22713-abaa31b.ngbuilds.io/. |
The `Logger.error()` method now only accepts a single `Error` parameter and passes this through to the error handler. This allows the error handler to serialize the error more accurately. The various places that use `Logger.error()` have been updated. See #21943#issuecomment-370230047 PR Close #22713
The `Logger.error()` method now only accepts a single `Error` parameter and passes this through to the error handler. This allows the error handler to serialize the error more accurately. The various places that use `Logger.error()` have been updated. See angular#21943#issuecomment-370230047 PR Close angular#22713
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The
Logger.error()
method now only accepts a singleError
parameterand passes this through to the error handler.
This allows the error handler to serialize the error more accurately.
The various places that use
Logger.error()
have been updated.See #21943#issuecomment-370230047