rotisserie/eris

Unpack external errors (especially coming from pkg/errors)

dnaka91 opened this issue ยท 7 comments

Is your feature request related to a problem? Please describe.
When getting errors from other libraries that use github.com/pkg/errors or forks of it, I noticed that the error is just flattened by calling error.Error() and then returning a rootError.
This is fine for custom errors, but pkg/errors is very widespread and integration with it would make the created errors more complete.

Describe the solution you'd like
It would be nice if eris tries to unpack the error instead of just turning it into string, by recursively calling Unpack. As pkg/errors is very popular, checking for the old func Cause() error in addition to func Unpack() error would be great.

Not just errors from pkg/errors but any error in general that implements Unpack makes sense to unpack in my opinion.

Not sure how to unmangle the stack trace though.

Describe alternatives you've considered

Additional context

I think while writing the Wrap() method, @morningvera would have come across this weird case of wrapping a wrapped external error and he might have made a conscious decision to just flatten the error by calling error.Error() instead of calling error.Unwrap() because fetching the original stack for this externally wrapped error would be impractical and/or complex.

A potential solution could be to discard the stack of this error but recursively Unwrap it to at least get all the messages in the chain. But it makes me wonder if that is really useful to the user.

A concocted example:

...
...
// Create an error from pkg/errors and wrap it using the external libraries' Wrap method
err := errors.New("router is off")
err = errors.Wrap("network layer faulty")
err = errors.Wrap("external error")

// Wrap the err in eris
err = eris.Wrap("wrapped external error")

// Print the error without the trace
fmt.Print(fmt.Sprintf("%v",err))

// Current implementation output when printing err without the trace 
wrapped external error: external error

// Proposed implementation output when printing err without the trace 
wrapped external error: external error: network layer faulty: router is off

// Print the error with the trace
fmt.Print(fmt.Sprintf("%+v",err))

// Current implementation output when printing err with the trace
wrapped external error
		------FRAME------
external error
		-----------------
		------STACK------
		------TRACE------
		-----------------

// Proposed implementation output when printing err with the trace
wrapped external error
		------FRAME------
external error: network layer faulty: router is off
		-----------------
		------STACK------
		------TRACE------
		-----------------	
...
...

The stack trace in the proposed output when printing err with trace would probably be misleading as the original stack trace was lost while wrapping the external error.

Personally, to have a coherent solution is preferable over something which is half right.

Thank you for checking so quickly. To give a little more context:
I was checking this library with the goal to get better JSON output in my application with more fine grained detail. The errors will never be seen by a user, as this is a service running periodically in an internal server and errors are visible by developers through the scheduler interface.

The output you describe as proposed implementation is actually the output I currently get from eris as pkg/errors is already concatenating the error chain when calling Error().
Getting the frame for an external error is probably impossible or very hard to get right. So I don't expect to get a frame for every external error but something like this:

// Currently
wrapped external error
		------FRAME------
external error: network layer faulty: router is off
		-----------------
		------STACK------
		------TRACE------
		-----------------

// Proposed
wrapped external error
		------FRAME------
external error
		------
network layer faulty
		------
router is off
		-----------------
		------STACK------
		------TRACE------
		-----------------	

Again, I'm only working with the JSON output not the typical text output of the error. Was just hoping to get the external error as a separate entries in the JSON instead of one long item with a concatenated string.

@dnaka91 I like this idea! I think this is possibly something we can do, but I want to test some things out over the next few days.

@dnaka91 I got what you meant! I misunderstood the initial question. Perhaps we can look into getting frames and stack trace from errors defined by pkg/errors. Although doing it for other error libraries might be an impossible task.

@dnaka91 Just curious as to why would you not completely switch to eris and rather handle errors via two separate libraries?

Hey @sum2000, sorry for the late answer, I was on vacation.

Switching completely within the single project is not a big deal and I already did that on a separate branch for the purpose of evaluating this error library.

The trouble are generators and 3rd party libraries.

  • I can't just simply go to every library owner and ask them to please switch to eris.
  • The project depends on other internal projects from different teams and they have their own rules and tasks, so they can't simply switch.
  • Some code is generated (sqlboiler) and it would be a pain to add and maintain some extra scripts that update the imports after generation.

@dnaka91 thanks for the followup. The reasons you mentioned suffice for supporting the trace inter-op between eris and pkg/errors.