-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Find replacement for https://github.com/pkg/errors #5176
Comments
I personally could live with your two first points; having stack trace functionality is nice. Option B would make most sense, at least for the time being, to me. |
Hi @bwplotka & @matej-g, Can I work on this?
If you wish to go with this approach, I have a better option. Recently, I have worked with a package It's very small (hardly 200 lines total) yet very elegant code (supports both wrapping and unwrapping). I think we can borrow it. What do you think? I'd be happy to the replacement. Thank you. |
Hey @bisakhmondal, thanks for your interest! Do you have any opinion on which solution to go with? Let's see if @bwplotka has any further opinion which route to take as well, would be cool if you'd take it! 🚀 |
Sure. I'd love to take it. Having a stack trace definitely helps in RCA and is easy to debug, provided it's simple (as pointed out by @bwplotka ). I think we can definitely pull it off if it's simple (and definitely with extensive documentation to ease code maintainability). And in this case, the source code that I shared is extremely simple and much much simpler than pkg/errors. Also the joy that comes while playing with the runtime package is off the charts : ) |
Awesome! I'd say if you found a good replacement, the code is not too complex and you have a good idea of how we can leverage it in Thanos repository, go for it 🚀 |
Yeah sure, on it. Thank you |
Looking at so many references to this issue and the PR, how about we publish it as a separate go pkg. Lots of redundant work might be saved. |
https://github.com/pkg/errors is now archived since
errors
package has a way to wrap errors AND the main maintainer is stepping down from the maintainer team. Here are details pkg/errors#245Unfortunately, default library
errors
wrapping is not the best. Personally, I don't like three facts:fmt.Errors("something: %w", err)
with%w
which is ultra-easy to forgot and do %v instead. There are linters for that, but add extra lint when our language is compiled... feels weird. Plus it is prone to inconsistent wrapping.errors.New
sometimes then relying onfmt
to wrap errors does not look clean. Swapping betweenfmt
anderrors
in return looks like unnecessary complexity for the reader.Options:
A. We could write our own wrapper over
errors
to accomplish this, borrowing some code from pkg/errors, but it is (especially stack tracing) a bit of complexity to the maintainer.B. We could use other drop-in replacements https://github.com/emperror/errors - it was not touched lately but seems like not many issues are on it. The API is a bit large though.
C. We could live without stack trace functionality. This is what the majority of projects are doing lately.
I am not a fan of standard library flow as mentioned before, and I don't think we mind setting a new standards in the ecosystem (e.g with https://github.com/efficientgo/tools/tree/main/core). But for time being, maybe it makes sense to start with B and check how it goes first? 🤔
Thoughts?
The text was updated successfully, but these errors were encountered: