Skip to content

Fix(PSRE-4513): Address Rails deprecation: use of exit statements in transaction - #2

Open
aamirsahmad wants to merge 1 commit into
masterfrom
aamir/PSRE-4513
Open

Fix(PSRE-4513): Address Rails deprecation: use of exit statements in transaction#2
aamirsahmad wants to merge 1 commit into
masterfrom
aamir/PSRE-4513

Conversation

@aamirsahmad

@aamirsahmadaamirsahmad commented May 18, 2026

Copy link
Copy Markdown
Collaborator

https://stackadapt.atlassian.net/browse/PSRE-4513

sa-web integration: https://github.com/StackAdapt/stackadapt-web/pull/39398

Summary

#halt and #succeed previously used throw :abort to short-circuit out of #execute. When called from inside an ActiveRecord::Base.transaction block, that throw crosses the transaction boundary, which Rails has deprecated:

DEPRECATION WARNING: Using `return`, `break` or `throw` to exit a transaction
block is deprecated without replacement. … This results in the transaction
being committed, but in the next release of Rails it will rollback.

This change replaces the throw :abort mechanism with a raised exception only when halt/succeed are called from inside #execute — the only context where a user transaction could be on the stack. From inside before/after callbacks, the existing throw :abort is preserved so ActiveSupport's callback-chain halting (and the "after callbacks still run after halt-in-before" semantics) is unchanged.

What changed

  • Added two control-flow exception classes (ActiveOperation::Halted, ActiveOperation::Succeeded). They inherit from Exception rather than StandardError so consumer code's bare rescue does not swallow them.
  • Base#perform tracks @in_execute and rescues the new exceptions around the execute call.
  • Base#halt and Base#succeed raise the appropriate exception when @in_execute is true, otherwise fall back to throw :abort.

Behavioral impact on consumers

  • halt inside a transaction: now causes a rollback (previously committed with a deprecation warning). This matches the documented intent of halt (failure path).
  • succeed inside a transaction: now causes a rollback. This is a behavior change for any caller that relied on succeed-inside-transaction implicitly committing. There is no gem-only fix — Rails' rule is "the only way to commit is to fall off the end of the block." Call sites that need a commit must call succeed after the transaction block. See the companion app PR for the affected sites.
  • Operations that had a halted callback compensating for the throw-commits bug (e.g. by destroying records that were "saved" before the throw) may need that callback removed — rollback now handles the cleanup automatically. See the companion app PR for one example.
  • Inside before/after callbacks: no change. throw :abort continues to drive AS's callback halting.

Thread safety

Profile is unchanged from the pre-patch gem:

  • The new @in_execute is a plain instance variable on instance methods, scoped to a single operation instance. The gem's idiom is one instance per call, matching the existing pattern (self.state = …, @output = … were already mutated during #execute).
  • The new Halted / Succeeded classes are immutable class definitions, evaluated once at load.
  • No new class-level mutable state (no class variables, no class instance variables, no Thread.current).
  • Ruby exceptions propagate per-thread, so a raise Halted in one thread cannot be caught by another's rescue.
  • rubocop --only ThreadSafety on the patched gem reports zero new offenses (the 6 pre-existing offenses on @inputs/@operations class instance variables in the DSL are untouched by this patch).

Tests

All 112 existing gem specs pass unchanged — the existing control_flow_spec.rb covers halt from before callbacks, in-execute halts, and the after-callback flow, all of which exercise both code paths.

Test plan

  • Run gem specs (bundle exec rspec) — 112 examples, 0 failures
  • rubocop --only ThreadSafety — no new offenses introduced
  • Consumer app passes its specs against this revision (companion PR)

@aamirsahmad
aamirsahmad requested review from a team and kthomas30 and removed request for a teamMay 18, 2026 18:08
@aamirsahmad
aamirsahmad marked this pull request as ready for review May 20, 2026 20:23
@aamirsahmadaamirsahmad added the rails-7-0 https://stackadapt.atlassian.net/browse/PSRE-4093 label Aug 10, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rails-7-0https://stackadapt.atlassian.net/browse/PSRE-4093

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@aamirsahmad@paul-ljh@kthomas30