diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index 0c3026320..35e44cac7 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/boundary/internal/oplog/store" "github.com/hashicorp/go-dbw" wrapping "github.com/hashicorp/go-kms-wrapping/v2" + "github.com/hashicorp/go-multierror" ) const ( @@ -438,11 +439,16 @@ func (rw *Db) DoTx(ctx context.Context, retries uint, backOff Backoff, handler T return info, errors.Wrap(ctx, err, op, errors.WithoutEvent()) } - if err := beginTx.Commit(ctx); err != nil { + var txnErr *multierror.Error + if commitErr := beginTx.Commit(ctx); commitErr != nil { + txnErr = multierror.Append(txnErr, errors.Wrap(ctx, commitErr, op, errors.WithMsg("commit error"))) + // unsure if rolling back is required or possible, but including + // this attempt to rollback on a commit error just in case it's + // possible. if err := beginTx.Rollback(ctx); err != nil { - return info, errors.Wrap(ctx, err, op) + return info, multierror.Append(txnErr, errors.Wrap(ctx, err, op, errors.WithMsg("rollback error"))) } - return info, errors.Wrap(ctx, err, op) + return info, txnErr } return info, nil // it all worked!!! } diff --git a/internal/db/read_writer_test.go b/internal/db/read_writer_test.go index adcfdb331..f00eb8660 100644 --- a/internal/db/read_writer_test.go +++ b/internal/db/read_writer_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/DATA-DOG/go-sqlmock" "github.com/hashicorp/boundary/internal/db/db_test" "github.com/hashicorp/boundary/internal/db/timestamp" "github.com/hashicorp/boundary/internal/errors" @@ -1323,6 +1324,35 @@ func TestDb_DoTx(t *testing.T) { require.NoError(err) assert.Equal(foundUser.Name, user.Name) }) + t.Run("commit-and-rollback-errors", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + mockConn, mock := TestSetupWithMock(t) + mockConn.Debug(true) + rw := New(mockConn) + user, err := db_test.NewTestUser() + require.NoError(err) + user.Id = 1 + + mock.ExpectBegin() + mock.ExpectExec(`UPDATE`).WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectQuery(`SELECT`).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(1)) + mock.ExpectCommit().WillReturnError(fmt.Errorf("commit-err")) + mock.ExpectRollback().WillReturnError(fmt.Errorf("rollback-err")) + _, err = rw.DoTx(context.Background(), 10, ExpBackoff{}, func(r Reader, w Writer) error { + user.Name = "friendly-name" + rowsUpdated, err := w.Update(context.Background(), user, []string{"Name"}, nil) + if err != nil { + return err + } + if rowsUpdated != 1 { + return fmt.Errorf("error in number of rows updated %d", rowsUpdated) + } + return nil + }) + require.Error(err) + assert.Contains(err.Error(), "commit error") + assert.Contains(err.Error(), "rollback error") + }) } func TestDb_Delete(t *testing.T) {