Skip to content
This repository was archived by the owner on May 24, 2023. It is now read-only.

Use errors.As()/errors.Is() syntax#108

Closed
jeffwidman wants to merge 1 commit intoVividCortex:gh-pagesfrom
jeffwidman:patch-1
Closed

Use errors.As()/errors.Is() syntax#108
jeffwidman wants to merge 1 commit intoVividCortex:gh-pagesfrom
jeffwidman:patch-1

Conversation

@jeffwidman
Copy link
Copy Markdown

Use the new errors.Is()/errors.As() syntax from go 1.13.

Use the new `errors.Is()`/`errors.As()` syntax from `go 1.13`.
@jeffwidman
Copy link
Copy Markdown
Author

tagging @pmatseykanets for visibility... looks like this repo has a few PR's that could be quickly merged or rejected in addition to this one...

@jeffwidman
Copy link
Copy Markdown
Author

nudge @efepapa @gkristic-vc

Copy link
Copy Markdown

@gkristic-vc gkristic-vc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Jeff. Thanks! I am not particularly inclined to this use of Is, though. The database/sql package has a well defined contract and errors are part of it. Well defined errors like sql.ErrNoRows can't be wrapped at this point without breaking the API. On the flip side, if an inner error was sql.ErrNoRows and the package decided to return something else (as to make Is different than ==), then Is would actually introduce a dependency on the internal implementation. Would you agree?

The use of As to catch specific driver errors makes more sense, in my opinion, although as of 1.14.2 the database/sql package doesn't seem to be wrapping errors at all. If it did, it'd likely break many applications that check for driver errors directly. So I'm not sure that this will ever be changed, at least for Go 1.x.

@jeffwidman
Copy link
Copy Markdown
Author

No problem. I don't fully agree with your reasoning, but it's your project, so your call.

@jeffwidman jeffwidman closed this Oct 6, 2022
@jeffwidman jeffwidman deleted the patch-1 branch October 6, 2022 05:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants