From e6a4b7ba2f655129e12459a860fecff106438752 Mon Sep 17 00:00:00 2001 From: Eygin Semen Leonidovich Date: Wed, 9 Nov 2022 16:19:44 +0300 Subject: [PATCH 1/5] Mark connection as not good when error on cancellation confirmation (cherry picked from commit c996b77da329362396948cfa3574c52e15fa3c7f) --- mssql.go | 2 ++ token.go | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/mssql.go b/mssql.go index d0361294..1bcce021 100644 --- a/mssql.go +++ b/mssql.go @@ -243,6 +243,8 @@ func (c *Conn) checkBadConn(ctx context.Context, err error, mayRetry bool) error return nil case io.EOF: c.connectionGood = false + case ErrorCancelConfirmation: + c.connectionGood = false case driver.ErrBadConn: // It is an internal programming error if driver.ErrBadConn // is ever passed to this function. driver.ErrBadConn should diff --git a/token.go b/token.go index 5f9e8202..1c138257 100644 --- a/token.go +++ b/token.go @@ -99,6 +99,8 @@ const ( // TODO implement more flags ) +var ErrorCancelConfirmation = errors.New("did not get cancellation confirmation from the server") + // interface for all tokens type tokenStruct interface{} @@ -959,7 +961,7 @@ func (t tokenProcessor) nextToken() (tokenStruct, error) { } // we did not get cancellation confirmation, something is not // right, this connection is not usable anymore - return nil, errors.New("did not get cancellation confirmation from the server") + return nil, ErrorCancelConfirmation } } From 539bf69bd6bab122791a3e9b0f99a6372fe03368 Mon Sep 17 00:00:00 2001 From: Eygin Semen Leonidovich Date: Wed, 9 Nov 2022 18:04:16 +0300 Subject: [PATCH 2/5] tests added (cherry picked from commit 2258616c0d8bc86d3dd6cae21f774c2343ce3eb6) --- mssql_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mssql_test.go b/mssql_test.go index 9c50b831..57beb9ed 100644 --- a/mssql_test.go +++ b/mssql_test.go @@ -127,6 +127,10 @@ func TestCheckBadConn(t *testing.T) { {io.EOF, true, false, newRetryableError(io.EOF), false}, {io.EOF, false, true, io.EOF, false}, {io.EOF, true, true, io.EOF, false}, + {ErrorCancelConfirmation, false, false, ErrorCancelConfirmation, false}, + {ErrorCancelConfirmation, true, false, newRetryableError(ErrorCancelConfirmation), false}, + {ErrorCancelConfirmation, false, true, ErrorCancelConfirmation, false}, + {ErrorCancelConfirmation, true, true, ErrorCancelConfirmation, false}, {netErr, false, false, netErr, false}, {netErr, true, false, newRetryableError(netErr), false}, {netErr, false, true, netErr, false}, From 85e531a2bfde3f2c55f2a52c559aed5a1c7f370e Mon Sep 17 00:00:00 2001 From: Eygin Semen Leonidovich Date: Wed, 9 Nov 2022 18:57:08 +0300 Subject: [PATCH 3/5] query tests added (cherry picked from commit 2fc42765180fd5341dad0a5136a51c035ae71ad9) --- queries_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/queries_test.go b/queries_test.go index 20bcfb6c..139fc684 100644 --- a/queries_test.go +++ b/queries_test.go @@ -5,6 +5,7 @@ import ( "context" "database/sql" "database/sql/driver" + "errors" "fmt" "io" "log" @@ -1387,6 +1388,46 @@ func TestProcessQueryErrors(t *testing.T) { } } +type mockReadWriteCloser struct { + io.ReadWriteCloser +} + +func (*mockReadWriteCloser) Read([]byte) (int, error) { return 0, errors.New("fake err") } + +func TestProcessQueryCancelConfirmationError(t *testing.T) { + tl := testLogger{t: t} + defer tl.StopLogging() + conn := internalConnection(t, &tl) + defer conn.Close() + + stmt, err := conn.prepareContext(context.Background(), "select 1") + if err != nil { + t.Fatal("prepareContext expected to succeed, but it failed with", err) + } + err = stmt.sendQuery(context.Background(), []namedValue{}) + if err != nil { + t.Fatal("sendQuery expected to succeed, but it failed with", err) + } + // mock real connection to imitate situation when you write but dont get response + conn.sess.buf.transport = &mockReadWriteCloser{ReadWriteCloser: conn.sess.buf.transport} + // canceling context to try to send attention request + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err = stmt.processQueryResponse(ctx) + if err == nil { + t.Error("processQueryResponse expected to fail but it succeeded") + } + // should not fail with ErrBadConn because query was successfully sent to server + if err != ErrorCancelConfirmation { + t.Error("processQueryResponse expected to fail with ErrorCancelConfirmation error but failed with other error: ", err) + } + + if conn.connectionGood { + t.Fatal("Connection should be in a bad state") + } +} + func TestProcessQueryNextErrors(t *testing.T) { tl := testLogger{t: t} defer tl.StopLogging() From 084a6c9524b674cd1c7596a4bdede7fda85aca76 Mon Sep 17 00:00:00 2001 From: Eygin Semen Leonidovich Date: Tue, 15 Nov 2022 12:28:16 +0300 Subject: [PATCH 4/5] return ServerError when failed to get cancelation confirmation --- mssql.go | 2 -- mssql_test.go | 4 ---- queries_test.go | 4 ++-- token.go | 5 +---- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/mssql.go b/mssql.go index 1bcce021..d0361294 100644 --- a/mssql.go +++ b/mssql.go @@ -243,8 +243,6 @@ func (c *Conn) checkBadConn(ctx context.Context, err error, mayRetry bool) error return nil case io.EOF: c.connectionGood = false - case ErrorCancelConfirmation: - c.connectionGood = false case driver.ErrBadConn: // It is an internal programming error if driver.ErrBadConn // is ever passed to this function. driver.ErrBadConn should diff --git a/mssql_test.go b/mssql_test.go index 57beb9ed..9c50b831 100644 --- a/mssql_test.go +++ b/mssql_test.go @@ -127,10 +127,6 @@ func TestCheckBadConn(t *testing.T) { {io.EOF, true, false, newRetryableError(io.EOF), false}, {io.EOF, false, true, io.EOF, false}, {io.EOF, true, true, io.EOF, false}, - {ErrorCancelConfirmation, false, false, ErrorCancelConfirmation, false}, - {ErrorCancelConfirmation, true, false, newRetryableError(ErrorCancelConfirmation), false}, - {ErrorCancelConfirmation, false, true, ErrorCancelConfirmation, false}, - {ErrorCancelConfirmation, true, true, ErrorCancelConfirmation, false}, {netErr, false, false, netErr, false}, {netErr, true, false, newRetryableError(netErr), false}, {netErr, false, true, netErr, false}, diff --git a/queries_test.go b/queries_test.go index 139fc684..7acda02c 100644 --- a/queries_test.go +++ b/queries_test.go @@ -1419,8 +1419,8 @@ func TestProcessQueryCancelConfirmationError(t *testing.T) { t.Error("processQueryResponse expected to fail but it succeeded") } // should not fail with ErrBadConn because query was successfully sent to server - if err != ErrorCancelConfirmation { - t.Error("processQueryResponse expected to fail with ErrorCancelConfirmation error but failed with other error: ", err) + if !errors.As(err, &ServerError{}) { + t.Error("processQueryResponse expected to fail with ServerError error but failed with other error: ", err) } if conn.connectionGood { diff --git a/token.go b/token.go index 1c138257..7d222307 100644 --- a/token.go +++ b/token.go @@ -3,7 +3,6 @@ package mssql import ( "context" "encoding/binary" - "errors" "fmt" "io" "io/ioutil" @@ -99,8 +98,6 @@ const ( // TODO implement more flags ) -var ErrorCancelConfirmation = errors.New("did not get cancellation confirmation from the server") - // interface for all tokens type tokenStruct interface{} @@ -961,7 +958,7 @@ func (t tokenProcessor) nextToken() (tokenStruct, error) { } // we did not get cancellation confirmation, something is not // right, this connection is not usable anymore - return nil, ErrorCancelConfirmation + return nil, ServerError{Error{Message: "did not get cancellation confirmation from the server"}} } } From fbd98649cf0cb8b47786a77fa09cb24a6bdfef56 Mon Sep 17 00:00:00 2001 From: Eygin Semen Leonidovich Date: Tue, 15 Nov 2022 13:33:17 +0300 Subject: [PATCH 5/5] fix ci tests --- queries_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/queries_test.go b/queries_test.go index 7acda02c..fa635be1 100644 --- a/queries_test.go +++ b/queries_test.go @@ -1419,7 +1419,7 @@ func TestProcessQueryCancelConfirmationError(t *testing.T) { t.Error("processQueryResponse expected to fail but it succeeded") } // should not fail with ErrBadConn because query was successfully sent to server - if !errors.As(err, &ServerError{}) { + if _, ok := err.(ServerError); !ok { t.Error("processQueryResponse expected to fail with ServerError error but failed with other error: ", err) }