fix: report every failure from parallel saga compensation - #786
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6aa65d8821
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (\Throwable $e) { | ||
| if ($sagaException === null) { | ||
| $sagaException = new CompensationException($e->getMessage(), (int) $e->getCode(), $e); |
There was a problem hiding this comment.
When any parallel compensation throws a PHP Error (for example, a TypeError caused by a deployment bug), this catch replaces it with CompensationException. Previously Promise::all surfaced the raw Error; the default ExceptionInterceptor classifies Error as retryable and Process::complete() panics the workflow task so it can retry, whereas the wrapper is treated as a normal exception and permanently fails the workflow execution. Await all scopes, but preserve the retryable/fatal classification when propagating their failures.
Useful? React with 👍 / 👎.
Parallel Saga::compensate() used Promise::all, which short-circuits on the first rejected compensation, so sibling failures were lost and the caller received only the first (raw) exception. Await every compensation scope and aggregate all failures into a CompensationException (first as cause, rest as suppressed), matching the sdk-java Saga semantics. The success return value stays null and the sequential branch is unchanged. Catch \Exception (not \Throwable) so a \Error thrown by a compensation (e.g. a TypeError from a deployment bug) still propagates raw. The default ExceptionInterceptor classifies \Error as retryable, so wrapping it in CompensationException would turn a retryable task failure into a permanent workflow failure. This matches sdk-java, which catches Exception (not Error). Tests (unit, mock harness): - parallel happy path runs every compensation (ports sdk-java testSagaParallelCompensation) - sequential runs compensations in reverse order (ports sdk-java testSaga) - sequential stops at first failure and throws the raw exception - compensate() return value stays null on success - parallel compensation reports every failure via CompensationException - parallel compensation propagates a raw \Error for retry
6aa65d8 to
440e257
Compare
1 participant
What was changed
Parallel
Saga::compensate()now awaits all compensations and aggregates every failure into aCompensationException, instead of short-circuiting on the first (Promise::all).Why?
Restores the documented "all fired, all errors returned" contract; matches sdk-java.
Checklist
Closes: n/a
How was this tested:
Unit tests (2 ported from sdk-java
SagaTest+ failure/return coverage). Unit suite green.No.