fix: settle await on first settled condition incl. rejection (#399) - #785
fix: settle await on first settled condition incl. rejection (#399)#785xepozz wants to merge 1 commit into
Conversation
await()/awaitWithTimeout() combined multiple conditions with Promise::any, which unblocks only on the first *fulfilled* condition and ignores rejections until every condition rejects (or the timer fires). A rejected promise condition therefore never woke the await, so awaitWithTimeout(t, rejecting) blocked for the full timeout instead of surfacing the failure. The single-condition path already propagated rejections, so the behavior was inconsistent. Switch the multi-condition path to Promise::race (unblock on the first *settled* condition and propagate its rejection) and cancel the internal awaitWithTimeout timer when a condition rejects. Because this changes the command sequence for in-flight histories, it is gated behind the new FeatureFlags::$settleAwaitOnFirstSettledCondition flag (default false = legacy Promise::any behavior), mirroring how sdk-java gates CANCEL_AWAIT_TIMER_ON_CONDITION behind an SdkFlag. Also guard the degenerate empty-conditions await so it fails fast with LengthException instead of hanging, and document the Promise::any() escape hatch on Workflow::await(). Tests cover both flag states, the empty-await guard, and the flag-independent baseline (closures, fulfilled promise, single rejection).
![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: dadfc77067
ℹ️ 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".
| return Promise::race($result)->then( | ||
| $onResolved, | ||
| function (\Throwable $reason) use ($conditionGroupId): never { | ||
| $this->rejectConditionGroup($conditionGroupId); |
There was a problem hiding this comment.
When the feature flag is enabled and a rejected promise wins a race that also contains a closure or Mutex, this only removes the condition group from $awaits; it does not reject the Deferred objects created by ScopeContext::addCondition(). Those deferreds were registered with Scope::onAwait(), whose cancellation callback retains each deferred until it settles, but after the group is removed its callable can never settle it. A long-running workflow that catches and repeatedly retries these rejections therefore accumulates pending callbacks and promise chains until the scope ends; settle the group's deferreds before discarding it.
Useful? React with 👍 / 👎.
1 participant
What was changed
Multi-condition
await/awaitWithTimeoutnow unblocks on the first settled condition andpropagates a rejected promise (
Promise::any→Promise::race), gated behindFeatureFlags::$settleAwaitOnFirstSettledCondition(default off = old behaviour).Why?
A rejected awaited promise was swallowed until the timeout. Flag-gated for replay safety,
like sdk-java's
CANCEL_AWAIT_TIMER_ON_CONDITION.Checklist
Closes [Not a Bug] await doesn't interrupts on the activity with error #399
How was this tested:
Unit tests for both flag states + baseline. Unit suite green.
No.