Skip to content

fix: roll back InsertImage transaction when related clears fail#69

Open
Abhineshhh wants to merge 3 commits into
microsoft:mainfrom
Abhineshhh:fix/insertimage-tx-rollback
Open

fix: roll back InsertImage transaction when related clears fail#69
Abhineshhh wants to merge 3 commits into
microsoft:mainfrom
Abhineshhh:fix/insertimage-tx-rollback

Conversation

@Abhineshhh

Copy link
Copy Markdown
Contributor

Summary

  • Fix InsertImage so failures while clearing related tables always roll back the transaction
  • Root cause: if _, err := tx.Exec(...) shadowed the outer err watched by deferred rollback
  • Use a named result err, assign with =, and guard rollback when tx is nil
  • Add regression test with MaxOpenConns(1) proving rollback + connection reuse after clear failure

Fixes #67

Test plan

  • go test ./pkg/infrastructure/database/ -count=1
  • New test: TestInsertImage_RollbackOnClearFailure
  • Existing insert/upsert tests still pass

@Abhineshhh

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@maniSbindra maniSbindra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the issue and this PR. A minor comment after which we will be good to merge.

Comment thread pkg/infrastructure/database/repository.go Outdated
@maniSbindra

maniSbindra commented Jul 13, 2026

Copy link
Copy Markdown
Member

Thanks @Abhineshhh, Could you enable commit signing and re-push?

The related-table DELETE loop used `if _, err := tx.Exec(...)`, which
shadowed the outer err watched by deferred rollback. On clear failure
the function returned an error but left the transaction open, which can
leak the sole SQLite connection (MaxOpenConns=1).

Use a named result err and assign with `=` so every failure path
triggers rollback.
Defer runs only after Begin succeeds, so tx is always set. Remove the
dead tx != nil guard and the comment that described an unreachable path.
@Abhineshhh Abhineshhh force-pushed the fix/insertimage-tx-rollback branch from 959eec0 to f287a71 Compare July 13, 2026 18:32
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 this pull request may close these issues.

InsertImage may fail to roll back transaction when clearing related tables fails

2 participants