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

Expose traceTree or some variant #71

Closed
bufdev opened this issue Dec 9, 2023 · 6 comments · Fixed by #102
Closed

Expose traceTree or some variant #71

bufdev opened this issue Dec 9, 2023 · 6 comments · Fixed by #102
Labels
enhancement New feature or request

Comments

@bufdev
Copy link

bufdev commented Dec 9, 2023

This was motivated by wanting to get a printout of just the stack trace, without the underlying error message. That use was was a situation where I had error interceptor logic in place, but didn't control the underlying location where the error was printed, as that was controlled by a different library (github.com/spf13/cobra). I just wanted to get a stack trace to print out to a debug logger, but realized I couldn't just yet.

This could take different forms:

  • Just expose traceTree directly. Likely not great, you probably don't want users mucking with this.
  • Have some public partial type that maps to traceTree under the hood.
  • Don't expose traceTree fully, but expose FormatTrace/FormatTraceString or the like that just drops the error itself (would solve my original issue, but isn't as universal).

Potentially the most idiomatic way to do this would be to expose some custom error type where I can use errors.Is/As, which would solve the problem more generally, but I assume this was ruled out.

I may be missing something here, just starting to muck around with the library - apologies if this is polluting the issue space!

@abhinav
Copy link
Contributor

abhinav commented Dec 9, 2023

Hey, @bufdev!

I think exporting traceTree is okay: it doesn't have any internal details—not even the program counters. It's just a plain data structure. (Thinking out loud, probably: func Inspect(error) *Tree.)

Although I agree—exporting this is not my favorite solution. However, it seems better than an overly complicated error tree walker, which is the other option we were considering here to optimize on allocs during formatting. We might still do that in the future, but it could be an internal detail for formatting and the public Inspect function can remain.

@prashantv Do you have thoughts on this?

@bufdev
Copy link
Author

bufdev commented Dec 9, 2023

Oh, that'd be great then if possible :-) But no worries either way. One suggestion: If you do expose Tree directly, some type of iterator-type function to iterate over what are now the traceFrames for the both Trace and Children would be fantastic, but I won't get picky.

@abhinav abhinav added the enhancement New feature or request label Dec 10, 2023
@abhinav
Copy link
Contributor

abhinav commented Dec 11, 2023

@bufdev Just confirming one thing for what you were trying to do: you didn't need to know/track the parent/child relationships between errors, right? You just wanted errors and their traces?

@bufdev
Copy link
Author

bufdev commented Dec 11, 2023

Yea I'm not interested in the relationships between the errors.

@prashantv
Copy link
Contributor

prashantv commented Dec 12, 2023

Since we allow annotating an error with a specific frame, I wonder if we should expose the converse -- an API that extracts a single frame from an error (if it's an errtrace wrapped error). The caller can then build up the same tree by unwrapping.

I'm OK with also having more APIs to expose the tree, though I'd start with the lowest-level first, then the more common need (customization options for formatting).

@abhinav
Copy link
Contributor

abhinav commented Dec 12, 2023

I like starting with that.
Strawman:

// UnwrapFrame unwraps the errtrace frame
// stored inside the given error.
//
// inner is the error inside the errtrace, if any.
// ok reports whether a frame was available.
func UnwrapFrame(error) (inner error, frame Frame, ok bool)

(I want to not take the name "unwrap" in case we want to provide an errors.Unwrap analog per #38 (comment).)

So a user could do something like:

// func printTrace(w io.Writer, err error) {

err, frame, ok := UnwrapFrame(err)
for ok {
  printFrame(w, frame)
  err, frame, ok = UnwrapFrame(err)
}

if err == nil {
  // end of stack
  return
}

if multi, ok := err.(interface{ Unwrap() []error }); ok {
  for _, err := range multi.Unwrap() {
    fmt.Fprintln(w) // separate traces with newlines
    printTrace(w, err)
  }
}

abhinav added a commit that referenced this issue Apr 15, 2024
Per discussion in #71, add an UnwrapFrame function
that wraps the outermost errtrace frame from an error.
As a result of this change,
it's possible to implement buildTraceTree or a variant of it
using exported functionality exclusively.

This makes it possible for users to use a different trace formatting
mechanism than the trees generated by FormatString.

Resolves #71
abhinav added a commit that referenced this issue May 4, 2024
Per discussion in #71, add an UnwrapFrame function
that wraps the outermost errtrace frame from an error.
As a result of this change,
it's possible to implement buildTraceTree or a variant of it
using exported functionality exclusively.

This makes it possible for users to use a different trace formatting
mechanism than the trees generated by FormatString.

Resolves #71
@abhinav abhinav closed this as completed in c3fabd4 May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants