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: added support for go-errors stack traces #490

Closed
wants to merge 1 commit into from

Conversation

alesbrelih
Copy link

@alesbrelih alesbrelih commented Oct 8, 2022

Added support to print out stack traces generated by the go-errors library.

I've put the new code in goerrors package, similar to the existing pkgerrors package, becase I wasn't sure what kind of folder structure would you prefer. If you want I can change this structure to something like:

stacktraces/{pkgerrors,goerrors}

Thank you for this library!

Added support to print out stack traces generated by the go-errors
library.
@alesbrelih alesbrelih force-pushed the feat/go-errors-stacktrace branch from 52d9519 to 6566900 Compare October 8, 2022 18:06
@mitar
Copy link
Contributor

mitar commented Aug 3, 2023

I think zerolog should go away from using external errors libraries to format stack traces but just support extracting stack traces from them. go-errors does implement the Callers() []uintptr interface and only that (without having to import go-errors) should be used to extract callers and then process those callers inside zerolog itself, in the same way for all errors.

The errors package I made as a v2 of pkgerrors implements StackTrace() []uintptr interface. So, zerolog would only have to check few of those interfaces and extract raw callers and then format them themselves (ideally into JSON first, and then from JSON into console).

pkgerrors sadly does not support that, so it has to be imported to obtain the stack trace.

BTW, I would suggest that the order of checks on the error is that first it s checked if error knows how to JSON marshall itself, or even if it supports zerolog's JSON conversion. And if not, then check if they support extracting []uintptr out.

@th0th
Copy link

th0th commented Oct 17, 2023

Looking forward for this to get merged 🙌

@rs rs closed this Mar 2, 2024
@th0th
Copy link

th0th commented Mar 2, 2024

Hey @rs, does your closing this PR mean the feature is rejected? 🙃

@rs
Copy link
Owner

rs commented Mar 2, 2024

Yes, I will not be adding any dependencies to this library.

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.

4 participants