Skip to content

Reenable logging stacktraces of uncaught exceptions#108

Merged
JelteF merged 1 commit into
mainfrom
log-uncaught-exceptions
Nov 29, 2021
Merged

Reenable logging stacktraces of uncaught exceptions#108
JelteF merged 1 commit into
mainfrom
log-uncaught-exceptions

Conversation

@JelteF

@JelteF JelteF commented Nov 29, 2021

Copy link
Copy Markdown
Contributor

Logging of stacktraces was disabled by #97. Based on PR comments the
reason for this was that for most types of errors error.Exception
would be null. Thus the exception logging code would actually create a
new error by dereferencing a null.

This was addressed by only logging the error Code and Message.
However, logging these is not very useful. They are already passed to
the frontend. So this fixes the problem in a better way, by only logging
to stderr when the error its error.Exception field is non-null.

Logging of stacktraces was disabled by #97. Based on PR comments the
reason for this was that for most types of errors `error.Exception`
would be `null`. Thus the exception logging code would actually create a
new error by dereferencing a `null`.

This was addressed by only logging the error `Code` and `Message`.
However, logging these is not very useful. They are already passed to
the frontend. So this fixes the problem in a better way, by only logging
to stderr when the error its `error.Exception` field is non-null.

@jarupatj jarupatj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for fixing this.

@seantleonard seantleonard left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, properly see Error message and stacktrace in Console now.

@JelteF JelteF merged commit a476433 into main Nov 29, 2021
@JelteF JelteF deleted the log-uncaught-exceptions branch November 29, 2021 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants