Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transaction canceled context may result in ErrInvalidConn instead of context.Canceled error #1690

Open
brad-defined opened this issue Apr 1, 2025 · 0 comments · May be fixed by #1691
Open

Comments

@brad-defined
Copy link

When executing a context-aware operation during a transaction, different error types are returned depending on when the context is canceled.

In the following example:

               tx, err := db.Begin()
               if err != nil {
                      fmt.Printf("Failed to obtain db connection: %v", err)
                      return
               }
               returnedErrors := make(chan error, 1)
               go func() {
                       _, err := tx.ExecContext(ctx, `INSERT INTO foo VALUES (3, "three")`)
                       returnedErrors <- err
               }()
               cancel()

               err = <-returnedErrors
               switch {
               case errors.Is(err, ErrInvalidConn):
                       fmt.Printf("The context was canceled, but ExecContext got error %v", err)
               case errors.Is(err, context.Canceled):
                       // cool
               case err != nil:
                       fmt.Printf("Got unexpected error %v", err)
               }

               err = tx.Rollback()
               switch {
               case errors.Is(err, ErrInvalidConn):
                      fmt.Printf("The context was canceled, but ROLLBACK got error %v", err)
               case errors.Is(err, context.Canceled):
                       // cool
               case err != nil:
                       fmt.Printf("Got unexpected error %v", err)
               }

The context may be canceled (1) before or during the ExecContext operation, (2) after the ExecContext operation returns, or (3) after the ExecContext operation succeeds but before the context is removed from the watcher.

(1) If the context is canceled before or during ExecContext, ExecContext returns the error as cached by the context watcher, returning context.Canceled.
(2) If the context is canceled after the ExecContext, no errors are returned, everything succeeds.
(3) !bug! If the context is canceled after the ExecContext operation succeeds but before the context is removed from the watcher, then tx.Rollback() or tx.Commit() return ErrInvalidConn.

I argue that the behavior in (3) should be consistent with the behavior in (1) - the error returned by tx.Rollback() or tx.Commit() should be context.Canceled. The connection being closed is a side effect of the root cause that the context passed to ExecContext has been canceled. (This is also a simpler fix than attempting to remove the race of a context being canceled after ExecContext's operation is successfully executed but before the context is removed from the watcher.)

In our application, we trigger alerts on unexpected errors like ErrInvalidConn, but log and do not alert on expected errors like context.Canceled. The Commit/Rollback behavior ends up causing alerts in our application.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant