From 8571a1a0bff95c3562719a565392c0e1a1003807 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Wed, 12 Aug 2026 14:10:05 -0500 Subject: [PATCH 1/6] feat: construct RpcPromise from a Promise Resolves the long-standing TODO on the RpcPromise constructor: the application may now pass a Promise (or any other thenable) for the eventual resolution. Calls made before the promise settles are queued and delivered in order once it does, so an RpcPromise can stand in for a capability that doesn't exist yet -- for example, one that will only become available after a broken session has been re-established. The promise may resolve to an RpcTarget, a stub, or a plain value. Promise.resolve() performs thenable assimilation natively, so no hand-rolled hardening against misbehaving thenables is needed. The resolution is adopted with return semantics (the same representation used for resolutions of local async calls), so awaiting delivers the value, pipelined calls forward through it without forcing a pull, and brokenness of a stub resolution is preserved. Passing an existing RpcPromise adopts its hook directly, keeping it lazy. A rejection is adopted as an ErrorStubHook rather than left to reject the backing promise, so the promise chains behind queued calls never reject: calls land on the ErrorStubHook (which disposes their arguments) and the error surfaces only through pull() or onBroken(). Without this, a discarded pipelined call on a promise-backed stub would raise an unhandled rejection event when the promise rejects (crashing Node under its default handling), even though fire-and-forget calls on the session-backed stub it stands in for reject only on pull. --- .changeset/rpc-promise-from-promise.md | 5 + README.md | 19 +++ __tests__/index.test.ts | 163 ++++++++++++++++++++++++- __type-tests__/rpc-base-cases.test.ts | 26 ++++ src/core.ts | 50 +++++++- src/index.ts | 13 +- 6 files changed, 270 insertions(+), 6 deletions(-) create mode 100644 .changeset/rpc-promise-from-promise.md diff --git a/.changeset/rpc-promise-from-promise.md b/.changeset/rpc-promise-from-promise.md new file mode 100644 index 00000000..b3433c28 --- /dev/null +++ b/.changeset/rpc-promise-from-promise.md @@ -0,0 +1,5 @@ +--- +"capnweb": minor +--- + +`RpcPromise` can now be constructed by the application from a `Promise` (or any other thenable) for the eventual resolution. Calls pipeline immediately and are queued, in order, until the promise settles, making it possible to publish a capability that doesn't exist yet, e.g. while re-establishing a broken session. diff --git a/README.md b/README.md index f73d0170..011b7905 100644 --- a/README.md +++ b/README.md @@ -267,6 +267,25 @@ let profile = await api.getUserProfile(user.id); Whenever an `RpcPromise` is passed in the parameters to an RPC, or returned as part of the result, the promise will be replaced with its resolution before delivery to the receiving application. So, you can use an `RpcPromise` anywhere where a `T` is required! +#### Constructing `RpcPromise` from a `Promise` + +You can construct an `RpcPromise` yourself from a regular `Promise` (or any other thenable), using `new RpcPromise(promise)`. The result supports pipelining immediately: calls made before the promise settles are queued and delivered, in order, once it does, while awaiting it yields the promise's resolution. The promise may resolve to an `RpcTarget`, a stub, or a plain value; if it rejects, queued calls and `await`s fail with the rejection error, and `onRpcBroken()` callbacks are invoked. + +This is useful when the application knows a capability will exist but doesn't have it yet. For example, while re-establishing a broken session, you can publish a promise-backed stand-in, so that interim calls queue up and flow to the new connection once it is ready: + +```ts +// reconnect() returns Promise>. +let promise = new RpcPromise(reconnect()); + +// Calls pipeline immediately, and are delivered once reconnect() resolves. +let result = await promise.doSomething(); +``` + +Two things to watch out for: + +* Ownership of the resolution transfers to the `RpcPromise`: disposing it disposes the target (or stub) that the promise resolved to. If you also want to keep the stub you resolved the promise with, resolve it with a `.dup()`. +* Calls made while the promise is pending queue unboundedly, holding copies of their arguments. If the awaited capability may never arrive -- e.g. reconnection fails permanently -- reject the promise, so that queued calls fail rather than accumulate. + ### The magic `map()` method Every RPC promise has a special method `.map()` which can be used to remotely transform a value, without pulling it back locally. Here's an example: diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 91685231..2b592d6d 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -4,7 +4,7 @@ import { expect, it, describe, inject } from "vitest" import { deserialize, serialize, RpcSession, type RpcSessionOptions, RpcTransport, - type RpcTransportWithCustomEncoding, RpcTarget, RpcStub, newWebSocketRpcSession, + type RpcTransportWithCustomEncoding, RpcTarget, RpcStub, RpcPromise, newWebSocketRpcSession, newMessagePortRpcSession, newHttpBatchRpcSession} from "../src/index.js" import { swapByteOrder } from "../src/serialize.js" @@ -549,6 +549,7 @@ class TestTransport implements RpcTransport { private waiter?: () => void; private aborter?: (err: any) => void; public log = false; + public sentLog: string[] = []; private fenced = false; send(message: string): void { @@ -557,6 +558,7 @@ class TestTransport implements RpcTransport { message = message.replaceAll("$remove$", ""); if (this.log) console.log(`${this.name}: ${message}`); + this.sentLog.push(message); this.partner!.queue.push(message); if (this.partner!.waiter && !this.partner!.fenced) { this.partner!.waiter(); @@ -2378,6 +2380,165 @@ describe("RpcImportHook argument disposal", () => { // ======================================================================================= +describe("constructing RpcPromise from a promise", () => { + it("pipelines through a pending promise without pulling the resolution", async () => { + await using harness = new TestHarness(new TestTarget()); + + let {promise, resolve} = Promise.withResolvers>(); + using stub = new RpcPromise(promise); + + using counter = stub.makeCounter(1); + let result = counter.increment(2); + resolve(harness.stub.dup()); + expect(await result).toBe(3); + + // Only the final result was pulled: neither the promise's resolution nor the intermediate + // counter was transmitted. + let sent = harness.clientTransport.sentLog; + expect(sent.some(msg => msg.startsWith('["push"'))).toBe(true); + expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); + }); + + it("queues calls made before resolution and delivers them in order", async () => { + let calls: number[] = []; + class Recorder extends RpcTarget { + record(i: number) { calls.push(i); return i; } + } + + let {promise, resolve} = Promise.withResolvers(); + using stub = new RpcPromise(promise); + + let results = [stub.record(1), stub.record(2), stub.record(3)]; + expect(calls).toStrictEqual([]); + + resolve(new Recorder()); + expect(await Promise.all(results)).toStrictEqual([1, 2, 3]); + expect(calls).toStrictEqual([1, 2, 3]); + }); + + it("accepts a promise for a target, a remote stub, or a plain value", async () => { + await using harness = new TestHarness(new TestTarget()); + + using target = new RpcPromise(Promise.resolve(new Counter(1))); + expect(await target.increment()).toBe(2); + + using remote = new RpcPromise(Promise.resolve(harness.stub.dup())); + expect(await remote.square(3)).toBe(9); + + using value = new RpcPromise<{foo: number}>(Promise.resolve({foo: 123})); + expect(await value.foo).toBe(123); + }); + + it("awaiting the RpcPromise yields the resolution", async () => { + using plain = new RpcPromise<{foo: number}>(Promise.resolve({foo: 123})); + expect(await plain).toStrictEqual({foo: 123}); + + await using harness = new TestHarness(new TestTarget()); + using remote = new RpcPromise(Promise.resolve(harness.stub.dup())); + let resolved = await remote; + expect(await resolved.square(4)).toBe(16); + }); + + it("reports rejection to queued calls, await, and onRpcBroken", async () => { + let error = new Error("nope"); + using stub = new RpcPromise(Promise.reject(error)); + + let broken: any[] = []; + stub.onRpcBroken(err => { broken.push(err); }); + + await expect(() => stub.increment()).rejects.toThrow("nope"); + await expect(Promise.resolve(stub)).rejects.toThrow("nope"); + expect(broken).toStrictEqual([error]); + }); + + it("does not report an unhandled rejection for an unused promise", async () => { + new RpcPromise(Promise.reject(new Error("ignored"))); + await pumpMicrotasks(); + }); + + it("does not report an unhandled rejection for a discarded queued call", async () => { + using stub = new RpcPromise(Promise.reject(new Error("ignored"))); + stub.increment(); // result intentionally discarded + await pumpMicrotasks(); + }); + + it("does not report an unhandled rejection for a discarded map() result", async () => { + using stub = new RpcPromise(Promise.reject(new Error("ignored"))); + stub.map(i => i); // result intentionally discarded + await pumpMicrotasks(); + }); + + it("disposes the eventual target when disposed before resolution", async () => { + let disposed = false; + class Disposable extends RpcTarget { + [Symbol.dispose]() { disposed = true; } + } + + let {promise, resolve} = Promise.withResolvers(); + let stub = new RpcPromise(promise); + stub[Symbol.dispose](); + + resolve(new Disposable()); + await pumpMicrotasks(); + expect(disposed).toBe(true); + }); + + it("delivers a call initiated before disposal", async () => { + let disposed = false; + class DisposableCounter extends Counter { + [Symbol.dispose]() { disposed = true; } + } + + let stub = new RpcPromise(Promise.resolve(new DisposableCounter(1))); + await pumpMicrotasks(); + + let result = stub.increment(2); + stub[Symbol.dispose](); + + expect(disposed).toBe(false); + expect(await result).toBe(3); + expect(disposed).toBe(true); + }); + + it("keeps an adopted RpcPromise lazy", async () => { + await using harness = new TestHarness(new TestTarget()); + + using counter = new RpcPromise(harness.stub.makeCounter(1)); + expect(await counter.increment(2)).toBe(3); + + let sent = harness.clientTransport.sentLog; + expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); + }); + + it("preserves brokenness of an adopted stub", async () => { + await using harness = new TestHarness(new TestTarget()); + using stub = new RpcPromise(harness.stub.dup()); + + let errors: any[] = []; + stub.onRpcBroken(error => { errors.push(error); }); + + harness.clientTransport.forceReceiveError(new Error("test disconnect")); + await pumpMicrotasks(); + expect(errors).toStrictEqual([new Error("test disconnect")]); + }); + + it("keeps adopted stub disposal idempotent", () => { + let disposals = 0; + class Disposable extends RpcTarget { + [Symbol.dispose]() { ++disposals; } + } + + let inner = new RpcStub(new Disposable()); + let outer = new RpcPromise(inner); + inner[Symbol.dispose](); + outer[Symbol.dispose](); + + expect(disposals).toBe(1); + }); +}); + +// ======================================================================================= + describe("HTTP requests", () => { it("can perform a batch HTTP request", async () => { let cap = newHttpBatchRpcSession(`http://${inject("testServerHost")}`); diff --git a/__type-tests__/rpc-base-cases.test.ts b/__type-tests__/rpc-base-cases.test.ts index 1850dd33..262ec60c 100644 --- a/__type-tests__/rpc-base-cases.test.ts +++ b/__type-tests__/rpc-base-cases.test.ts @@ -150,3 +150,29 @@ api.invoke((name: string, attempt: number) => { // @ts-expect-error headers argument must be Headers api.roundTripHeaders(new Map([["x-id", "1"]])) + +// An RpcPromise can be constructed from a promise for a plain value, an RpcTarget, or a stub, +// keeping the resolution's type in each case. +expectType>(new RpcPromise(Promise.resolve(42))) +expectType>(new RpcPromise(Promise.resolve(new PointTarget()))) +expectType>(new RpcPromise(Promise.reject(new Error("x")))) + +// The target type is inferred exactly from a promise for a stub -- no explicit type argument. +const promisedFromStub = new RpcPromise(Promise.resolve(pointStub)) +type _PromisedFromStubInfersTarget = Expect>> + +async function assertAwaitedConstructedPromiseShapes() { + const target = await new RpcPromise(Promise.resolve(new PointTarget())) + expectType>(target) + + const value = await new RpcPromise(Promise.resolve(42)) + expectType(value) +} + +void assertAwaitedConstructedPromiseShapes + +// @ts-expect-error a non-thenable value cannot back an RpcPromise +void new RpcPromise(42) + +// @ts-expect-error a bare stub cannot back an RpcPromise; pass a promise for the stub instead +void new RpcPromise(pointStub) diff --git a/src/core.ts b/src/core.ts index d3c81a46..7453efa6 100644 --- a/src/core.ts +++ b/src/core.ts @@ -561,9 +561,18 @@ export class RpcStub extends RpcTarget { } export class RpcPromise extends RpcStub { - // TODO: Support passing target value or promise to constructor. - constructor(hook: StubHook, pathIfPromise: PropertyPath) { - super(hook, pathIfPromise); + // Internally, an `RpcPromise` is constructed from a `StubHook` plus a property path. The + // application may instead pass a promise (or any other thenable) for the eventual resolution; + // calls made before it settles are queued and delivered, in order, once it does. + constructor(hook: StubHook | PromiseLike, pathIfPromise?: PropertyPath) { + if (hook instanceof StubHook) { + super(hook, pathIfPromise!); + } else { + if (pathIfPromise !== undefined) { + throw new TypeError("RpcPromise constructor expected one argument, received two."); + } + super(hookForPromiseArg(hook), []); + } } then(onfulfilled?: ((value: unknown) => unknown) | undefined | null, @@ -605,6 +614,41 @@ export function unwrapStubTakingOwnership(stub: RpcStub): StubHook { } } +// Given the application's argument to `new RpcPromise(value)`, produce the hook backing the +// promise. +function hookForPromiseArg(value: PromiseLike): StubHook { + let type = typeForRpc(value); + if (type === "stub" || type === "rpc-promise") { + // Adopt an existing stub or promise directly, transferring ownership of its hook. In + // particular, this keeps an adopted `RpcPromise` lazy -- assimilating it as a thenable would + // instead force its resolution to be pulled -- and preserves hook-local behavior such as + // brokenness. + return unwrapStubTakingOwnership(value); + } + + // `Promise.resolve()` natively handles the hazards of assimilating an arbitrary thenable + // (`then` getters with side effects, self-resolution, cross-realm thenables), so + // `hookForResolution()` only ever sees settled, non-thenable values. + // + // A rejection is adopted as an ErrorStubHook rather than left to reject the backing promise. + // This way the promise chains backing queued calls never reject -- the calls land on the + // ErrorStubHook, which disposes their arguments -- and the error only surfaces through pull(), + // so a discarded pipelined call can't produce an unhandled rejection event. + let hook = new PromiseStubHook( + Promise.resolve(value).then(hookForResolution, err => new ErrorStubHook(err))); + hook.ignoreUnhandledRejections(); + return hook; +} + +// Adopt the resolution of a promise passed to `new RpcPromise()` with "return" semantics, taking +// ownership of any stubs within (including a stub as the root value). This is the same +// representation used for the resolution of a local async call: pull() delivers the value, +// pipelined calls forward through the payload without forcing a pull, and a single-stub payload +// forwards onBroken(), preserving brokenness. +function hookForResolution(value: unknown): StubHook { + return new PayloadStubHook(RpcPayload.fromAppReturn(value)); +} + // Given a stub (still wrapped in a Proxy), extract the underlying `StubHook`, and duplicate it, // returning the duplicate. // diff --git a/src/index.ts b/src/index.ts index 3d80501a..7820e2f9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,7 @@ import { RpcTarget as RpcTargetImpl, RpcStub as RpcStubImpl, RpcPromise as RpcPr import { serialize, deserialize, EncodingLevel } from "./serialize.js"; import { RpcTransport, RpcTransportWithCustomEncoding, AnyRpcTransport, RpcSession as RpcSessionImpl, RpcSessionOptions } from "./rpc.js"; import { RpcLimits, DEFAULT_LIMITS, DEFAULT_MAX_DEPTH } from "./serialize.js"; -import { RpcTargetBranded, RpcCompatible, Stub, type RpcPromise as RpcPromiseType, +import { RpcTargetBranded, RpcCompatible, Stub, Stubable, type RpcPromise as RpcPromiseType, __RPC_TARGET_BRAND } from "./types.js"; import { newWebSocketRpcSession as newWebSocketRpcSessionImpl, newWorkersWebSocketRpcResponse, WebSocketTransport } from "./websocket.js"; @@ -58,10 +58,19 @@ export const RpcStub: { * until you actually `await` the promise (or call `then()`, etc. on it). This is an optimization: * if you only intend to use the promise for pipelining and you never await it, then there's no * need to transmit the resolution! + * + * You may also construct an `RpcPromise` yourself from a regular `Promise` (or any other + * thenable), using `new RpcPromise(promise)`. The promise may resolve to an `RpcTarget`, a stub, + * or a plain value. Calls made before the promise settles are queued and delivered, in order, + * once it does, so an `RpcPromise` can stand in for a capability that doesn't exist yet -- for + * example, one that will only become available after a broken session has been re-established. + * Note that the `RpcPromise` takes ownership of the resolution: disposing it disposes the + * resolution, so resolve the promise with a `dup()` if you also intend to keep the stub. */ export type RpcPromise> = RpcPromiseType; export const RpcPromise: { - // Note: Cannot construct directly! + new (value: PromiseLike>): RpcPromise; + new >(value: PromiseLike>): RpcPromise; } = RpcPromiseImpl; /** From 97026b6a09b32e9e645e710da6d86a354274af97 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Wed, 12 Aug 2026 18:26:35 -0500 Subject: [PATCH 2/6] fix: address review feedback on RpcPromise promise construction - Only adopt the hook of an existing RpcPromise; a bare stub's hook may not implement pull(), so bare stubs now take the generic path, whose resolution payload handles them correctly (await previously rejected with "Tried to resolve a non-promise stub."). Regression test added. - Inline hookForPromiseArg and hookForResolution into the constructor. - Collapse the constructor's type overloads into a single signature, narrowing the accepted type to Promise (runtime still assimilates arbitrary thenables). - Reframe the README section around the local-loopback RPC equivalence, and align the jsdoc and changeset with it. --- .changeset/rpc-promise-from-promise.md | 2 +- README.md | 23 +++++---- __tests__/index.test.ts | 16 +++++- src/core.ts | 71 ++++++++++++-------------- src/index.ts | 20 ++++---- 5 files changed, 71 insertions(+), 61 deletions(-) diff --git a/.changeset/rpc-promise-from-promise.md b/.changeset/rpc-promise-from-promise.md index b3433c28..0428ba5d 100644 --- a/.changeset/rpc-promise-from-promise.md +++ b/.changeset/rpc-promise-from-promise.md @@ -2,4 +2,4 @@ "capnweb": minor --- -`RpcPromise` can now be constructed by the application from a `Promise` (or any other thenable) for the eventual resolution. Calls pipeline immediately and are queued, in order, until the promise settles, making it possible to publish a capability that doesn't exist yet, e.g. while re-establishing a broken session. +`RpcPromise` can now be constructed from a `Promise`: pipelined calls queue in order until it settles, so you can publish a capability that doesn't exist yet. diff --git a/README.md b/README.md index 011b7905..a1dc73e5 100644 --- a/README.md +++ b/README.md @@ -269,22 +269,25 @@ Whenever an `RpcPromise` is passed in the parameters to an RPC, or returned as p #### Constructing `RpcPromise` from a `Promise` -You can construct an `RpcPromise` yourself from a regular `Promise` (or any other thenable), using `new RpcPromise(promise)`. The result supports pipelining immediately: calls made before the promise settles are queued and delivered, in order, once it does, while awaiting it yields the promise's resolution. The promise may resolve to an `RpcTarget`, a stub, or a plain value; if it rejects, queued calls and `await`s fail with the rejection error, and `onRpcBroken()` callbacks are invoked. +You can construct an `RpcPromise` directly from a regular `Promise`, allowing you to perform promise pipelining on a regular local promise. Pipelined calls will wait until the inner promise resolves, then will be delivered, in-order, to the resolution. This is useful when you plan to obtain some stub in the future, but you want to allow code to start queuing calls on it immediately. -This is useful when the application knows a capability will exist but doesn't have it yet. For example, while re-establishing a broken session, you can publish a promise-backed stand-in, so that interim calls queue up and flow to the new connection once it is ready: +Wrapping a `Promise` in this way is semantically identical to creating a local-loopback RPC and then invoking it. That is: ```ts -// reconnect() returns Promise>. -let promise = new RpcPromise(reconnect()); +// this... +let rpcPromise = new RpcPromise(myPromise); -// Calls pipeline immediately, and are delivered once reconnect() resolves. -let result = await promise.doSomething(); +// is semantically the same as this... +let rpcFunc = new RpcStub(() => myPromise); +let rpcPromise = rpcFunc(); ``` -Two things to watch out for: - -* Ownership of the resolution transfers to the `RpcPromise`: disposing it disposes the target (or stub) that the promise resolved to. If you also want to keep the stub you resolved the promise with, resolve it with a `.dup()`. -* Calls made while the promise is pending queue unboundedly, holding copies of their arguments. If the awaited capability may never arrive -- e.g. reconnection fails permanently -- reject the promise, so that queued calls fail rather than accumulate. +In other words, this means: +* The result of the promise must be serializable. +* If the promise resolution contains `RpcTarget`s or `Function`s, the `RpcPromise`'s resolution will replace them with stubs. +* Ownership of any stubs in the Promise result is transferred away. If you want to keep your own copies, you need to `dup()` them. +* If the promise rejects, the rejection propagates to all pipelined calls. +* etc. ### The magic `map()` method diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 2b592d6d..80d1f5b9 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -2510,7 +2510,7 @@ describe("constructing RpcPromise from a promise", () => { expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); }); - it("preserves brokenness of an adopted stub", async () => { + it("preserves brokenness of a bare stub it was constructed from", async () => { await using harness = new TestHarness(new TestTarget()); using stub = new RpcPromise(harness.stub.dup()); @@ -2522,7 +2522,7 @@ describe("constructing RpcPromise from a promise", () => { expect(errors).toStrictEqual([new Error("test disconnect")]); }); - it("keeps adopted stub disposal idempotent", () => { + it("keeps disposal idempotent when constructed from a bare stub", async () => { let disposals = 0; class Disposable extends RpcTarget { [Symbol.dispose]() { ++disposals; } @@ -2533,8 +2533,20 @@ describe("constructing RpcPromise from a promise", () => { inner[Symbol.dispose](); outer[Symbol.dispose](); + await pumpMicrotasks(); expect(disposals).toBe(1); }); + + it("resolves when awaited after construction from a bare local stub", async () => { + // Regression test: the constructor previously adopted a bare stub's hook directly, producing + // a promise whose pipelined calls worked but whose await rejected, because non-promise hooks + // don't implement pull(). + using stub = new RpcPromise(new RpcStub(new Counter(1))); + + expect(await stub.increment(2)).toBe(3); + let resolved = await stub; + expect(await resolved.increment(3)).toBe(6); + }); }); // ======================================================================================= diff --git a/src/core.ts b/src/core.ts index 7453efa6..4d554a99 100644 --- a/src/core.ts +++ b/src/core.ts @@ -562,8 +562,8 @@ export class RpcStub extends RpcTarget { export class RpcPromise extends RpcStub { // Internally, an `RpcPromise` is constructed from a `StubHook` plus a property path. The - // application may instead pass a promise (or any other thenable) for the eventual resolution; - // calls made before it settles are queued and delivered, in order, once it does. + // application may instead pass a promise for the eventual resolution; calls made before it + // settles are queued and delivered, in order, once it does. constructor(hook: StubHook | PromiseLike, pathIfPromise?: PropertyPath) { if (hook instanceof StubHook) { super(hook, pathIfPromise!); @@ -571,7 +571,37 @@ export class RpcPromise extends RpcStub { if (pathIfPromise !== undefined) { throw new TypeError("RpcPromise constructor expected one argument, received two."); } - super(hookForPromiseArg(hook), []); + + if (typeForRpc(hook) === "rpc-promise") { + // Adopt an existing `RpcPromise` directly, transferring ownership of its hook. In + // particular, this keeps the adopted promise lazy -- assimilating it as a thenable would + // instead force its resolution to be pulled -- and preserves hook-local behavior such as + // brokenness. This applies only to promises, not bare stubs: a non-promise hook may not + // implement pull(), so a bare stub takes the generic path below, which adopts the stub + // into the resolution payload. + super(unwrapStubTakingOwnership(hook), []); + } else { + // `Promise.resolve()` natively handles the hazards of assimilating an arbitrary thenable + // (`then` getters with side effects, self-resolution, cross-realm thenables), so the + // resolution callback below only ever sees settled, non-thenable values. + // + // The resolution is adopted with "return" semantics, taking ownership of any stubs + // within (including a stub as the root value). This is the same representation used for + // the resolution of a local async call: pull() delivers the value, pipelined calls + // forward through the payload without forcing a pull, and a single-stub payload forwards + // onBroken(), preserving brokenness. + // + // A rejection is adopted as an ErrorStubHook rather than left to reject the backing + // promise. This way the promise chains backing queued calls never reject -- the calls + // land on the ErrorStubHook, which disposes their arguments -- and the error only + // surfaces through pull(), so a discarded pipelined call can't produce an unhandled + // rejection event. + let promiseHook = new PromiseStubHook(Promise.resolve(hook).then( + value => new PayloadStubHook(RpcPayload.fromAppReturn(value)), + err => new ErrorStubHook(err))); + promiseHook.ignoreUnhandledRejections(); + super(promiseHook, []); + } } } @@ -614,41 +644,6 @@ export function unwrapStubTakingOwnership(stub: RpcStub): StubHook { } } -// Given the application's argument to `new RpcPromise(value)`, produce the hook backing the -// promise. -function hookForPromiseArg(value: PromiseLike): StubHook { - let type = typeForRpc(value); - if (type === "stub" || type === "rpc-promise") { - // Adopt an existing stub or promise directly, transferring ownership of its hook. In - // particular, this keeps an adopted `RpcPromise` lazy -- assimilating it as a thenable would - // instead force its resolution to be pulled -- and preserves hook-local behavior such as - // brokenness. - return unwrapStubTakingOwnership(value); - } - - // `Promise.resolve()` natively handles the hazards of assimilating an arbitrary thenable - // (`then` getters with side effects, self-resolution, cross-realm thenables), so - // `hookForResolution()` only ever sees settled, non-thenable values. - // - // A rejection is adopted as an ErrorStubHook rather than left to reject the backing promise. - // This way the promise chains backing queued calls never reject -- the calls land on the - // ErrorStubHook, which disposes their arguments -- and the error only surfaces through pull(), - // so a discarded pipelined call can't produce an unhandled rejection event. - let hook = new PromiseStubHook( - Promise.resolve(value).then(hookForResolution, err => new ErrorStubHook(err))); - hook.ignoreUnhandledRejections(); - return hook; -} - -// Adopt the resolution of a promise passed to `new RpcPromise()` with "return" semantics, taking -// ownership of any stubs within (including a stub as the root value). This is the same -// representation used for the resolution of a local async call: pull() delivers the value, -// pipelined calls forward through the payload without forcing a pull, and a single-stub payload -// forwards onBroken(), preserving brokenness. -function hookForResolution(value: unknown): StubHook { - return new PayloadStubHook(RpcPayload.fromAppReturn(value)); -} - // Given a stub (still wrapped in a Proxy), extract the underlying `StubHook`, and duplicate it, // returning the duplicate. // diff --git a/src/index.ts b/src/index.ts index 7820e2f9..a66a7d92 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,7 @@ import { RpcTarget as RpcTargetImpl, RpcStub as RpcStubImpl, RpcPromise as RpcPr import { serialize, deserialize, EncodingLevel } from "./serialize.js"; import { RpcTransport, RpcTransportWithCustomEncoding, AnyRpcTransport, RpcSession as RpcSessionImpl, RpcSessionOptions } from "./rpc.js"; import { RpcLimits, DEFAULT_LIMITS, DEFAULT_MAX_DEPTH } from "./serialize.js"; -import { RpcTargetBranded, RpcCompatible, Stub, Stubable, type RpcPromise as RpcPromiseType, +import { RpcTargetBranded, RpcCompatible, Stub, type RpcPromise as RpcPromiseType, __RPC_TARGET_BRAND } from "./types.js"; import { newWebSocketRpcSession as newWebSocketRpcSessionImpl, newWorkersWebSocketRpcResponse, WebSocketTransport } from "./websocket.js"; @@ -59,18 +59,18 @@ export const RpcStub: { * if you only intend to use the promise for pipelining and you never await it, then there's no * need to transmit the resolution! * - * You may also construct an `RpcPromise` yourself from a regular `Promise` (or any other - * thenable), using `new RpcPromise(promise)`. The promise may resolve to an `RpcTarget`, a stub, - * or a plain value. Calls made before the promise settles are queued and delivered, in order, - * once it does, so an `RpcPromise` can stand in for a capability that doesn't exist yet -- for - * example, one that will only become available after a broken session has been re-established. - * Note that the `RpcPromise` takes ownership of the resolution: disposing it disposes the - * resolution, so resolve the promise with a `dup()` if you also intend to keep the stub. + * You may also construct an `RpcPromise` yourself from a regular `Promise`, using + * `new RpcPromise(promise)`, allowing you to perform promise pipelining on a local promise. This + * is semantically identical to creating a local-loopback RPC that returns the promise, and then + * invoking it: pipelined calls wait until the promise resolves, then are delivered, in order, to + * the resolution. This is useful when you plan to obtain some stub in the future, but want to + * allow code to start queuing calls on it immediately. Note that the `RpcPromise` takes + * ownership of the resolution: disposing it disposes the resolution, so resolve the promise + * with a `dup()` if you also intend to keep the stub. */ export type RpcPromise> = RpcPromiseType; export const RpcPromise: { - new (value: PromiseLike>): RpcPromise; - new >(value: PromiseLike>): RpcPromise; + new >(value: Promise>): RpcPromise; } = RpcPromiseImpl; /** From b4d83b8395ffa494baeb6f757bfe052f2beab8f1 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 17 Aug 2026 18:43:59 -0500 Subject: [PATCH 3/6] fix: address code-review findings on RpcPromise-from-Promise - Adopting an existing RpcPromise now consumes the source: its hook is neutered to DISPOSED_HOOK, so disposing the source can no longer silently kill the wrapper. Using the source after wrapping reports the standard disposed error. - Restore the invariant that every RpcPromise has a defined path by defaulting pathIfPromise to [] on the internal StubHook path. - Wrap workerd-native RpcPromise/RpcProperty values (rpc-thenable) in a TargetStubHook so pipelined calls aren't eagerly assimilated. - Document ownership transfer on adoption and the dup() workaround for keeping a deferred capability lazy when resolving a native Promise with an RpcPromise. --- README.md | 4 ++++ __tests__/index.test.ts | 34 ++++++++++++++++++++++++++++++++++ __tests__/workerd.test.ts | 13 ++++++++++++- src/core.ts | 33 ++++++++++++++++++++++++--------- src/index.ts | 9 +++++++++ 5 files changed, 83 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index a1dc73e5..6a2595dd 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,10 @@ In other words, this means: * If the promise rejects, the rejection propagates to all pipelined calls. * etc. +Two special cases to be aware of: +* Passing an existing `RpcPromise` (rather than a regular `Promise`) to the constructor adopts it directly, transferring ownership: the original promise is consumed and must not be used afterwards — use the wrapper in its place. +* If a regular `Promise` resolves to an `RpcPromise`, JavaScript's promise machinery assimilates the thenable, which forces its resolution to be pulled. To keep a deferred capability lazy, resolve the promise with `rpcPromise.dup()` instead: `dup()` returns a non-thenable stub, which is adopted into the resolution and forwards pipelined calls without pulling. + ### The magic `map()` method Every RPC promise has a special method `.map()` which can be used to remotely transform a value, without pulling it back locally. Here's an example: diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 80d1f5b9..78dc8e5f 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -2510,6 +2510,40 @@ describe("constructing RpcPromise from a promise", () => { expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); }); + it("consumes the source when adopting an existing RpcPromise", async () => { + await using harness = new TestHarness(new TestTarget()); + + let source = harness.stub.makeCounter(1); + using wrapper = new RpcPromise(source); + + // The source was neutered: using it now reports the standard disposed error, and disposing + // it is a harmless no-op that doesn't affect the wrapper. + await expect(source.increment(1)).rejects.toThrow( + "Attempted to use RPC stub after it has been disposed."); + source[Symbol.dispose](); + + expect(await wrapper.increment(2)).toBe(3); + }); + + it("stays lazy when a deferred promise is resolved with dup()", async () => { + await using harness = new TestHarness(new TestTarget()); + + using counter = harness.stub.makeCounter(1); + + // Resolving with the RpcPromise itself would let the native promise machinery assimilate it + // as a thenable, pulling the resolution. dup() returns a non-thenable stub, which the + // resolution adopts, keeping calls pipelined. + let {promise, resolve} = Promise.withResolvers>(); + using stub = new RpcPromise(promise); + + let result = stub.increment(2); + resolve(counter.dup()); + expect(await result).toBe(3); + + let sent = harness.clientTransport.sentLog; + expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); + }); + it("preserves brokenness of a bare stub it was constructed from", async () => { await using harness = new TestHarness(new TestTarget()); using stub = new RpcPromise(harness.stub.dup()); diff --git a/__tests__/workerd.test.ts b/__tests__/workerd.test.ts index b8768c84..b23fea55 100644 --- a/__tests__/workerd.test.ts +++ b/__tests__/workerd.test.ts @@ -5,7 +5,7 @@ /// import { expect, it, describe } from "vitest"; import { RpcStub as NativeRpcStub, RpcTarget as NativeRpcTarget, env, DurableObject } from "cloudflare:workers"; -import { newHttpBatchRpcSession, newWebSocketRpcSession, RpcStub, RpcTarget } from "../src/index-workers.js"; +import { newHttpBatchRpcSession, newWebSocketRpcSession, RpcStub, RpcPromise, RpcTarget } from "../src/index-workers.js"; import { v, wrapServerTarget, type ServiceValidator } from "../packages/capnweb-validate/src/internal/core.js"; import { Counter, TestTarget } from "./test-util.js"; @@ -126,6 +126,17 @@ describe("workerd compatibility", () => { } }) + it("can wrap a native promise in a userspace promise", async () => { + // Wrapping in RpcPromise (rather than RpcStub) exercises the rpc-thenable adoption path, + // which pipelines calls on the native thenable without eagerly awaiting it. + let factory = new NativeRpcStub(new CounterFactory()); + let stub = new RpcPromise(factory.getNative()); + expect(await stub.increment()).toBe(1); + expect(await stub.increment()).toBe(2); + + expect(await stub.value).toBe(2); + }) + it("can pipeline on a native stub returned from a userspace call", async () => { { let factory = new RpcStub(new CounterFactory()); diff --git a/src/core.ts b/src/core.ts index 4d554a99..763f0efa 100644 --- a/src/core.ts +++ b/src/core.ts @@ -566,20 +566,35 @@ export class RpcPromise extends RpcStub { // settles are queued and delivered, in order, once it does. constructor(hook: StubHook | PromiseLike, pathIfPromise?: PropertyPath) { if (hook instanceof StubHook) { - super(hook, pathIfPromise!); + super(hook, pathIfPromise ?? []); } else { if (pathIfPromise !== undefined) { throw new TypeError("RpcPromise constructor expected one argument, received two."); } - if (typeForRpc(hook) === "rpc-promise") { - // Adopt an existing `RpcPromise` directly, transferring ownership of its hook. In - // particular, this keeps the adopted promise lazy -- assimilating it as a thenable would - // instead force its resolution to be pulled -- and preserves hook-local behavior such as - // brokenness. This applies only to promises, not bare stubs: a non-promise hook may not - // implement pull(), so a bare stub takes the generic path below, which adopts the stub - // into the resolution payload. - super(unwrapStubTakingOwnership(hook), []); + let kind = typeForRpc(hook); + if (kind === "rpc-promise") { + // Adopt an existing `RpcPromise` directly, transferring ownership of its hook: the source + // promise is neutered, as if disposed, and must not be used afterwards. In particular, + // adoption keeps the promise lazy -- assimilating it as a thenable would instead force its + // resolution to be pulled -- and preserves hook-local behavior such as brokenness. This + // applies only to promises, not bare stubs: a non-promise hook may not implement pull(), + // so a bare stub takes the generic path below, which adopts the stub into the resolution + // payload. + let raw = unwrapStubAndPath(hook); + if (raw.pathIfPromise!.length > 0) { + // Property promise: get() returns an independent hook, and properties have no + // disposer, so there is nothing to neuter. + super(raw.hook.get(raw.pathIfPromise!), []); + } else { + let adopted = raw.hook; + raw.hook = DISPOSED_HOOK; + super(adopted, []); + } + } else if (kind === "rpc-thenable") { + // Workerd-native RpcPromise/RpcProperty: wrap in a TargetStubHook, which pipelines calls + // directly on the thenable and awaits it only on pull(). + super(TargetStubHook.create(hook, undefined), []); } else { // `Promise.resolve()` natively handles the hazards of assimilating an arbitrary thenable // (`then` getters with side effects, self-resolution, cross-realm thenables), so the diff --git a/src/index.ts b/src/index.ts index a66a7d92..8effebe2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -67,6 +67,15 @@ export const RpcStub: { * allow code to start queuing calls on it immediately. Note that the `RpcPromise` takes * ownership of the resolution: disposing it disposes the resolution, so resolve the promise * with a `dup()` if you also intend to keep the stub. + * + * Passing an existing `RpcPromise` to the constructor adopts it directly, transferring ownership: + * the original promise is consumed (as if disposed) and the wrapper must be used in its place. + * + * Note that if a regular `Promise` resolves to an `RpcPromise`, JavaScript's promise machinery + * assimilates the thenable, which forces its resolution to be pulled. To keep a deferred + * capability lazy, resolve the promise with `rpcPromise.dup()` instead: `dup()` returns a + * non-thenable stub, which is adopted into the resolution and forwards pipelined calls without + * pulling. */ export type RpcPromise> = RpcPromiseType; export const RpcPromise: { From ed8f6c5b52d774ba12093221bf61575a0cc0576c Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Tue, 18 Aug 2026 09:20:24 -0500 Subject: [PATCH 4/6] docs: remove constructor special-case paragraphs per review Per review feedback on #242: the ownership-transfer note (nobody wraps an RpcPromise they already hold on purpose) and the thenable-assimilation note (not specific to this constructor) don't belong in the public docs. The behaviors themselves are unchanged and remain pinned by tests. --- README.md | 4 ---- src/index.ts | 9 --------- 2 files changed, 13 deletions(-) diff --git a/README.md b/README.md index 6a2595dd..a1dc73e5 100644 --- a/README.md +++ b/README.md @@ -289,10 +289,6 @@ In other words, this means: * If the promise rejects, the rejection propagates to all pipelined calls. * etc. -Two special cases to be aware of: -* Passing an existing `RpcPromise` (rather than a regular `Promise`) to the constructor adopts it directly, transferring ownership: the original promise is consumed and must not be used afterwards — use the wrapper in its place. -* If a regular `Promise` resolves to an `RpcPromise`, JavaScript's promise machinery assimilates the thenable, which forces its resolution to be pulled. To keep a deferred capability lazy, resolve the promise with `rpcPromise.dup()` instead: `dup()` returns a non-thenable stub, which is adopted into the resolution and forwards pipelined calls without pulling. - ### The magic `map()` method Every RPC promise has a special method `.map()` which can be used to remotely transform a value, without pulling it back locally. Here's an example: diff --git a/src/index.ts b/src/index.ts index 8effebe2..a66a7d92 100644 --- a/src/index.ts +++ b/src/index.ts @@ -67,15 +67,6 @@ export const RpcStub: { * allow code to start queuing calls on it immediately. Note that the `RpcPromise` takes * ownership of the resolution: disposing it disposes the resolution, so resolve the promise * with a `dup()` if you also intend to keep the stub. - * - * Passing an existing `RpcPromise` to the constructor adopts it directly, transferring ownership: - * the original promise is consumed (as if disposed) and the wrapper must be used in its place. - * - * Note that if a regular `Promise` resolves to an `RpcPromise`, JavaScript's promise machinery - * assimilates the thenable, which forces its resolution to be pulled. To keep a deferred - * capability lazy, resolve the promise with `rpcPromise.dup()` instead: `dup()` returns a - * non-thenable stub, which is adopted into the resolution and forwards pipelined calls without - * pulling. */ export type RpcPromise> = RpcPromiseType; export const RpcPromise: { From 52bdc896051e25744ddedc40f712cde8aef8517f Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Wed, 19 Aug 2026 16:11:02 -0500 Subject: [PATCH 5/6] fix: repair dup(), onRpcBroken, and property adoption for promise-wrapped native stubs - get([]) on a thenable-backed TargetStubHook now returns dup() instead of throwing, fixing dup() and argument-passing of wrapped native promises. - onBroken() now subscribes to a thenable target's rejection, so onRpcBroken fires when a wrapped native promise rejects instead of silently no-oping. - Property promises share the source hook and path so the get() happens lazily on first use, avoiding eager wire pushes / getter side effects. --- __tests__/index.test.ts | 38 +++++++++++++++++++++++++++++++ __tests__/workerd.test.ts | 47 +++++++++++++++++++++++++++++++++++++++ src/core.ts | 27 ++++++++++++++++------ 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 78dc8e5f..93cb5d8d 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -2510,6 +2510,44 @@ describe("constructing RpcPromise from a promise", () => { expect(sent.filter(msg => msg.startsWith('["pull"'))).toHaveLength(1); }); + it("transmits nothing when constructed from a remote property promise", async () => { + await using harness = new TestHarness(new TestTarget()); + + using counter = harness.stub.makeCounter(5); + await pumpMicrotasks(); // let the makeCounter push flush + + let source = counter.value; + let wrapped = new RpcPromise(source); + + // Construction shares the source's hook and path; the get() producing an independent hook + // happens lazily on first await, so nothing goes over the wire yet. + let sentBefore = harness.clientTransport.sentLog.length; + await pumpMicrotasks(); + expect(harness.clientTransport.sentLog.length).toBe(sentBefore); + + expect(await wrapped).toBe(5); + + // The source property promise remains usable. + expect(await source).toBe(5); + }); + + it("does not invoke a local getter when constructed from a property promise", async () => { + let reads = 0; + class Gettable extends RpcTarget { + get prop() { ++reads; return 42; } + } + + using stub = new RpcStub(new Gettable()); + let source = stub.prop; + let wrapped = new RpcPromise(source); + + await pumpMicrotasks(); + expect(reads).toBe(0); + + expect(await wrapped).toBe(42); + expect(await source).toBe(42); + }); + it("consumes the source when adopting an existing RpcPromise", async () => { await using harness = new TestHarness(new TestTarget()); diff --git a/__tests__/workerd.test.ts b/__tests__/workerd.test.ts index b23fea55..64404b72 100644 --- a/__tests__/workerd.test.ts +++ b/__tests__/workerd.test.ts @@ -44,6 +44,10 @@ class CounterFactory extends RpcTarget { return new NativeRpcStub(new NativeCounter()); } + getBroken(): NativeRpcStub { + throw new RangeError("test error"); + } + getNativeEmbedded() { return {stub: new NativeRpcStub(new NativeCounter())}; } @@ -57,6 +61,12 @@ class CounterFactory extends RpcTarget { } } +async function pumpMicrotasks() { + for (let i = 0; i < 16; i++) { + await Promise.resolve(); + } +} + describe("workerd compatibility", () => { it("allows native RpcStubs to be created using userspace RpcTargets", async () => { let stub = new NativeRpcStub(new JsCounter()); @@ -137,6 +147,43 @@ describe("workerd compatibility", () => { expect(await stub.value).toBe(2); }) + it("can dup a userspace promise wrapping a native promise", async () => { + let factory = new NativeRpcStub(new CounterFactory()); + let promise = new RpcPromise(factory.getNative()); + + // dup() routes through get([]), which must produce an independent hook aliasing the same + // underlying native promise. + let dup = promise.dup(); + expect(await dup.increment()).toBe(1); + expect(await promise.increment()).toBe(2); + }) + + it("can pass a wrapped native promise as an RPC argument", async () => { + class CounterUser extends RpcTarget { + useCounter(counter: RpcStub) { + return counter.increment(5); + } + } + + let factory = new NativeRpcStub(new CounterFactory()); + let user = new RpcStub(new CounterUser()); + let arg = new RpcPromise(factory.getNative()); + expect(await user.useCounter(arg)).toBe(5); + }) + + it("reports brokenness when a wrapped native promise rejects", async () => { + let factory = new NativeRpcStub(new CounterFactory()); + let promise = new RpcPromise(factory.getBroken()); + + let errors: any[] = []; + promise.onRpcBroken(err => { errors.push(err); }); + + await expect(Promise.resolve(promise)).rejects.toThrow("test error"); + await pumpMicrotasks(); + expect(errors.length).toBe(1); + expect(errors[0].message).toBe("test error"); + }) + it("can pipeline on a native stub returned from a userspace call", async () => { { let factory = new RpcStub(new CounterFactory()); diff --git a/src/core.ts b/src/core.ts index 763f0efa..d9dc1ca8 100644 --- a/src/core.ts +++ b/src/core.ts @@ -583,9 +583,10 @@ export class RpcPromise extends RpcStub { // payload. let raw = unwrapStubAndPath(hook); if (raw.pathIfPromise!.length > 0) { - // Property promise: get() returns an independent hook, and properties have no - // disposer, so there is nothing to neuter. - super(raw.hook.get(raw.pathIfPromise!), []); + // Property promise: share the source's hook and path, exactly like the source promise. + // The get() producing an independent hook happens lazily on first use, and properties + // have no disposer, so there is nothing to neuter. + super(raw.hook, raw.pathIfPromise); } else { let adopted = raw.hook; raw.hook = DISPOSED_HOOK; @@ -1842,10 +1843,16 @@ abstract class ValueStubHook extends StubHook { let {value, owner} = this.getValue(); if (path.length === 0 && owner === null) { + if (value instanceof Object && "then" in value) { + // The hook wraps a thenable (e.g. a workerd-native RpcPromise or RpcProperty), so it + // really does back a promise. get([]) asks for an independent hook aliasing the same + // promise, which is exactly dup(). + return this.dup(); + } + // The only way this happens is if someone sends "pipeline" and references a - // TargetStubHook, but they shouldn't do that, because TargetStubHook never backs a - // promise, and a non-promise cannot be converted to a promise. - // TODO: Is this still correct for rpc-thenable? + // TargetStubHook wrapping a non-thenable, but they shouldn't do that, because such a + // hook never backs a promise, and a non-promise cannot be converted to a promise. throw new Error("Can't dup an RpcTarget stub as a promise."); } @@ -2056,7 +2063,13 @@ class TargetStubHook extends ValueStubHook { } onBroken(callback: (error: any) => void): void { - // TODO: Should RpcTargets be able to implement onRpcBroken? + let target = this.target; + if (target && "then" in target) { + // The target is thenable (e.g. a workerd-native RpcPromise), so it backs a promise, which + // becomes broken if it rejects. + Promise.resolve(target).then(() => {}, callback); + } + // TODO: Should non-thenable RpcTargets be able to implement onRpcBroken? } } From bf3c7be4cbf1cb67cdb65e8e50c2391adff17b50 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Thu, 20 Aug 2026 07:07:35 -0500 Subject: [PATCH 6/6] fix: let rejection reject the backing promise instead of adopting ErrorStubHook Per review: PromiseStubHook already handles a rejected backing promise -- it disposes the arguments of queued calls (since #241) and surfaces the error through pull() and onBroken() -- so the constructor no longer maps rejection to an ErrorStubHook resolution. Observable change: a pipelined call whose result is neither awaited nor disposed now fires an unhandled rejection event, matching the existing behavior of local async calls. The unhandled-rejection tests now dispose the discarded results, which both silences the event and models correct usage. --- __tests__/index.test.ts | 8 ++++---- src/core.ts | 13 ++++++------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts index 93cb5d8d..f943e8d6 100644 --- a/__tests__/index.test.ts +++ b/__tests__/index.test.ts @@ -2456,15 +2456,15 @@ describe("constructing RpcPromise from a promise", () => { await pumpMicrotasks(); }); - it("does not report an unhandled rejection for a discarded queued call", async () => { + it("does not report an unhandled rejection for a disposed, unawaited queued call", async () => { using stub = new RpcPromise(Promise.reject(new Error("ignored"))); - stub.increment(); // result intentionally discarded + using result = stub.increment(); // never awaited; disposal alone must observe the error await pumpMicrotasks(); }); - it("does not report an unhandled rejection for a discarded map() result", async () => { + it("does not report an unhandled rejection for a disposed, unawaited map() result", async () => { using stub = new RpcPromise(Promise.reject(new Error("ignored"))); - stub.map(i => i); // result intentionally discarded + using result = stub.map(i => i); // never awaited; disposal alone must observe the error await pumpMicrotasks(); }); diff --git a/src/core.ts b/src/core.ts index d9dc1ca8..b4109748 100644 --- a/src/core.ts +++ b/src/core.ts @@ -607,14 +607,13 @@ export class RpcPromise extends RpcStub { // forward through the payload without forcing a pull, and a single-stub payload forwards // onBroken(), preserving brokenness. // - // A rejection is adopted as an ErrorStubHook rather than left to reject the backing - // promise. This way the promise chains backing queued calls never reject -- the calls - // land on the ErrorStubHook, which disposes their arguments -- and the error only - // surfaces through pull(), so a discarded pipelined call can't produce an unhandled - // rejection event. + // A rejection is left on the backing promise, like the rejection of a local async call: + // PromiseStubHook disposes the arguments of queued calls and surfaces the error through + // pull() and onBroken(). The ignoreUnhandledRejections() call keeps a never-used promise + // from firing an unhandled rejection event; a pipelined call whose result is neither + // awaited nor disposed still fires one, matching local async calls. let promiseHook = new PromiseStubHook(Promise.resolve(hook).then( - value => new PayloadStubHook(RpcPayload.fromAppReturn(value)), - err => new ErrorStubHook(err))); + value => new PayloadStubHook(RpcPayload.fromAppReturn(value)))); promiseHook.ignoreUnhandledRejections(); super(promiseHook, []); }