Skip to content

Commit

Permalink
remove errBadConnNoWrite and markBadConn
Browse files Browse the repository at this point in the history
  • Loading branch information
methane committed Jun 14, 2024
1 parent 87443b9 commit e0d410d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 70 deletions.
34 changes: 12 additions & 22 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,23 +111,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
Expand All @@ -140,7 +129,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) {
Expand Down Expand Up @@ -189,7 +178,6 @@ 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
Expand Down Expand Up @@ -324,7 +312,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 {
Expand All @@ -344,15 +331,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
Expand Down Expand Up @@ -382,11 +369,10 @@ func (mc *mysqlConn) Query(query string, args []driver.Value) (driver.Rows, erro

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 {
if !mc.cfg.InterpolateParams {
return nil, driver.ErrSkip
Expand All @@ -398,17 +384,18 @@ func (mc *mysqlConn) query(query string, args []driver.Value) (*textRows, error)
}
query = prepared
}

// Send command
err := mc.writeCommandPacketStr(comQuery, query)
if err != nil {
return nil, mc.markBadConn(err)
return nil, err
}

// Read Result
var resLen int
resLen, err = handleOk.readResultSetHeaderPacket()
if err != nil {
return nil, mc.markBadConn(err)
return nil, err
}

rows := new(textRows)
Expand Down Expand Up @@ -482,7 +469,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
}

Expand All @@ -493,7 +479,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()
Expand Down Expand Up @@ -699,8 +685,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{}
9 changes: 5 additions & 4 deletions connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down
6 changes: 0 additions & 6 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
64 changes: 29 additions & 35 deletions packets.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,39 +117,33 @@ func (mc *mysqlConn) writePacket(data []byte) error {
// Write packet
if mc.writeTimeout > 0 {
if err := mc.netConn.SetWriteDeadline(time.Now().Add(mc.writeTimeout)); err != nil {
mc.cleanup()
mc.log(err)
mc.cleanup()
return err
}
}

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 != 4+size {
// io.Writer(b) must return a non-nil error if it cannot write len(b) bytes.
// The io.ErrShortWrite error is used to indicate that this rule has not been followed.
mc.cleanup()
mc.log(err)
return io.ErrShortWrite
}
return ErrInvalidConn

mc.sequence++
if size != maxPacketSize {
return nil
}
pktLen -= size
data = data[size:]
}
}

Expand Down Expand Up @@ -305,8 +299,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()
return err
}

// ClientFlags [32 bit]
Expand Down Expand Up @@ -394,8 +388,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]
Expand All @@ -414,8 +408,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
Expand All @@ -433,8 +427,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
Expand All @@ -454,8 +448,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
Expand Down Expand Up @@ -997,8 +991,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]
Expand Down Expand Up @@ -1196,8 +1190,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
}
}

Expand Down
5 changes: 2 additions & 3 deletions statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e0d410d

Please sign in to comment.