search

pkg / errors

Simple error handling primitives
https://godoc.org/github.com/pkg/errors
BSD 2-Clause "Simplified" License
8.18k stars 691 forks source link

feat: support std errors functions #213

Closed Sherlock-Holo closed 4 years ago

Sherlock-Holo commented 4 years ago

add function Is, As and Unwrap, like std errors, so that we can continue to use pkg/errors with go1.13 compatibility

jayschwa commented 4 years ago

If this package is going into maintenance mode (according to the README), why add new API surface area? "Maintenance mode" and "drop-in replacement" seem like competing motivations, especially as the standard library beefs up its package.

abraithwaite commented 4 years ago

If this package is going into maintenance mode (according to the README), why add new API surface area? "Maintenance mode" and "drop-in replacement" seem like competing motivations, especially as the standard library beefs up its package.

The change is small, it improves the user experience, and probably improves interoperability of tooling as well (less imports, don't have to alias the import). I think it would be silly not to include it personally.

Sherlock-Holo commented 4 years ago

@aperezg please see are there anything need to improve, if has, let me know to optimize codes :-)

mfridman commented 4 years ago

@jayschwa why return nil instead of the original error that was passed in?

https://github.com/pkg/errors/pull/213#discussion_r346045555

I.e., Unwrap may not get a causer or unwrapper, so shouldn't it return the error without modification?

jayschwa commented 4 years ago

@jayschwa why return nil instead of the original error that was passed in?

Because we want to match the behavior of the standard library's Unwrap:

Unwrap returns the result of calling the Unwrap method on err, if err's type contains an Unwrap method returning error. Otherwise, Unwrap returns nil.

johan-lejdung commented 4 years ago

What's the status on this PR, it's been almost a month since the initiative to update this package started. I'd be happy to help out any way I can.

Sherlock-Holo commented 4 years ago

you can tell us if you want these functions only support 113 or all version

Johan Lejdung notifications@github.com 于 2019年11月24日周日 20:46写道：

What's the status on this PR, it's been almost a month since the initiative to update this package started. I'd be happy to help out any way I can.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pkg/errors/pull/213?email_source=notifications&email_token=ACNA6KLNDDD27ZBXC5HOZS3QVJZTBA5CNFSM4JIMD2U2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFAKNTQ#issuecomment-557885134, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNA6KJ3S7DYMDY2VEO5RN3QVJZTBANCNFSM4JIMD2UQ .

aperezg commented 4 years ago

Sorry I was very busy this month, I'll try to catch up with all comments ASAP

Sherlock-Holo commented 4 years ago

@aperezg I think this PR should focus on add this functions for 1.13, and if many<1.13 users want them, we cloud create another PR to help them

johan-lejdung commented 4 years ago

I agree with that @Sherlock-Holo

The pr should focus on adding support for the new std errors functionalities. Rebuilding those functionalities for use on <1.13 doesn't make as much sense and isn't as pressing of an issue

ajpetersons commented 4 years ago

How soon could this get merged? I would really love to see this PR merged in master and tagged so we can use Go 1.13 functions.

Sherlock-Holo commented 4 years ago

these days I am a little busy too…maybe this weekend I can update this pr for just support these functions >=1.13

Artūrs Jānis Pētersons notifications@github.com 于 2019年12月6日周五 21:24写道：

How soon could this get merged? I would really love to see this PR merged in master and tagged so we can use Go 1.13 functions.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pkg/errors/pull/213?email_source=notifications&email_token=ACNA6KLGVZRYDBMULWRDMV3QXJHAPA5CNFSM4JIMD2U2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGECFXA#issuecomment-562569948, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNA6KJCDXV33TCZNHPEPBLQXJHAPANCNFSM4JIMD2UQ .

aperezg commented 4 years ago

For me, I think that this PR would be only for add support to the new features on Go 1.13, so I think that as is now seems good.

these days I am a little busy too…maybe this weekend I can update this pr for just support these functions >=1.13

@Sherlock-Holo What do yo need to update on this PR to be merged?

Furthermore I see that @puellanivis and @jayschwa I see that you also reviewed this PR, is good for you now?

Sherlock-Holo commented 4 years ago

@aperezg Nothing need to do before merge, now this PR only add new feature for >= 1.13