Skip to content

fix: make sure after transaction is called#397

Closed
barnabasJ wants to merge 4 commits intomainfrom
fix/make-sure-after-transaction-is-called
Closed

fix: make sure after transaction is called#397
barnabasJ wants to merge 4 commits intomainfrom
fix/make-sure-after-transaction-is-called

Conversation

@barnabasJ
Copy link
Copy Markdown
Contributor

@barnabasJ barnabasJ commented Jan 22, 2026

Fixes after_transaction callbacks not being invoked when errors occur during GraphQL mutations.

Background

Ash recently added support for after_transaction hooks in the atomic path for bulk operations. However, when
rollback_on_error? is true (the default) and an error occurs, Ash.DataLayer.rollback throws to exit the transaction
immediately. This unwinds the call stack and bypasses process_results where after_transaction hooks would normally
be executed—the changeset context needed to run the hooks is effectively abandoned.

Since GraphQL mutations use bulk operations with the atomic strategy, after_transaction callbacks weren't being
called when errors occurred (e.g., database constraint violations).

Solution

Add rollback_on_error?: false to bulk_update and bulk_destroy calls in the resolver. This allows the transaction to
complete normally (with errors collected) so that process_results runs and after_transaction callbacks are invoked
with the error result.

Note: Setting rollback_on_error?: false is safe here because ash_graphql mutations only operate on a single record
at a time—there's no risk of partial updates across multiple records.

The last commit should also fix the compile warnings in ci

Contributor checklist

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@barnabasJ barnabasJ force-pushed the fix/make-sure-after-transaction-is-called branch from 944eaa1 to a5e99ab Compare January 22, 2026 14:10
Add rollback_on_error?: false to bulk_update and bulk_destroy calls.
This ensures after_transaction callbacks are invoked even when errors
occur (e.g., database constraint violations).

Previously, errors caused immediate rollback that skipped after_transaction.
Add test resources (AfterTransactionEts, AfterTransactionMnesia) and tests
verifying after_transaction callbacks run on both success and error cases
for create, update, and destroy mutations.

Mnesia tests use real transactions to verify the fix works with actual
transaction semantics.
@barnabasJ barnabasJ force-pushed the fix/make-sure-after-transaction-is-called branch from 6b761b7 to 3a3fbf6 Compare January 22, 2026 14:17
@barnabasJ barnabasJ self-assigned this Jan 22, 2026
@barnabasJ barnabasJ force-pushed the fix/make-sure-after-transaction-is-called branch from 3a3fbf6 to 05002dd Compare January 22, 2026 14:47
@barnabasJ barnabasJ marked this pull request as ready for review January 22, 2026 14:57
@barnabasJ barnabasJ requested a review from zachdaniel January 22, 2026 14:57
@barnabasJ
Copy link
Copy Markdown
Contributor Author

This has another issue

@barnabasJ barnabasJ closed this Jan 22, 2026
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.

1 participant