v5.7.1 - Cache-Invalidation Hooks, Handler Isolation, Transaction Rollback - #38
Open
AdrianCurtin wants to merge 3 commits into
Open
v5.7.1 - Cache-Invalidation Hooks, Handler Isolation, Transaction Rollback#38AdrianCurtin wants to merge 3 commits into
AdrianCurtin wants to merge 3 commits into
Conversation
Capture object state at first mutation inside `Parse::Object.transaction` instead of waiting for `batch.add`, so mutate-then-add flows roll back correctly. The snapshot and restore logic now covers undefined ivars, nested mutable values, relation proxy internals, ACL and dirty-tracking ivars, and newly created transaction objects. Added focused rollback tests for deep mutation isolation, relation operation arrays, preexisting dirty state, failed creates, and transaction context cleanup.
Refactor cache invalidation webhook handlers to call helper methods through an explicit module receiver instead of relying on `self` inside webhook blocks. This prevents `NoMethodError` in `_User`, `_Role`, and `_Session` triggers and avoids aborting later application handlers on the same route. Update cache invalidation tests to invoke handlers through `Parse::Webhooks.invoke_handler`, add coverage for payload-bound dispatch behavior, and add an end-to-end assertion that SDK hooks cannot starve app hooks. Also bump the gem version to 5.7.1 and add the corresponding changelog entry.
Introduce `Parse::Webhooks.abort_after_callbacks_on_error` to control whether a raised `after_*` handler aborts remaining handlers. The default is `true` to preserve existing behavior; when set to `false`, dispatch continues, logs a warning, and emits `parse.webhooks.handler_error` notifications instead of propagating the exception. Refactor composed handler execution into `dispatch_composed`, add error reporting helper logic, and add focused tests covering default abort behavior, isolation behavior, reporting, and unaffected `before_*` semantics. Update CHANGELOG with the new setting and behavior notes.
There was a problem hiding this comment.
Pull request overview
This PR bumps the gem to 5.7.1 and delivers three related reliability fixes: (1) webhook handler execution is made resilient/controllable when an after_* handler raises, (2) cache invalidation webhooks are corrected to not depend on self being the registering module, and (3) failed transactions now correctly roll back in-memory object state (including nested/mutable values and dirty-tracking state).
Changes:
- Add
Parse::Webhooks.abort_after_callbacks_on_errorand route array-dispatch logic to optionally isolate failingafter_*handlers while still reporting errors. - Fix cache-invalidation webhook handlers to call invalidation helpers on an explicit receiver (independent of dispatcher
selfbinding) and strengthen tests to dispatch via the realinvoke_handler. - Rework transaction rollback snapshotting/restoration to capture instance-variable-backed property values (deeply), relation proxy mutation queues, and key dirty-tracking ivars; capture state before first mutation inside the transaction.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/lib/parse/webhook_handler_isolation_test.rb | New unit tests covering abort-vs-isolate semantics for composed after_* webhook handlers. |
| test/lib/parse/transaction_rollback_state_test.rb | Expanded rollback tests for deep-dup snapshots, undefined ivars, relation queues, and transaction-context restoration. |
| test/lib/parse/cache_invalidation_test.rb | Updates tests to dispatch handlers through the real dispatcher binding (invoke_handler) and adds a regression for “SDK handler shouldn’t starve app handler”. |
| lib/parse/webhooks.rb | Introduces configurable composed-dispatch behavior, adds error reporting via warn + notifications, and routes composed triggers through the new dispatcher. |
| lib/parse/stack/version.rb | Version bump to 5.7.1. |
| lib/parse/model/object.rb | Hooks object initialization and ACL mutation into transaction state capture. |
| lib/parse/model/core/properties.rb | Captures transaction state before getter materializes defaults/mutable values. |
| lib/parse/model/core/fetching.rb | Captures transaction state before dirty-tracking marks/replaces values via setters. |
| lib/parse/model/core/actions.rb | Implements fiber-local transaction snapshotting, deep duplication, restoration (including undefined ivars), and revised transaction rollback behavior. |
| lib/parse/model/associations/has_many.rb | Captures transaction state before returning/materializing mutable collection proxies. |
| lib/parse/model/associations/collection_proxy.rb | Captures transaction state when collection proxies are mutated (before dirty notification). |
| lib/parse/model/associations/belongs_to.rb | Captures transaction state after lazy hydration but before callers can mutate returned association objects. |
| lib/parse/cache/invalidation.rb | Refactors webhook handlers to invoke invalidation methods on an explicit receiver (no reliance on bound self). |
| Gemfile.lock | Updates locked local gem version to 5.7.1. |
| CHANGELOG.md | Documents the 5.7.1 fixes and behavioral notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cache-invalidation webhooks stop starving application hooks
A fix release for webhook dispatch and transaction rollback. The SDK's own cache-invalidation triggers raised on every
_User,_Role, and_Sessionwebhook and took every application handler for the same trigger down with them, whether one failingafter_*handler stops the rest is now an explicit setting, and a failedParse::Object.transactionnow actually restores the object it rolled back.Changes
Cache-invalidation webhooks no longer break every application hook for the same trigger
Parse::Cache::InvalidationraisedNoMethodError: undefined method 'guard'on every_User,_Role, and_Sessiontrigger it registered, and the failure was not contained. A webhook handler block is bound to the payload before it runs, so the bareguard/bump_subject/subject_idcalls in the handler bodies resolved againstParse::Webhooks::Payload, which defines none of them.guard's ownrescuenever ran, because it lives inside the body that was never entered, so the error escaped into the route dispatcher. The dispatcher folds a trigger's handlers withArray#map, which abandons the collection on the first raise, and these triggers install duringParse.setupand therefore sit ahead of every application handler for the same trigger. One unresolvable method name silently prevented an application's ownafter_save "_User"hook from running at all, so any work downstream of it stopped without a visible cause. The handlers now call the module on an explicit receiver captured in a local, which is independent of whateverselfthe dispatcher binds. Applications that setcache_invalidation_hooks: falseto work around this can remove it.Proc#call, which leavesselfbound to the module the block closed over, so they passed against handlers that could not run in production. They now dispatch through the real handler invocation and fail against the broken code.Whether one failing
after_*handler stops the rest is now a settingParse::Webhooks.abort_after_callbacks_on_errordecides whether an exception raised by oneafter_*handler prevents the remaining handlers for that trigger from running. It defaults totrue, which is the existing behavior, so nothing changes on upgrade. Set it tofalseto isolate handlers from each other: each one runs regardless of what an earlier one raised, and the failure is reported with a warning and aparse.webhooks.handler_errornotification instead of being swallowed silently. This was previously not a decision at all but a side effect of folding handlers withArray#map, which abandons the collection on the first raise. Registration order is not fully under an application's control, since the SDK's own cache-invalidation triggers install duringParse.setupand therefore sit ahead of handlers registered by application files loaded later, so a single raising handler could silently prevent an application's own hooks from running with nothing logged to say they had been skipped.Failed transactions restore the object they rolled back
Parse::Object.transactionleft the in-memory object holding its modified values. The rollback snapshottedParse::Object#attributesand restored it, but that method returns a schema map of field name to type symbol rather than values, so nothing was ever restored. Property values live in@<field>instance variables; those are now what the rollback captures and restores, including values nested inside arrays and hashes, relation operation queues, and properties whose ivar did not exist before the transaction. State is captured when the object first enters the transaction rather than atbatch.add, so the documented pattern of mutating an object and adding it afterwards rolls back correctly.@attributes, which is the instance variableActiveModel::Dirtykeys its behavior on: once defined it builds anAttributeMutationTrackerover that hash instead of theForcedMutationTrackeraParse::Objectneeds, andclear_changes!then calledforgetting_assignmenton the[key, value]pairsHash#mapyields. Both call sites rescue and warn, so every rollback quietly downgraded the object rather than failing. The rollback no longer defines@attributes.A silently skipped integration test actually runs again
ANALYTICS_DATABASE_URI, a production variable name the test stack never sets, so both of its traversal assertions skipped on every run while the per-file reporter still reported the file as passing. It now readsPARSE_TEST_MONGO_URIlike every other mongo-direct integration file and still honorsANALYTICS_DATABASE_URIas an override.Behavior Notes
abort_after_callbacks_on_errorgoverns only whether later handlers still run. Neither mode reverts anything: anafter_*trigger fires once the write has already committed, so there is no version of this setting that can undo a save.before_*dispatch is untouched. Only the accumulating, non-rejectable triggers (after_save,after_delete,after_logout) can hold more than one handler in the first place, and a rejectablebefore_*trigger must deny if any handler denies, so its raise must continue to abort.Code Example