Skip to content

Fix handling timeout errors #728

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

Closed
wants to merge 1 commit into from
Closed

Conversation

mvijaykarthik
Copy link

@mvijaykarthik mvijaykarthik commented Mar 19, 2018

The comment in database/sql/driver/driver.go mentions

// To prevent duplicate operations, ErrBadConn should NOT be returned
// if there's a possibility that the database server might have
// performed the operation. Even if the server sends back an error,
// you shouldn't return ErrBadConn.
var ErrBadConn = errors.New("driver: bad connection")

Network error could be a timeout error where the database might
have executed the query.

This commit adds a check so that ErrBadConn is not returned
for timeout errors

Fixes #422

@andreimatei
Copy link

Please reference #422 in the commit.
Haven't looked at the PR, but I believe the problem is more general than just timeouts.

The comment in `database/sql/driver/driver.go` mentions
```
// To prevent duplicate operations, ErrBadConn should NOT be returned
// if there's a possibility that the database server might have
// performed the operation. Even if the server sends back an error,
// you shouldn't return ErrBadConn.
var ErrBadConn = errors.New("driver: bad connection")
```

Network error could be a timeout error where the database might
have executed the query.

This commit adds a check so that ErrBadConn is not returned
for timeout errors

Fixes lib#422
@mvijaykarthik
Copy link
Author

mvijaykarthik commented Mar 19, 2018

Referenced the issue in the commit. The original post in #422 is related to timeouts. I'm facing a similar issue where an INSERT query times out, and is internally retried by database/sql, and I get a duplicate primary key error for a row being inserted the first time (because the first insert succeeded in the database, but timed out).

maddyblue added a commit that referenced this pull request Mar 19, 2018
This has been bad behavior for a long time and no one has brought up
any reasons not to change it in #422. This is a more general fix than
in #728.

Fixes #422
Closes #728
@maddyblue
Copy link
Collaborator

maddyblue commented Mar 19, 2018

I agree this should apply for more errors. I've opened #730 to address this.

maddyblue added a commit that referenced this pull request Mar 19, 2018
This has been bad behavior for a long time and no one has brought up
any reasons not to change it in #422. This is a more general fix than
in #728.

Fixes #422
Closes #728
maddyblue added a commit that referenced this pull request Mar 21, 2018
This has been bad behavior for a long time and no one has brought up
any reasons not to change it in #422. This is a more general fix than
in #728.

Fixes #422
Closes #728
maddyblue added a commit that referenced this pull request Mar 25, 2018
This has been bad behavior for a long time and no one has brought up
any reasons not to change it in #422. This is a more general fix than
in #728.

Fixes #422
Closes #728
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants