scany
scany copied to clipboard
georgysavva
use fmt to wrap errors, not archived pkg/error
currently, github.com/pkg/errors is used to return error, however since go 1.13 introduced error wrapping, this is deprecated, and the repo has been archived since.
Hi. Thanks for mentioning this; I didn't know that. Do you experience any particular problem with scany using the github.com/pkg/errors package?
Although it's archived, I don't see any urgent reasons to replace it.
But If it's something you would like to work on, I encourage you to shoot a PR with the change.
firstly I'd say this error checking isn't very idiomatic after 1.13 release AFAIK.
it's more idiomatic to export the err, such as ErrNotFound, then users check it like errors.Is(err, scany.ErrNotFound), instead of the current scany.NotFound(err).
another major issue with pkg/errors especially WithStack is that, iirc, you can't call it more than 1 time. e.g., if I use this lib, and again wrap it with WithStack in my code, the previous stack info in this lib will be lost. or, if one day this lib needs to rely another lib which does WithStack there, the stack info there is lost. I haven't used it for quite long, feel free to correct me.
it's more idiomatic to export the err, such as ErrNotFound, then users check it like errors.Is(err, scany.ErrNotFound), instead of the current scany.NotFound(err).
Maybe you are right, and after 1.13 errors.Is(err, scany.ErrNotFound) makes more sense. But I don't see any problems with the scany.NotFound(err) approach either.
another major issue with pkg/errors especially WithStack is that, iirc, you can't call it more than 1 time
I think you are mistaken; it captures all stacks and keeps piling them. Then, later, you unwrap them with errors.Is() or errors.As() when checking for known errors.
All in all, I am optimistic about replacing pkg/errors with the standard implementation, but I would want not to break the current scany API.
Maybe you are right, and after 1.13
errors.Is(err, scany.ErrNotFound)makes more sense. But I don't see any problems with thescany.NotFound(err)approach either.
I suppose the issue is with idiom. users new to scany wouldn't know they need this to check for a certain error
All in all, I am optimistic about replacing
pkg/errorswith the standard implementation, but I would want not to break the current scany API.
sure, we can have both, i.e., export the ErrNotFound, wrap them, and change the scany.NotFound(err) with checks like errors.Is(err, ErrNotFound) (with the standard lib errors) , how does that sound to you?
I suppose the issue is with idiom. users new to scany wouldn't know they need this to check for a certain error
We could solve it by providing the documentation. Besides that, the new users, after not founding any exported sentinel error in the library, would most likely discover the function with the corresponding name.
sure, we can have both, i.e., export the ErrNotFound, wrap them, and change the scany.NotFound(err) with checks like errors.Is(err, ErrNotFound) (with the standard lib errors) , how does that sound to you?
Sounds good!
We could solve it by providing the documentation. Besides that, the new users, after not founding any exported sentinel error in the library, would most likely discover the function with the corresponding name.
Haha, exactly what I did. 😃
sure, we can have both, i.e., export the ErrNotFound, wrap them, and change the scany.NotFound(err) with checks like errors.Is(err, ErrNotFound) (with the standard lib errors) , how does that sound to you?
Sounds good!
I'll shoot a PR for a small subset of errors, if you'd find that appropriate, will do it for all.
+1 on switching to stdlib, i use a framework that check errors with errors.Is and translates to a corresponding API error. Incompatible with the current approach, need to add some hacks around it
Here is the new release with stdlib errors, thanks to @mrkagelui: https://github.com/georgysavva/scany/releases/tag/v1.2.0 cc: @steeling