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

Always include stacktrace for console messages #854

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Conversation

OrKoN
Copy link
Contributor

@OrKoN OrKoN commented Jan 13, 2025

As an alternative to #807 it makes sense to always include the stacktrace so that the client can figure out where the logs are coming from.


Preview | Diff

1. If |method| is "<code>assert</code>", "<code>error</code>",
"<code>trace</code>", or "<code>warn</code>", let |stack| be the [=current
stack trace=]. Otherwise let |stack| be null.
1. Let |stack| be the [=current stack trace=].
Copy link
Contributor

Choose a reason for hiding this comment

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

With this change stack can't be null so we can simplify the next step:

the stackTrace field set to |stack| if |stack| is not null, or
omitted otherwise,

can become

the stackTrace field set to |stack|

Also I think stackTrace is now mandatory in log.BaseLogEntry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess GenericLogEntry can still be without stack? (it does not seem to be used but maybe reserved for other kinds of entries?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes indeed. A bit weird to have an unused / undefined log type here, but that's another topic :)

Thanks for the patch and sorry about the delay

Copy link
Contributor

@juliandescottes juliandescottes left a comment

Choose a reason for hiding this comment

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

Thanks! Some small changes needed now that stack will never be null

@OrKoN OrKoN force-pushed the orkon/stacktrace-always branch from fcf1d86 to e6001f1 Compare January 14, 2025 17:05
@OrKoN OrKoN requested a review from juliandescottes January 14, 2025 17:05
@OrKoN OrKoN merged commit c6bc3f2 into main Jan 15, 2025
5 checks passed
@OrKoN OrKoN deleted the orkon/stacktrace-always branch January 15, 2025 12:04
github-actions bot added a commit that referenced this pull request Jan 15, 2025
SHA: c6bc3f2
Reason: push, by OrKoN

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants