Skip to content
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

handle driver timeout error #8

Merged
merged 3 commits into from
Aug 2, 2020

Conversation

shlomi-noach
Copy link

Heh, sorry for the mess. Replaces #3

re-submitting github#835 by @ajm188

Closes github#822.

In go-sql-driver/mysql#1075, @acharis notes
that the way the go-sql driver is written, query timeout errors don't
get set in rows.Err() until after a call to rows.Next() is made.

Because this kind of error means there will be no rows in the result
set, the for rows.Next() will never enter the for loop, so we must
check the value of rows.Err() after the loop, and surface the error up
appropriately.

I then went ahead and fixed up the error checking in the two remaining
places we use db.Query, and validated I've covered all cases by
running:

$ golangci-lint run --disable-all -E rowserrcheck

ajm188 and others added 3 commits March 31, 2020 16:25
Closes github#822.

In go-sql-driver/mysql#1075, @acharis notes
that the way the go-sql driver is written, query timeout errors don't
get set in `rows.Err()` until _after_ a call to `rows.Next()` is made.

Because this kind of error means there will be no rows in the result
set, the `for rows.Next()` will never enter the for loop, so we must
check the value of `rows.Err()` after the loop, and surface the error up
appropriately.
@shlomi-noach shlomi-noach merged commit 6012e80 into master Aug 2, 2020
@shlomi-noach shlomi-noach deleted the ajm188-handle_driver_timeout_error branch August 2, 2020 08:57
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.

Server query timeout lead to data loss
2 participants