From d9709e7eec448e39c7cec4303446511a2e29be58 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Thu, 4 Jan 2018 18:23:09 -0800 Subject: [PATCH] driver.ErrBadConn when init packet read fails when initializing a connection, we can return drivers.ErrBadConn so that the sql package can use this information to retry getting a connection. the sql package will not give us this behavior unless we return that specific error. pursuant to #302, this operation is another in the limited set of things that seems safe to retry. we are trying to move to the post-302 world and ran into this :) I've run into this when using the new drivers.Conn() interface specifically, though after digging into the sql package this seems like a similar path for any query. After changing this return, I end up eventually getting a new conn that is valid. It appears in my repro like the db was not yet ready, and would just end up returning an invalid conn and giving up, the InvalidConn err came from https://github.com/go-sql-driver/mysql/blob/4a0c3d73d8579f9fc535cf5e654a651cbd57dd6e/packets.go#L38 which was the first read on the packet (an EOF). At least, returning a drivers.ErrBadConn to let the db retry initializing the connection seems like a safe operation. thanks for maintaining this library :) --- AUTHORS | 1 + packets.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/AUTHORS b/AUTHORS index 861905d23..4702c83ab 100644 --- a/AUTHORS +++ b/AUTHORS @@ -60,6 +60,7 @@ oscarzhao Paul Bonser Peter Schultz Rebecca Chin +Reed Allman Runrioter Wung Robert Russell Shuode Li diff --git a/packets.go b/packets.go index 4e66639c4..18251aaf1 100644 --- a/packets.go +++ b/packets.go @@ -157,6 +157,11 @@ func (mc *mysqlConn) writePacket(data []byte) error { func (mc *mysqlConn) readInitPacket() ([]byte, error) { data, err := mc.readPacket() if err != nil { + // for init we can rewrite this to ErrBadConn for sql.Driver to retry, since + // in connection initialization we don't risk retrying non-idempotent actions. + if err == ErrInvalidConn { + return nil, driver.ErrBadConn + } return nil, err }