You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add SkipCaller(int) to support helper function around errtrace #70
Quoting: "It's important that the errtrace.Wrap function is called inside the same function that's actually returning the error. A helper function will not suffice."
It will be nice to be able to support helper functions, and mutating the idea from zap.Logger (AddSkipCaller) it should be possible.
Hey, thanks for the issue!
This is definitely possible on the safe path, but much more difficult on the unsafe path.
If there's need for this functionality, we could probably change the unsafe path to fall back to the slow path if skip > 0.
Well, it will be a nice to have, but I don't think it is a must to have functionality.
Anyway thanks for taking it in consideration.
Checking the performance comparison, with the safe approach errtrace will still be comparable with FmtErrorf... so much slower but still (I think) a more than acceptable compromise... and anyway there are no free lunch :)
Thanks! I think this is a valid feature request, and I do think we should do it at some point.
It allows folks to write helpers for error building use cases that we haven't accounted for.
(e.g., if we didn't include errtrace.New, someone couldn't write their own errors.New wrapper that has the correct positioning.)
Out of curiosity, did you have a specific case where you were hoping to use this ability?
I'm just adopting errtrace, so in total honesty I don't have any real use case yet, everything is still a WIP.
I'll most probably work on something around the lines of adding the trace plus a string/annotation/extra value to simplify the life of other users
for example (or something something around this lines): return errtracePlus.wrap(err, <format>, args ...any)
or the capability to join 2 or more errors and add the trace.
Note: I'm not join to call it errtracePlus ;)
The overall point is to make the life as easy as possible to my collegues to lower the entry bar for adoption.
Wanted to double down on this feature. We had implemented something similar to this package but were looking to replace it with this by swapping the functions. We would need a way for skip to be implemented since our package would wrap errtrace.
@theFong Thanks for dropping that in. I think more data points on this issue helps.
I'm curious about the wrappers: if you're implementing your own wrappers, are you not auto-instrumenting your code? While that's not absolutely required for errtrace, we were under the impression that that was highly convenient.
I did see the automatic instrumentation feature(should try it out). But I don't like the idea of having to review the diff across our entire codebase haha. Maybe I misunderstood how this feature works.
We already have a helper wrap function implemented. Our helper api accepts a string(s) and want to keep that functionality by appending it to the error string. func WrapAndTrace(err error, messages ...string) error
Wanted to follow up here. We've ended up adopting errtrace with manual instrumentation and have simply refactored out codebase to use Errorf when adding a message to the error.
Activity
abhinav commentedon Dec 10, 2023
Hey, thanks for the issue!
This is definitely possible on the safe path, but much more difficult on the unsafe path.
If there's need for this functionality, we could probably change the unsafe path to fall back to the slow path if
skip > 0.RobertoMontagna commentedon Dec 15, 2023
Well, it will be a nice to have, but I don't think it is a must to have functionality.
Anyway thanks for taking it in consideration.
Checking the performance comparison, with the safe approach errtrace will still be comparable with FmtErrorf... so much slower but still (I think) a more than acceptable compromise... and anyway there are no free lunch :)
abhinav commentedon Dec 15, 2023
Thanks! I think this is a valid feature request, and I do think we should do it at some point.
It allows folks to write helpers for error building use cases that we haven't accounted for.
(e.g., if we didn't include
errtrace.New, someone couldn't write their ownerrors.Newwrapper that has the correct positioning.)Out of curiosity, did you have a specific case where you were hoping to use this ability?
RobertoMontagna commentedon Dec 16, 2023
I'm just adopting errtrace, so in total honesty I don't have any real use case yet, everything is still a WIP.
I'll most probably work on something around the lines of adding the trace plus a string/annotation/extra value to simplify the life of other users
for example (or something something around this lines):
return errtracePlus.wrap(err, <format>, args ...any)or the capability to join 2 or more errors and add the trace.
Note: I'm not join to call it errtracePlus ;)
The overall point is to make the life as easy as possible to my collegues to lower the entry bar for adoption.
theFong commentedon Jan 25, 2024
Wanted to double down on this feature. We had implemented something similar to this package but were looking to replace it with this by swapping the functions. We would need a way for skip to be implemented since our package would wrap errtrace.
abhinav commentedon Jan 25, 2024
@theFong Thanks for dropping that in. I think more data points on this issue helps.
I'm curious about the wrappers: if you're implementing your own wrappers, are you not auto-instrumenting your code? While that's not absolutely required for errtrace, we were under the impression that that was highly convenient.
theFong commentedon Jan 26, 2024
I did see the automatic instrumentation feature(should try it out). But I don't like the idea of having to review the diff across our entire codebase haha. Maybe I misunderstood how this feature works.
We already have a helper wrap function implemented. Our helper api accepts a string(s) and want to keep that functionality by appending it to the error string.
func WrapAndTrace(err error, messages ...string) errortheFong commentedon Jan 26, 2024
Wanted to follow up here. We've ended up adopting errtrace with manual instrumentation and have simply refactored out codebase to use Errorf when adding a message to the error.
Add GetCaller for use in error helpers (bracesdev#108)