remove errBadConnNoWrite and markBadConn

Author: methane

connection.go

@@ -99,23 +99,12 @@ func (mc *mysqlConn) handleParams() (err error) {
 	return
 }
 
-func (mc *mysqlConn) markBadConn(err error) error {
-	if mc == nil {
-		return err
-	}
-	if err != errBadConnNoWrite {
-		return err
-	}
-	return driver.ErrBadConn
-}
-
 func (mc *mysqlConn) Begin() (driver.Tx, error) {
 	return mc.begin(false)
 }
 
 func (mc *mysqlConn) begin(readOnly bool) (driver.Tx, error) {
 	if mc.closed.Load() {
-		mc.log(ErrInvalidConn)
 		return nil, driver.ErrBadConn
 	}
 	var q string
@@ -128,7 +117,7 @@ func (mc *mysqlConn) begin(readOnly bool) (driver.Tx, error) {
 	if err == nil {
 		return &mysqlTx{mc}, err
 	}
-	return nil, mc.markBadConn(err)
+	return nil, err
 }
 
 func (mc *mysqlConn) Close() (err error) {
@@ -177,15 +166,12 @@ func (mc *mysqlConn) error() error {
 
 func (mc *mysqlConn) Prepare(query string) (driver.Stmt, error) {
 	if mc.closed.Load() {
-		mc.log(ErrInvalidConn)
 		return nil, driver.ErrBadConn
 	}
 	// Send command
 	err := mc.writeCommandPacketStr(comStmtPrepare, query)
 	if err != nil {
-		// STMT_PREPARE is safe to retry.  So we can return ErrBadConn here.
-		mc.log(err)
-		return nil, driver.ErrBadConn
+		return nil, err
 	}
 
 	stmt := &mysqlStmt{
@@ -218,8 +204,8 @@ func (mc *mysqlConn) interpolateParams(query string, args []driver.Value) (strin
 	buf, err := mc.buf.takeCompleteBuffer()
 	if err != nil {
 		// can not take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return "", ErrInvalidConn
+		mc.cleanup()
+		return "", err
 	}
 	buf = buf[:0]
 	argPos := 0
@@ -310,7 +296,6 @@ func (mc *mysqlConn) interpolateParams(query string, args []driver.Value) (strin
 
 func (mc *mysqlConn) Exec(query string, args []driver.Value) (driver.Result, error) {
 	if mc.closed.Load() {
-		mc.log(ErrInvalidConn)
 		return nil, driver.ErrBadConn
 	}
 	if len(args) != 0 {
@@ -330,15 +315,15 @@ func (mc *mysqlConn) Exec(query string, args []driver.Value) (driver.Result, err
 		copied := mc.result
 		return &copied, err
 	}
-	return nil, mc.markBadConn(err)
+	return nil, err
 }
 
 // Internal function to execute commands
 func (mc *mysqlConn) exec(query string) error {
 	handleOk := mc.clearResult()
 	// Send command
 	if err := mc.writeCommandPacketStr(comQuery, query); err != nil {
-		return mc.markBadConn(err)
+		return err
 	}
 
 	// Read Result
@@ -370,7 +355,6 @@ func (mc *mysqlConn) query(query string, args []driver.Value) (*textRows, error)
 	handleOk := mc.clearResult()
 
 	if mc.closed.Load() {
-		mc.log(ErrInvalidConn)
 		return nil, driver.ErrBadConn
 	}
 	if len(args) != 0 {
@@ -410,7 +394,7 @@ func (mc *mysqlConn) query(query string, args []driver.Value) (*textRows, error)
 			return rows, err
 		}
 	}
-	return nil, mc.markBadConn(err)
+	return nil, err
 }
 
 // Gets the value of the given MySQL System Variable
@@ -465,7 +449,6 @@ func (mc *mysqlConn) finish() {
 // Ping implements driver.Pinger interface
 func (mc *mysqlConn) Ping(ctx context.Context) (err error) {
 	if mc.closed.Load() {
-		mc.log(ErrInvalidConn)
 		return driver.ErrBadConn
 	}
 
@@ -476,7 +459,7 @@ func (mc *mysqlConn) Ping(ctx context.Context) (err error) {
 
 	handleOk := mc.clearResult()
 	if err = mc.writeCommandPacket(comPing); err != nil {
-		return mc.markBadConn(err)
+		return err
 	}
 
 	return handleOk.readResultOK()
@@ -682,8 +665,12 @@ func (mc *mysqlConn) ResetSession(ctx context.Context) error {
 	return nil
 }
 
+var _ driver.SessionResetter = &mysqlConn{}
+
 // IsValid implements driver.Validator interface
 // (From Go 1.15)
 func (mc *mysqlConn) IsValid() bool {
 	return !mc.closed.Load()
 }
+
+var _ driver.Validator = &mysqlConn{}

connection_test.go

@@ -163,12 +163,13 @@ func TestPingMarkBadConnection(t *testing.T) {
 		netConn:          nc,
 		buf:              newBuffer(nc),
 		maxAllowedPacket: defaultMaxAllowedPacket,
+		closech:          make(chan struct{}),
 	}
 
 	err := mc.Ping(context.Background())
 
-	if err != driver.ErrBadConn {
-		t.Errorf("expected driver.ErrBadConn, got  %#v", err)
+	if !errors.Is(err, nc.err) {
+		t.Errorf("expected %v, got  %#v", nc.err, err)
 	}
 }
 
@@ -184,8 +185,8 @@ func TestPingErrInvalidConn(t *testing.T) {
 
 	err := mc.Ping(context.Background())
 
-	if err != ErrInvalidConn {
-		t.Errorf("expected ErrInvalidConn, got  %#v", err)
+	if !errors.Is(err, nc.err) {
+		t.Errorf("expected %v, got  %#v", nc.err, err)
 	}
 }
 

errors.go

@@ -29,12 +29,6 @@ var (
 	ErrPktSyncMul        = errors.New("commands out of sync. Did you run multiple statements at once?")
 	ErrPktTooLarge       = errors.New("packet for query is too large. Try adjusting the `Config.MaxAllowedPacket`")
 	ErrBusyBuffer        = errors.New("busy buffer")
-
-	// errBadConnNoWrite is used for connection errors where nothing was sent to the database yet.
-	// If this happens first in a function starting a database interaction, it should be replaced by driver.ErrBadConn
-	// to trigger a resend.
-	// See https://github.com/go-sql-driver/mysql/pull/302
-	errBadConnNoWrite = errors.New("bad connection")
 )
 
 var defaultLogger = Logger(log.New(os.Stderr, "[mysql] ", log.Ldate|log.Ltime|log.Lshortfile))

packets.go

@@ -122,32 +122,25 @@ func (mc *mysqlConn) writePacket(data []byte) error {
 		}
 
 		n, err := mc.netConn.Write(data[:4+size])
-		if err == nil && n == 4+size {
-			mc.sequence++
-			if size != maxPacketSize {
-				return nil
-			}
-			pktLen -= size
-			data = data[size:]
-			continue
-		}
-
-		// Handle error
-		if err == nil { // n != len(data)
+		if err != nil {
 			mc.cleanup()
-			mc.log(ErrMalformPkt)
-		} else {
 			if cerr := mc.canceled.Value(); cerr != nil {
 				return cerr
 			}
-			if n == 0 && pktLen == len(data)-4 {
-				// only for the first loop iteration when nothing was written yet
-				return errBadConnNoWrite
-			}
+			return err
+		}
+		if n != size+4 {
 			mc.cleanup()
-			mc.log(err)
+			return io.ErrShortWrite
 		}
-		return ErrInvalidConn
+
+		mc.sequence++
+		if size != maxPacketSize {
+			return nil
+		}
+		pktLen -= size
+		data = data[size:]
+		continue
 	}
 }
 
@@ -303,8 +296,8 @@ func (mc *mysqlConn) writeHandshakeResponsePacket(authResp []byte, plugin string
 	data, err := mc.buf.takeBuffer(pktLen + 4)
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup() // Avoid repeated "busy buffer" errors.
+		return err
 	}
 
 	// ClientFlags [32 bit]
@@ -392,8 +385,8 @@ func (mc *mysqlConn) writeAuthSwitchPacket(authData []byte) error {
 	data, err := mc.buf.takeSmallBuffer(pktLen)
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup()
+		return err
 	}
 
 	// Add the auth data [EOF]
@@ -412,8 +405,8 @@ func (mc *mysqlConn) writeCommandPacket(command byte) error {
 	data, err := mc.buf.takeSmallBuffer(4 + 1)
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup()
+		return err
 	}
 
 	// Add command byte
@@ -431,8 +424,8 @@ func (mc *mysqlConn) writeCommandPacketStr(command byte, arg string) error {
 	data, err := mc.buf.takeBuffer(pktLen + 4)
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup()
+		return err
 	}
 
 	// Add command byte
@@ -452,8 +445,8 @@ func (mc *mysqlConn) writeCommandPacketUint32(command byte, arg uint32) error {
 	data, err := mc.buf.takeSmallBuffer(4 + 1 + 4)
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup()
+		return err
 	}
 
 	// Add command byte
@@ -994,8 +987,8 @@ func (stmt *mysqlStmt) writeExecutePacket(args []driver.Value) error {
 	}
 	if err != nil {
 		// cannot take the buffer. Something must be wrong with the connection
-		mc.log(err)
-		return errBadConnNoWrite
+		mc.cleanup()
+		return err
 	}
 
 	// command [1 byte]
@@ -1193,8 +1186,8 @@ func (stmt *mysqlStmt) writeExecutePacket(args []driver.Value) error {
 		if valuesCap != cap(paramValues) {
 			data = append(data[:pos], paramValues...)
 			if err = mc.buf.store(data); err != nil {
-				mc.log(err)
-				return errBadConnNoWrite
+				mc.cleanup()
+				return err
 			}
 		}
 

statement.go

@@ -57,7 +57,7 @@ func (stmt *mysqlStmt) Exec(args []driver.Value) (driver.Result, error) {
 	// Send command
 	err := stmt.writeExecutePacket(args)
 	if err != nil {
-		return nil, stmt.mc.markBadConn(err)
+		return nil, err
 	}
 
 	mc := stmt.mc
@@ -95,13 +95,12 @@ func (stmt *mysqlStmt) Query(args []driver.Value) (driver.Rows, error) {
 
 func (stmt *mysqlStmt) query(args []driver.Value) (*binaryRows, error) {
 	if stmt.mc.closed.Load() {
-		stmt.mc.log(ErrInvalidConn)
 		return nil, driver.ErrBadConn
 	}
 	// Send command
 	err := stmt.writeExecutePacket(args)
 	if err != nil {
-		return nil, stmt.mc.markBadConn(err)
+		return nil, err
 	}
 
 	mc := stmt.mc