Summary
Hit a real, concerning case where POST batch with "atomic": true reported the whole batch failed and rolled back, but every operation had actually committed to the database.
Repro
Batch of ~17 create/update ops against SiteTree pages, atomic: true. One update op targeted a SYSClass (FoxyStripe ProductPage subclass) record whose ReceiptTitle column was NULL (pre-existing legacy data, unrelated to this batch). That record's class hierarchy has a buggy onBeforeWrite() (dynamic/foxystripe, filed separately — see dynamic/foxystripe#422) that calls trim($this->ReceiptTitle) with no null-guard, which is a PHP 8.1+ deprecation notice. This project's SS_ENVIRONMENT_TYPE=dev error handler renders that deprecation as an inline ERROR [Deprecated]: block mid-response rather than a clean thrown exception.
The batch endpoint returned:
VALIDATION_FAILED: Atomic batch failed at operation N — rolled back.
But on inspecting the actual database afterward, every single operation in the batch — including the one that "failed" and everything after it — had committed: all four create ops, all thirteen update ops including the one that hit the deprecation notice, with correct ParentID/field values throughout. No duplicates, no partial state — just a fully-successful write that the endpoint reported as a total failure and rollback.
Why this matters
This is worse than an ordinary false-positive error: a caller that trusts the "rolled back" response and retries the batch (or a differently-shaped follow-up batch assuming a clean slate) risks double-applying operations, especially any create ops with the same externalId (would 409/upsert-collide) or, in a non-idempotent op, genuinely duplicate data.
Suspected cause
A PHP-level E_DEPRECATED (or similar non-Throwable, non-ApiError diagnostic) emitted mid-request by application code the module doesn't control appears to not participate in whatever mechanism decides to roll back the SQL transaction — the DB transaction this module wraps around the batch doesn't seem to actually roll back when the "failure" is detected via output/response-shape inspection rather than a caught exception unwinding normally. I did not dig into BatchHandler's transaction-wrapping code to confirm the exact mechanism — flagging the externally-observable behavior since it's serious regardless of root cause.
Suggested fix
At minimum, the batch endpoint's transaction wrapper should ensure a genuine SQL ROLLBACK actually happens (or verify one did) before reporting rolledBack: true — and/or the response contract should make it very hard for a client to trust "rolled back" without being able to independently verify. A caller has no way to distinguish "truly rolled back, safe to retry" from "actually committed, response is lying" without an out-of-band DB check, which defeats the whole purpose of the atomic contract.
Happy to share more repro detail/logs if useful — this was caught mid-work on a real project (dynamic/sheboygan-youth-sailing-installer) rather than a synthetic test, so I don't have a minimal isolated repro yet.
Summary
Hit a real, concerning case where
POST batchwith"atomic": truereported the whole batch failed and rolled back, but every operation had actually committed to the database.Repro
Batch of ~17 create/update ops against SiteTree pages,
atomic: true. Oneupdateop targeted aSYSClass(FoxyStripeProductPagesubclass) record whoseReceiptTitlecolumn wasNULL(pre-existing legacy data, unrelated to this batch). That record's class hierarchy has a buggyonBeforeWrite()(dynamic/foxystripe, filed separately — see dynamic/foxystripe#422) that callstrim($this->ReceiptTitle)with no null-guard, which is a PHP 8.1+ deprecation notice. This project'sSS_ENVIRONMENT_TYPE=deverror handler renders that deprecation as an inlineERROR [Deprecated]:block mid-response rather than a clean thrown exception.The batch endpoint returned:
But on inspecting the actual database afterward, every single operation in the batch — including the one that "failed" and everything after it — had committed: all four
createops, all thirteenupdateops including the one that hit the deprecation notice, with correctParentID/field values throughout. No duplicates, no partial state — just a fully-successful write that the endpoint reported as a total failure and rollback.Why this matters
This is worse than an ordinary false-positive error: a caller that trusts the "rolled back" response and retries the batch (or a differently-shaped follow-up batch assuming a clean slate) risks double-applying operations, especially any
createops with the sameexternalId(would 409/upsert-collide) or, in a non-idempotent op, genuinely duplicate data.Suspected cause
A PHP-level
E_DEPRECATED(or similar non-Throwable, non-ApiErrordiagnostic) emitted mid-request by application code the module doesn't control appears to not participate in whatever mechanism decides to roll back the SQL transaction — the DB transaction this module wraps around the batch doesn't seem to actually roll back when the "failure" is detected via output/response-shape inspection rather than a caught exception unwinding normally. I did not dig intoBatchHandler's transaction-wrapping code to confirm the exact mechanism — flagging the externally-observable behavior since it's serious regardless of root cause.Suggested fix
At minimum, the batch endpoint's transaction wrapper should ensure a genuine SQL
ROLLBACKactually happens (or verify one did) before reportingrolledBack: true— and/or the response contract should make it very hard for a client to trust "rolled back" without being able to independently verify. A caller has no way to distinguish "truly rolled back, safe to retry" from "actually committed, response is lying" without an out-of-band DB check, which defeats the whole purpose of the atomic contract.Happy to share more repro detail/logs if useful — this was caught mid-work on a real project (dynamic/sheboygan-youth-sailing-installer) rather than a synthetic test, so I don't have a minimal isolated repro yet.