Skip to content

Commit

Permalink
remove unnecessary logs (go-sql-driver#1599)
Browse files Browse the repository at this point in the history
Logging ErrInvalidConn when the connection already closed doesn't
provide any help to users.

Additonally, database/sql now uses Validator() to check connection
liveness before calling query methods.
So stop using `mc.log(ErrInvalidConn)` idiom.

This PR includes some cleanup and documentation relating to
`mc.markBadConn()`.
  • Loading branch information
methane authored Jun 16, 2024
1 parent 2f69712 commit 52c1917
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 15 deletions.
21 changes: 9 additions & 12 deletions connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,13 @@ func (mc *mysqlConn) handleParams() (err error) {
return
}

// markBadConn replaces errBadConnNoWrite with driver.ErrBadConn.
// This function is used to return driver.ErrBadConn only when safe to retry.
func (mc *mysqlConn) markBadConn(err error) error {
if mc == nil {
return err
}
if err != errBadConnNoWrite {
return err
if err == errBadConnNoWrite {
return driver.ErrBadConn
}
return driver.ErrBadConn
return err
}

func (mc *mysqlConn) Begin() (driver.Tx, error) {
Expand All @@ -127,7 +126,6 @@ func (mc *mysqlConn) Begin() (driver.Tx, error) {

func (mc *mysqlConn) begin(readOnly bool) (driver.Tx, error) {
if mc.closed.Load() {
mc.log(ErrInvalidConn)
return nil, driver.ErrBadConn
}
var q string
Expand Down Expand Up @@ -189,7 +187,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 +321,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 Down Expand Up @@ -384,7 +380,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 {
Expand All @@ -408,7 +403,7 @@ func (mc *mysqlConn) query(query string, args []driver.Value) (*textRows, error)
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 +477,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 Down Expand Up @@ -704,3 +698,6 @@ func (mc *mysqlConn) ResetSession(ctx context.Context) error {
func (mc *mysqlConn) IsValid() bool {
return !mc.closed.Load()
}

var _ driver.SessionResetter = &mysqlConn{}
var _ driver.Validator = &mysqlConn{}
2 changes: 1 addition & 1 deletion errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var (

// 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.
// to trigger a resend. Use mc.markBadConn(err) to do this.
// See https://github.com/go-sql-driver/mysql/pull/302
errBadConnNoWrite = errors.New("bad connection")
)
Expand Down
2 changes: 0 additions & 2 deletions statement.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ func (stmt *mysqlStmt) CheckNamedValue(nv *driver.NamedValue) (err error) {

func (stmt *mysqlStmt) Exec(args []driver.Value) (driver.Result, error) {
if stmt.mc.closed.Load() {
stmt.mc.log(ErrInvalidConn)
return nil, driver.ErrBadConn
}
// Send command
Expand Down Expand Up @@ -95,7 +94,6 @@ 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
Expand Down

0 comments on commit 52c1917

Please sign in to comment.