From d115a62f60685a86d8847f5b584776af51fe5c6b Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 12 Oct 2018 10:37:08 +0200 Subject: [PATCH 1/5] return `driver.ErrBadConn` if `Dial()` fails because this error is retried in the driver and means the connection never succeeded ``` // ErrBadConn should be returned by a driver to signal to the sql // package that a driver.Conn is in a bad state (such as the server // having earlier closed the connection) and the sql package should // retry on a new connection. // // 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. ``` --- driver.go | 2 +- driver_test.go | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/driver.go b/driver.go index ba1297825..881cf1383 100644 --- a/driver.go +++ b/driver.go @@ -77,7 +77,7 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { mc.netConn, err = nd.Dial(mc.cfg.Net, mc.cfg.Addr) } if err != nil { - return nil, err + return nil, driver.ErrBadConn } // Enable TCP Keepalives on TCP connections diff --git a/driver_test.go b/driver_test.go index f2bf344e5..3eab832b7 100644 --- a/driver_test.go +++ b/driver_test.go @@ -1801,6 +1801,23 @@ func TestConcurrent(t *testing.T) { }) } +func TestDialErrBadConn(t *testing.T) { + RegisterDial("mydial", func(addr string) (net.Conn, error) { + return nil, fmt.Errorf("test") + }) + + db, err := sql.Open("mysql", fmt.Sprintf("%s:%s@mydial(%s)/%s?timeout=30s", user, pass, addr, dbname)) + if err != nil { + t.Fatalf("error connecting: %s", err.Error()) + } + defer db.Close() + + _, err = db.Exec("DO 1") + if err != driver.ErrBadConn { + t.Fatalf("was expecting ErrBadConn. Got: %s", err) + } +} + // Tests custom dial functions func TestCustomDial(t *testing.T) { if !available { From f0fc32905d1781d5cd487b7404a58eebfc3f527f Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 12 Oct 2018 11:33:00 +0200 Subject: [PATCH 2/5] log actual error when dial() fails --- driver.go | 1 + 1 file changed, 1 insertion(+) diff --git a/driver.go b/driver.go index 881cf1383..08f2e2c26 100644 --- a/driver.go +++ b/driver.go @@ -77,6 +77,7 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { mc.netConn, err = nd.Dial(mc.cfg.Net, mc.cfg.Addr) } if err != nil { + errLog.Print("err from Dial()': ", err.Error()) return nil, driver.ErrBadConn } From 5d02613b3e85dd4f853f853dc4307b08d4f67d08 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Fri, 12 Oct 2018 13:34:56 +0200 Subject: [PATCH 3/5] add name to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index fbe4ec442..1c37f1ee8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -73,6 +73,7 @@ Soroush Pour Stan Putrya Stanley Gunawan Thomas Wodarek +Tom Jenkinson Xiangyu Hu Xiaobing Jiang Xiuming Chen From bc42363afd0124f428ef0ef06daf41bfb0d2d234 Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Thu, 18 Oct 2018 15:52:38 +0200 Subject: [PATCH 4/5] only return driver.ErrBadConn on temporary error or timeout --- driver.go | 7 +++++-- driver_test.go | 45 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/driver.go b/driver.go index 08f2e2c26..5ef035406 100644 --- a/driver.go +++ b/driver.go @@ -77,8 +77,11 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { mc.netConn, err = nd.Dial(mc.cfg.Net, mc.cfg.Addr) } if err != nil { - errLog.Print("err from Dial()': ", err.Error()) - return nil, driver.ErrBadConn + if nerr, ok := err.(net.Error); ok && (nerr.Temporary() || nerr.Timeout()) { + errLog.Print("net.Error from Dial()': ", nerr.Error()) + return nil, driver.ErrBadConn + } + return nil, err } // Enable TCP Keepalives on TCP connections diff --git a/driver_test.go b/driver_test.go index 3eab832b7..a8ba257b2 100644 --- a/driver_test.go +++ b/driver_test.go @@ -85,6 +85,23 @@ type DBTest struct { db *sql.DB } +type netErrorMock struct { + temporary bool + timeout bool +} + +func (e netErrorMock) Temporary() bool { + return e.temporary +} + +func (e netErrorMock) Timeout() bool { + return e.timeout +} + +func (e netErrorMock) Error() string { + return fmt.Sprintf("mock net error. Temporary: %v, Timeout %v", e.temporary, e.timeout) +} + func runTestsWithMultiStatement(t *testing.T, dsn string, tests ...func(dbt *DBTest)) { if !available { t.Skipf("MySQL server not running on %s", netAddr) @@ -1801,9 +1818,9 @@ func TestConcurrent(t *testing.T) { }) } -func TestDialErrBadConn(t *testing.T) { +func testDialError(t *testing.T, dialErr error, expectErr error) { RegisterDial("mydial", func(addr string) (net.Conn, error) { - return nil, fmt.Errorf("test") + return nil, dialErr }) db, err := sql.Open("mysql", fmt.Sprintf("%s:%s@mydial(%s)/%s?timeout=30s", user, pass, addr, dbname)) @@ -1813,11 +1830,31 @@ func TestDialErrBadConn(t *testing.T) { defer db.Close() _, err = db.Exec("DO 1") - if err != driver.ErrBadConn { - t.Fatalf("was expecting ErrBadConn. Got: %s", err) + if err != expectErr { + t.Fatalf("was expecting %s. Got: %s", dialErr, err) } } +func TestDialUnknownError(t *testing.T) { + testErr := fmt.Errorf("test") + testDialError(t, testErr, testErr) +} + +func TestDialNonRetryableNetErr(t *testing.T) { + testErr := netErrorMock{} + testDialError(t, testErr, testErr) +} + +func TestDialTimeoutNetErr(t *testing.T) { + testErr := netErrorMock{timeout: true} + testDialError(t, testErr, driver.ErrBadConn) +} + +func TestDialTemporaryNetErr(t *testing.T) { + testErr := netErrorMock{temporary: true} + testDialError(t, testErr, driver.ErrBadConn) +} + // Tests custom dial functions func TestCustomDial(t *testing.T) { if !available { From eebeab7ae28970ffbf9608e117dc05ff6a5b52ba Mon Sep 17 00:00:00 2001 From: Tom Jenkinson Date: Sat, 20 Oct 2018 11:57:25 +0200 Subject: [PATCH 5/5] only return driver.ErrBadConn on temporary error --- driver.go | 2 +- driver_test.go | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/driver.go b/driver.go index 5ef035406..eeb83df01 100644 --- a/driver.go +++ b/driver.go @@ -77,7 +77,7 @@ func (d MySQLDriver) Open(dsn string) (driver.Conn, error) { mc.netConn, err = nd.Dial(mc.cfg.Net, mc.cfg.Addr) } if err != nil { - if nerr, ok := err.(net.Error); ok && (nerr.Temporary() || nerr.Timeout()) { + if nerr, ok := err.(net.Error); ok && nerr.Temporary() { errLog.Print("net.Error from Dial()': ", nerr.Error()) return nil, driver.ErrBadConn } diff --git a/driver_test.go b/driver_test.go index a8ba257b2..cec4b5867 100644 --- a/driver_test.go +++ b/driver_test.go @@ -1845,11 +1845,6 @@ func TestDialNonRetryableNetErr(t *testing.T) { testDialError(t, testErr, testErr) } -func TestDialTimeoutNetErr(t *testing.T) { - testErr := netErrorMock{timeout: true} - testDialError(t, testErr, driver.ErrBadConn) -} - func TestDialTemporaryNetErr(t *testing.T) { testErr := netErrorMock{temporary: true} testDialError(t, testErr, driver.ErrBadConn)