fix(runtime): adopt returned promise state on async tail-return (#4828)#4832
Merged
Conversation
An async fn whose tail expression returns an un-awaited promise (`return someAsyncCall()`) corrupted the resolved value: awaiting the outer fn yielded the inner Promise object itself (typeof "object", JSON.stringify "", every property undefined) instead of the inner promise's eventual value. Root cause: js_async_step_done's in-place reuse fast path settled the result promise (INLINE_TRAP.trap_next) with a raw js_promise_resolve, skipping the ECMAScript thenable/promise adoption that the slow path (js_promise_resolved) performs. The sibling js_async_step_chain (the `await x` path) already adopts, which is why explicit `await` masked it. Fix: resolve_trap_next_with_adoption mirrors js_promise_resolved's adoption probes (primitive short-circuit, native-Promise chain via js_promise_resolve_with_promise, thenable assimilation) but settles the existing trap_next so the runner's self-chain fast path still fires.
31cc634 to
9006a3b
Compare
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.
Fixes #4828.
Problem
An
asyncfunction whose tail expression returns an un-awaited promise (return someAsyncCall()) corrupted the resolved value. Awaiting the outer fn produced the inner Promise object itself —typeof === "object",JSON.stringify === "", every propertyundefined— instead of the inner promise's eventual value. Adding an explicitawait(const r = await someAsyncCall(); return r;) worked. This is a very common JS pattern, so impact is broad (acreateCustomerflow returned""to its caller, breaking redirects).Root cause
js_async_step_done(crates/perry-runtime/src/promise/async_step.rs) settles the async fn's result promise two ways. The slow path usesjs_promise_resolved(value), which performs ECMAScript thenable/promise adoption (native-Promise short-circuit #2823, thenable assimilation #586). The in-place reuse fast path — taken in steady state when the microtask runner has stashed the result promise inINLINE_TRAP.trap_next— bypassed adoption with the low-leveljs_promise_resolve(trap_next, value), storing the returned Promise object as the literal resolution value. The siblingjs_async_step_chain(theawait xpath) already adopts, which is why explicitawaitmasked the bug.Fix
Added
resolve_trap_next_with_adoption, mirroringjs_promise_resolved's adoption probes (primitive short-circuit, native-Promise chain viajs_promise_resolve_with_promise, thenable assimilation) but settling the existingtrap_nextrather than allocating a fresh promise — so the runner's self-chain fast path still fires. The fast path now calls it instead of the rawjs_promise_resolve.Verification
Compiled and ran the issue's repro (
test-files/test_issue_4828_async_tail_promise.ts). Before:broken: ... json="" .id=undefined. After, both lines identical and correct: