diff --git a/CONCEPTS.md b/CONCEPTS.md new file mode 100644 index 000000000..53a863b53 --- /dev/null +++ b/CONCEPTS.md @@ -0,0 +1,27 @@ +# Concepts + +Shared domain vocabulary for this project — entities, named processes, and status concepts with project-specific meaning. Seeded with core domain vocabulary, then accretes as ce-compound and ce-compound-refresh process learnings; direct edits are fine. Glossary only, not a spec or catch-all. + +## Host and Product + +### Product +A web application that calls TrUAPI methods on the Host embedding it. A Product holds no keys and performs no signing itself; every privileged action is a request to its Host. + +### Host +The native Polkadot application that embeds a Product, owns the keys and the user-facing prompts, and answers the Product's TrUAPI calls. Host and Product run in separate execution contexts and share no memory, so everything between them crosses a process boundary as bytes. + +### Action +The wire-level unit a TrUAPI method expands into: a plain call becomes a request/response pair, a subscription becomes a start/stop/interrupt/receive lifecycle. Each action carries a discriminant id that is append-only and never reused, which is what lets a newer Host and an older Product still understand each other. + +### Remote authority +A separately paired device or host that answers on the user's behalf when the embedding Host cannot answer locally — the signing side of an SSO pairing. A remote-authority answer is bounded by a deadline the Host sets, not by the Product; a Product that bounds such a call more tightly than the Host does is choosing to abandon an answer the Host is still willing to deliver. + +## Request bounds + +### Request timeout floor +The minimum bound a client applies to one specific method, overriding a shorter bound the embedding product configured, so a request is never abandoned while the Host is still permitted to answer it. + +The effective bound for a request is the larger of the product's configured bound and the method's floor; a per-request override, when supplied, wins outright over both. When a bound fires, the client drops its correlation entry, so a reply that arrives afterwards is inert rather than late. Nothing is sent to the Host on expiry — requests have no cancel action, unlike subscriptions, which have a stop — so a timed-out call may still complete Host-side, and a caller retrying a call with side effects can cause it to happen twice. + +### Prompt-backed request +A request whose answer waits on a person — a consent dialog, a pairing approval, a payment confirmation — rather than on computation or a chain. Such calls carry no Host-side deadline at all, so they are the slowest calls in the system and cannot be bounded by reasoning about Host deadlines. Its complement is a prompt-free request, which the Host answers without involving anyone. diff --git a/README.md b/README.md index 6d967f9ed..3c13a5beb 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,11 @@ import { const transport = createTransport(createMessagePortProvider(port)); const truapi = createClient(transport); -const result = await truapi.accountManagement.accountGet({ - productAccountId: { dotNsIdentifier: "my-product.dot", derivationIndex: { tag: "Index", value: 0 } }, +const result = await truapi.account.getAccount({ + productAccountId: { + dotNsIdentifier: "my-product.dot", + derivationIndex: { tag: "Index", value: 0 }, + }, }); ``` diff --git a/docs/solutions/runtime-errors/unanswered-request-never-settles.md b/docs/solutions/runtime-errors/unanswered-request-never-settles.md new file mode 100644 index 000000000..d517238a3 --- /dev/null +++ b/docs/solutions/runtime-errors/unanswered-request-never-settles.md @@ -0,0 +1,175 @@ +--- +title: "Timeout unanswered TrUAPI requests so an embedder's promise always settles" +date: 2026-08-14 +category: runtime-errors +module: "@parity/truapi client transport (js/packages/truapi)" +problem_type: runtime_error +component: service_object +symptoms: + - "A request the host accepts and never answers neither resolves nor rejects while the message channel stays open" + - "A signed-in person sits on a permanently disabled button with no error logged" + - "The pending map keeps the entry indefinitely; entries leave only on a response frame, a transport close, or a synchronous send failure" +root_cause: async_timing +resolution_type: code_fix +severity: high +related_components: + - tooling +tags: + - request-timeout + - promise-hang + - timeout-floor + - pending-map + - transport + - host-deadline +--- + +# Timeout unanswered TrUAPI requests so an embedder's promise always settles + +## Problem + +A product awaiting a TrUAPI request such as `client.account.getAccount()` got no answer when the host accepted the request and then replied with nothing while the message channel stayed nominally open: the returned promise neither resolved nor rejected. Nothing timed it out, so the product waited forever and the calling code could not tell a slow host from a silent one. + +## Symptoms + +- A request frame the peer accepts and never answers returns a promise that stays pending indefinitely — no resolution, no rejection. +- A signed-in person is left looking at a permanently disabled button with no error to log: the hang produces no observable signal at all. +- The transport's `pending` map keeps the request entry for the lifetime of the transport. Entries left it on only three paths — a matching response frame, a transport/provider close, or a synchronous `send` failure — none of which fire when the channel simply goes silent. +- The protocol spec does not save you here: it requires exactly one response per `requestId` (`docs/design/truapi-protocol.md:124`) but mandates no client-side deadline, and names a timeout only for the handshake (`docs/design/truapi-protocol.md:150`). + +## What Didn't Work + +**A floor table whose inclusion rule was "the host deadline exceeds the default" silently dropped the slowest methods in the system.** Per-method floors are needed because a flat default would abort answers the host is still allowed to send — but that rule cannot express *"this call has no deadline at all"*, and those are exactly the slowest calls. In `rust/crates/truapi-server/src/runtime.rs`, `request_device_permission` (line 813) and `request_remote_permission` (line 835) ignore `_cx` entirely and await `check_or_prompt_device` / `check_or_prompt_remote`, which block on a human; `request_login` (line 1303) ignores `_cx` and delegates to an unbounded pairing loop. `rust/crates/truapi-server/src/native.rs:378-382` documents that these host callbacks "may keep the future pending arbitrarily long". Under a flat 30s default, every one of them would have been aborted mid-prompt while the host went on to record the user's answer. + +The same rule also missed methods that *do* sit behind the 180s remote-authority deadline, because it was applied from the list of Rust timeout constants rather than from the call sites that use them. Six handlers route through `remote_authority_context(cx)` and were absent from the first table: `register_ring_vrf_key` (`runtime.rs:1090`), `list_ring_vrf_keys` (`:1136`), `ring_vrf_sign` (`:1184`), `sign_vrf` (`:1226`), and both statement-store proof helpers (`rust/crates/truapi-server/src/runtime/statement_store.rs:319`, `:343`). Five wire methods had no floor at all as a result; the sixth, authorized proof creation, was floored in the wrong class. Deriving the table from `remote_authority_context` call sites, and adding a class for "no host deadline", is what made it complete. + +**Tests that asserted only settle ordering let a wrong precedence survive.** The first cut raced a floored request against a 5ms control and asserted the control settled first. Ordering alone is consistent with both `max(configured, floor)` and the mutant `floor ?? configured`, because every ordering test configured a bound *below* the floor, where both expressions pick the floor. A deliberately long configured bound was never exercised, so the mutant — which silently caps a product's long timeout at the floor — stayed green. + +**Unref'd timers.** An earlier cut armed `setTimeout(…).unref()` so a pending request could not hold a process open. Measured on Node, an unref'd timer's rejection is dropped when the event loop is otherwise empty: the process exits before the timer fires and the rejection is lost, restoring the original hang in exactly the case the timeout exists to fix. The shipped timer is ref'd, and the tests dispose the transport instead. + +## Solution + +Every request gets a bound — a per-request override, else `max(configured, method floor)`. A `setTimeout` armed *before* `send` rejects with a typed error, one choke point removes pending entries, and invalid bounds are rejected at the call site. + +A typed timeout error, discriminated from a close error by type and never by message text (`js/packages/truapi/src/transport.ts:65`): + +```ts +export class RequestTimeoutError extends Error { + readonly timeoutMs: number; + constructor(timeoutMs: number) { + super(`TrUAPI request timed out after ${timeoutMs}ms`); + this.name = "RequestTimeoutError"; + this.timeoutMs = timeoutMs; + } +} +``` + +The timer is armed before `send`, so a synchronous `send` failure still rejects with the close error rather than the timeout (`js/packages/truapi/src/client.ts:617`): + +```ts +const bound = resolveRequestTimeoutMs(ids.request, requestTimeoutMs, timeoutMs); +const promise = new Promise>((resolve, reject) => { + if (closedError) { + reject(closedError); + return; + } + + const requestId = `p:${++idCounter}`; + const timer = setTimeout(() => { + takePending(requestId); + reject(new RequestTimeoutError(bound)); + }, bound); + pending.set(requestId, { + ids, + resolve: (response) => resolve(decodeResponse(response)), + reject, + timer, + }); + try { + send({ requestId, payload: { id: ids.request, value: payload } }); + } catch (error) { + takePending(requestId); + reject(toError(error)); + } +}); +``` + +One removal choke point. Response dispatch, the close loop, and the timeout callback all settle through it, so deletion and timer-clearing can never drift apart (`client.ts:353`): + +```ts +function takePending(requestId: string) { + const entry = pending.get(requestId); + if (!entry) return undefined; + pending.delete(requestId); + clearTimeout(entry.timer); + return entry; +} +``` + +Bound resolution as one testable decision (`client.ts:180`): + +```ts +export function resolveRequestTimeoutMs( + requestFrameId: number, + transportTimeoutMs: number, + perRequestTimeoutMs: number | undefined, +): number { + if (perRequestTimeoutMs !== undefined) { + return checkRequestTimeoutMs(perRequestTimeoutMs); + } + return Math.max( + transportTimeoutMs, + REQUEST_TIMEOUT_FLOOR_MS.get(requestFrameId) ?? 0, + ); +} +``` + +`checkRequestTimeoutMs` (`client.ts:160`) rejects the values `setTimeout` silently collapses into an immediate fire — `0`, `Infinity`, `NaN`, and anything above `MAX_REQUEST_TIMEOUT_MS = 2_147_483_647` (`client.ts:61`) — and it throws at the call site rather than rejecting through the promise, matching the transport-level validation. + +The default is `DEFAULT_REQUEST_TIMEOUT_MS = 30_000` (`client.ts:71`), chosen against budgets the repo already ships rather than freely: the package waits 20s for a host-injected message port (`HOST_PORT_TIMEOUT_MS`, `js/packages/truapi/src/sandbox.ts:80`), and the playground bounds prompt-backed protocol calls at 30s (`playground/src/lib/auto-test.ts:17`). + +`REQUEST_TIMEOUT_FLOOR_MS` (`client.ts:109`) holds 23 entries keyed on generated `W.*.request` ids — so codegen renumbering cannot silently re-bind a floor to another method — in three classes: + +| Class | Value | Entries | Derivation | +| --- | --- | --- | --- | +| `REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS` (`client.ts:79`) | `190_000` | 15 | above the runtime's 180s remote-authority deadline (`runtime.rs:186`), which itself follows host-spec B.6.2 (`runtime.rs:183-185`) | +| `USER_APPROVAL_REQUEST_TIMEOUT_MS` (`client.ts:89`) | `420_000` | 5 | no host deadline exists; a person answers, so this is the client's own ceiling | +| `LIVE_ALLOCATION_REQUEST_TIMEOUT_MS` (`client.ts:95`) | `420_000` | 3 | above the 300s allocation and 360s preimage caps (`runtime.rs:191`, `:196`) | + +## Why This Works + +The invariant is the single removal choke point. `takePending` is the only place an entry leaves `pending`, and it always clears the timer, so no settled request leaves a live timer that could reject an already-settled promise, and a reply arriving after the bound fired finds no entry to resolve — it is inert rather than throwing (`client.test.ts:744`). Arming the timer before `send` is what keeps the discriminant the caller needs: the synchronous send-failure path settles through the same `takePending` with the close error rather than a timeout (`client.ts:647-650`), so "the channel is gone" stays distinguishable from "the host went silent" — the distinction `client.test.ts:831` pins by asserting a disposed transport rejects with a plain `Error`, never `RequestTimeoutError`. + +The floors work because they are derived from the host's own permission to answer, not from a UI preference. The runtime allows 180s for a remote-authority answer — over six times the default — and the person-backed methods carry no deadline at all, so bounding either at 30s would convert a legitimate slow answer into a spurious timeout, and worse, a caller who then retries a non-idempotent submit can double-execute it (requests carry no cancel frame, unlike subscriptions, which have a `stop`). `Math.max(configured, floor)` lets a product tighten or loosen its own bound while never aborting an answer the host is permitted to deliver. + +## Prevention + +**Gate the classification, not the values.** A generated request method added without a floor fails this test rather than silently inheriting the default (`client.test.ts:913`, with the prompt-free set at `client.test.ts:26`): + +```ts +it("classifies every generated request method as floored or prompt-free", () => { + const unclassified = Object.entries(W) + .filter(([name, ids]) => { + if (!(ids && typeof ids === "object" && "request" in ids)) return false; + const requestId = ids.request; + return ( + typeof requestId === "number" && + !REQUEST_TIMEOUT_FLOOR_MS.has(requestId) && + !PROMPT_FREE_REQUESTS.has(name) + ); + }) + .map(([name]) => name); + + expect(unclassified).toEqual([]); +}); +``` + +**Show each test failing on the bug it targets.** Passing on correct code proves nothing about a test's power. Four mutants were run against this suite when the fix landed, and each failed the case that targets it — the per-mutant failure counts below are that session's measurement, not something the tree records: revert the timer arming (5 cases failed), swap `max(configured, floor)` for `floor ?? configured` (1 — `client.test.ts:882`, which asserts a 500s configured bound survives a 190s floor), delete the per-request override branch (2), drop one method from the classification sets (1). If a plausible regression leaves the suite green, the suite does not test that property. + +**Know that the floors are an ungated cross-language restatement.** `190_000` and `420_000` restate Rust deadlines (`runtime.rs:186`, `:191`, `:196`) with nothing tying them together, and the repo already demonstrates that this drifts: `rust/crates/truapi-host-cli/js/diagnosis.ts:19` bounds prompt-backed methods at `190_000` while `playground/src/lib/auto-test.ts:18` bounds the same methods at `60_000`. Re-verify the floor table whenever a host-side deadline moves; the durable fix is emitting each method's deadline into the generated wire table from rustdoc, so a Rust change either updates the client or fails the build. + +## Related Issues + +- GitHub paritytech/truapi#406 — the hang this learning fixes. +- `docs/design/truapi-protocol.md:124` — the protocol requires exactly one response per `requestId`; `:150` names a timeout only for the handshake. +- `docs/local-e2e-testing.md` — the manual E2E guide names the same symptom ("if a method call hangs") without a budget for it. +- `js/packages/truapi/README.md` — the shipped "Request timeouts" contract, including the floor table an embedder sees. diff --git a/js/packages/truapi/README.md b/js/packages/truapi/README.md index 0981c3169..da4613549 100644 --- a/js/packages/truapi/README.md +++ b/js/packages/truapi/README.md @@ -28,8 +28,11 @@ const provider = createMessagePortProvider(port); const transport = createTransport(provider); const truapi: Client = createClient(transport); -const result = await truapi.accountManagement.accountGet({ - productAccountId: { dotNsIdentifier: "my-product.dot", derivationIndex: { tag: "Index", value: 0 } }, +const result = await truapi.account.getAccount({ + productAccountId: { + dotNsIdentifier: "my-product.dot", + derivationIndex: { tag: "Index", value: 0 }, + }, }); if (result.isErr()) throw result.error; @@ -62,6 +65,67 @@ const sub: Subscription = truapi.chainInteraction sub.unsubscribe(); ``` +## Request timeouts + +Every request carries a time bound so a silent host never hangs the product. `createTransport` +accepts a transport-wide bound in `requestTimeoutMs` — an integer between `1` and `2147483647`, +defaulting to `30_000` — and a per-call `timeoutMs` on `transport.request` overrides it. A request +that outlives its bound rejects with `RequestTimeoutError`, which carries the bound it outlived on +`timeoutMs`. + +```ts +import { + createClient, + createMessagePortProvider, + createTransport, + RequestTimeoutError, + type Client, +} from "@parity/truapi"; + +const provider = createMessagePortProvider(port); +const transport = createTransport(provider, { requestTimeoutMs: 10_000 }); +const truapi: Client = createClient(transport); + +try { + const result = await truapi.account.getAccount({ + productAccountId: { + dotNsIdentifier: "my-product.dot", + derivationIndex: { tag: "Index", value: 0 }, + }, + }); + // … +} catch (error) { + if (error instanceof RequestTimeoutError) { + // The peer accepted the frame and never replied. Requests carry no cancel + // frame, so the host may still be executing: re-query state for a method + // with side effects (submit, allocate, sign) rather than resubmitting. + } else { + // The transport or provider closed; re-establish the channel. + } +} +``` + +A timeout surfaces as a **promise rejection**, not an `Err` in the `ResultAsync`: `request` builds +its result with `ResultAsync.fromSafePromise`, so a `.match(onOk, onErr)` runs neither callback and +the error is thrown instead — await the call inside `try`/`catch`, and discriminate with +`instanceof`, never on message text. + +The effective bound is the larger of the configured `requestTimeoutMs` and the method's floor; +`timeoutMs` overrides both. Floors cover the methods whose answer either outlives the default under +a host deadline or waits on a person with no host deadline at all, so a bound never aborts an answer +the host is still allowed to send: + +| Floor | Methods | Why | +| ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------- | +| `190_000` ms | account get, alias, and proof; VRF sign and ring-VRF register, list, and sign; every signing method; statement-store create-proof and create-proof-authorized | clears the runtime's 180s remote-authority deadline | +| `420_000` ms | request login; device and remote permission prompts; payment request and top-up | a person answers, and the host applies no deadline | +| `420_000` ms | resource allocation; preimage submit; statement-store submit | clears the 300s allocation and 360s preimage caps | + +Generated methods take the request value only, so the per-call `timeoutMs` is reachable through +`transport.request` rather than through a generated client method: a floored method cannot be +shortened from `Client`. `@parity/truapi/sandbox`'s `getClientSync()` takes no options, so it uses +the default `30_000` ms bound and the floors. + ## What's in the package - **Transport providers** for `MessagePort` pipes (used by both webview hosts and iframe hosts). diff --git a/js/packages/truapi/src/client.test.ts b/js/packages/truapi/src/client.test.ts index c49dcc73f..72f35afa8 100644 --- a/js/packages/truapi/src/client.test.ts +++ b/js/packages/truapi/src/client.test.ts @@ -1,17 +1,73 @@ import type { Result } from "neverthrow"; import { describe, expect, it } from "bun:test"; -import { createTransport } from "./client.js"; +import { createTransport, REQUEST_TIMEOUT_FLOOR_MS, resolveRequestTimeoutMs } from "./client.js"; import { CallError, indexedTaggedUnion, Result as ScaleResult, str, _void } from "./scale.js"; import type { Codec } from "./scale.js"; import { createClient, SubscriptionError } from "./generated/client.js"; import * as T from "./generated/types.js"; import * as W from "./generated/wire-table.js"; -import { encodeWireMessage } from "./transport.js"; +import { + createMessagePortProvider, + decodeWireMessage, + encodeWireMessage, + RequestTimeoutError, +} from "./transport.js"; /** Wrap a codec in the `{ V1: [0, codec] }` indexed-tagged-union envelope. */ const versionedV1 = (codec: Codec) => indexedTaggedUnion({ V1: [0, codec] }); +/** + * Request methods the host answers without waiting on a person or a chain: RPC + * reads, local storage, chat and notification writes, and the payment surfaces + * hosts answer as unsupported. Adding a generated request method to neither + * this set nor `REQUEST_TIMEOUT_FLOOR_MS` fails the classification test below. + */ +const PROMPT_FREE_REQUESTS = new Set([ + "SYSTEM_HANDSHAKE", + "SYSTEM_FEATURE_SUPPORTED", + "SYSTEM_NAVIGATE_TO", + "NOTIFICATIONS_SEND_PUSH_NOTIFICATION", + "NOTIFICATIONS_CANCEL_PUSH_NOTIFICATION", + "LOCAL_STORAGE_READ", + "LOCAL_STORAGE_WRITE", + "LOCAL_STORAGE_CLEAR", + "ACCOUNT_GET_LEGACY_ACCOUNTS", + "ACCOUNT_GET_USER_ID", + "CHAT_CREATE_ROOM", + "CHAT_REGISTER_BOT", + "CHAT_POST_MESSAGE", + "CHAIN_GET_HEAD_HEADER", + "CHAIN_GET_HEAD_BODY", + "CHAIN_GET_HEAD_STORAGE", + "CHAIN_CALL_HEAD", + "CHAIN_UNPIN_HEAD", + "CHAIN_CONTINUE_HEAD", + "CHAIN_STOP_HEAD_OPERATION", + "CHAIN_GET_SPEC_GENESIS_HASH", + "CHAIN_GET_SPEC_CHAIN_NAME", + "CHAIN_GET_SPEC_PROPERTIES", + "CHAIN_GET_CHAIN_INFO", + "CHAIN_BROADCAST_TRANSACTION", + "CHAIN_STOP_TRANSACTION", + "ENTROPY_DERIVE", + "COIN_PAYMENT_CREATE_PURSE", + "COIN_PAYMENT_QUERY_PURSE", + "COIN_PAYMENT_CREATE_RECEIVABLE", + "COIN_PAYMENT_CREATE_CHEQUE", +]); + +/** + * Await a promise-like and return its outcome, so a rejection can be asserted + * on instead of escaping the test. + */ +function settled(promise: PromiseLike): Promise { + return Promise.resolve(promise).then( + (value: unknown) => value, + (error: unknown) => error, + ); +} + function toHex(u: Uint8Array): string { return Array.from(u) .map((b) => b.toString(16).padStart(2, "0")) @@ -671,3 +727,202 @@ describe("generated client transport", () => { expect(errors[0].cause).toBe(providerError); }); }); + +describe("request timeouts", () => { + it("rejects a request the peer accepts and never answers", async () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider, { requestTimeoutMs: 5 }); + const client = createClient(transport); + + const outcome = await settled(client.system.handshake()); + + expect(fixture.sent).toHaveLength(1); + expect(outcome).toBeInstanceOf(RequestTimeoutError); + expect((outcome as RequestTimeoutError).timeoutMs).toBe(5); + }); + + it("ignores a reply that arrives after the bound fired", async () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider, { requestTimeoutMs: 5 }); + let decodeCalls = 0; + + const outcome = await settled( + transport.request({ + ids: W.SYSTEM_HANDSHAKE, + payload: T.VersionedHostHandshakeRequest.enc({ + tag: "V1", + value: { codecVersion: 1 }, + }), + decodeResponse: () => { + decodeCalls += 1; + return { success: true, value: undefined }; + }, + }), + ); + expect(outcome).toBeInstanceOf(RequestTimeoutError); + + const sentRequestId = unwrap( + decodeWireMessage(fixture.sent[0]), + "decode the sent request frame", + ).requestId; + + const lateReply = unwrap( + encodeWireMessage({ + requestId: sentRequestId, + payload: { + id: W.SYSTEM_HANDSHAKE.response, + value: handshakeResponsePayload({ success: true, value: undefined }), + }, + }), + "encode late handshake_response", + ); + expect(() => fixture.receive(lateReply)).not.toThrow(); + expect(decodeCalls).toBe(0); + }); + + it("bounds a request buffered by a provider whose port never resolves", async () => { + const { promise: unresolvedPort } = Promise.withResolvers(); + const provider = createMessagePortProvider(unresolvedPort); + const transport = createTransport(provider, { requestTimeoutMs: 5 }); + const client = createClient(transport); + + const outcome = await settled(client.system.handshake()); + + expect(outcome).toBeInstanceOf(RequestTimeoutError); + }); + + it("keeps a slow-answering method on its floor instead of the configured bound", async () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider, { requestTimeoutMs: 5 }); + + // Ordering, not wall-clock: the floored request carries a 420s bound, so + // the 5ms control request must settle first. Racing the two keeps the + // assertion deterministic without a guessed sleep. + const floored = transport + .request({ + ids: W.RESOURCE_ALLOCATION_REQUEST, + payload: new Uint8Array(), + decodeResponse: () => ({ success: true, value: undefined }), + }) + .then( + () => "floored", + () => "floored", + ); + const control = transport + .request({ + ids: W.SYSTEM_HANDSHAKE, + payload: T.VersionedHostHandshakeRequest.enc({ + tag: "V1", + value: { codecVersion: 1 }, + }), + decodeResponse: () => ({ success: true, value: undefined }), + }) + .then( + () => "control", + () => "control", + ); + + expect(await Promise.race([floored, control])).toBe("control"); + expect(fixture.sent).toHaveLength(2); + transport.dispose(); + expect(await floored).toBe("floored"); + }); + + it("rejects with the close error, not a timeout, when the transport is disposed", async () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider, { requestTimeoutMs: 5_000 }); + const client = createClient(transport); + + const response = client.system.handshake(); + transport.dispose(); + + const outcome = await settled(response); + expect(outcome).toBeInstanceOf(Error); + expect(outcome).not.toBeInstanceOf(RequestTimeoutError); + expect((outcome as Error).message).toBe("transport disposed"); + }); + + it("surfaces a timeout as a rejection that neither match callback sees", async () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider, { requestTimeoutMs: 5 }); + const client = createClient(transport); + let okCalls = 0; + let errCalls = 0; + + const outcome = await settled( + client.system.handshake().match( + () => { + okCalls += 1; + }, + () => { + errCalls += 1; + }, + ), + ); + + expect(outcome).toBeInstanceOf(RequestTimeoutError); + expect(okCalls).toBe(0); + expect(errCalls).toBe(0); + }); + + it("rejects a request bound that setTimeout cannot schedule", () => { + const fixture = providerFixture(); + + expect(() => createTransport(fixture.provider, { requestTimeoutMs: Infinity })).toThrow( + /Invalid TrUAPI request timeout/, + ); + expect(() => createTransport(fixture.provider, { requestTimeoutMs: 0 })).toThrow( + /Invalid TrUAPI request timeout/, + ); + expect(() => + createTransport(fixture.provider, { requestTimeoutMs: 2_147_483_648 }), + ).toThrow(/Invalid TrUAPI request timeout/); + }); + + it("keeps a configured bound that is longer than the method's floor", () => { + expect(resolveRequestTimeoutMs(W.SIGNING_SIGN_PAYLOAD.request, 500_000, undefined)).toBe( + 500_000, + ); + expect(resolveRequestTimeoutMs(W.SIGNING_SIGN_PAYLOAD.request, 5, undefined)).toBe(190_000); + expect(resolveRequestTimeoutMs(W.SYSTEM_HANDSHAKE.request, 5, undefined)).toBe(5); + }); + + it("lets a per-request bound override both the configured bound and the floor", () => { + expect(resolveRequestTimeoutMs(W.SIGNING_SIGN_PAYLOAD.request, 500_000, 5)).toBe(5); + expect(resolveRequestTimeoutMs(W.SYSTEM_HANDSHAKE.request, 5, 90_000)).toBe(90_000); + expect(() => resolveRequestTimeoutMs(W.SYSTEM_HANDSHAKE.request, 5, 0)).toThrow( + /Invalid TrUAPI request timeout/, + ); + }); + + it("rejects an invalid per-request bound at the call site, not through the promise", () => { + const fixture = providerFixture(); + const transport = createTransport(fixture.provider); + + expect(() => + transport.request({ + ids: W.SYSTEM_HANDSHAKE, + payload: new Uint8Array(), + decodeResponse: () => ({ success: true, value: undefined }), + timeoutMs: -1, + }), + ).toThrow(/Invalid TrUAPI request timeout/); + expect(fixture.sent).toHaveLength(0); + }); + + it("classifies every generated request method as floored or prompt-free", () => { + const unclassified = Object.entries(W) + .filter(([name, ids]) => { + if (!(ids && typeof ids === "object" && "request" in ids)) return false; + const requestId = ids.request; + return ( + typeof requestId === "number" && + !REQUEST_TIMEOUT_FLOOR_MS.has(requestId) && + !PROMPT_FREE_REQUESTS.has(name) + ); + }) + .map(([name]) => name); + + expect(unclassified).toEqual([]); + }); +}); diff --git a/js/packages/truapi/src/client.ts b/js/packages/truapi/src/client.ts index 8265b465b..456da50fa 100644 --- a/js/packages/truapi/src/client.ts +++ b/js/packages/truapi/src/client.ts @@ -8,6 +8,7 @@ import { type ProtocolMessage, type RegisterHostInitiatedSubscriptionParams, type RequestFrameIds, + RequestTimeoutError, type RequestParams, type SubscriptionFrameIds, type SubscribeRawParams, @@ -42,6 +43,152 @@ export interface CreateTransportOptions { * `TRUAPI_CODEC_VERSION` directly. */ codecVersion?: number; + + /** + * Bound applied to every request issued through this transport, in + * milliseconds. Must be an integer between 1 and 2147483647; there is no + * value that disables the bound. Defaults to `DEFAULT_REQUEST_TIMEOUT_MS`. + * Methods the host answers more slowly take the larger of this bound and + * their own floor. + */ + requestTimeoutMs?: number; +} + +/** + * Largest delay `setTimeout` schedules faithfully. Above it, and for `Infinity` + * or `NaN`, timers fire almost immediately, which would reject every request. + */ +const MAX_REQUEST_TIMEOUT_MS = 2_147_483_647; + +/** + * Default bound for one request. + * + * 30s is this codebase's UI-grade budget: the package waits 20s for a + * host-injected message port, and the playground bounds prompt-backed protocol + * calls at 30s. A product that wants a tighter or looser bound sets + * `requestTimeoutMs`. + */ +const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; + +/** + * Floor for a request the host answers behind a remote authority, above the + * runtime's 180s remote-authority response deadline + * (`rust/crates/truapi-server/src/runtime.rs`, + * `DEFAULT_REMOTE_AUTHORITY_RESPONSE_TIMEOUT`). + */ +const REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS = 190_000; + +/** + * Floor for a request whose answer waits on a person and carries no host-side + * deadline at all: a pairing login, a device or remote consent dialog, a + * payment confirmation. The host keeps such a call pending for as long as the + * person takes, so this is the client's own ceiling rather than a cleared host + * deadline, and it matches the longest client-side budget this repo already + * uses for a prompt-backed call. + */ +const USER_APPROVAL_REQUEST_TIMEOUT_MS = 420_000; + +/** + * Floor for a request that waits on a live resource allocation or an on-chain + * preimage, above the runtime's 300s allocation cap and 360s preimage cap. + */ +const LIVE_ALLOCATION_REQUEST_TIMEOUT_MS = 420_000; + +/** + * Requests whose answer either outlives `DEFAULT_REQUEST_TIMEOUT_MS` under a + * host deadline or waits on a person with no host deadline at all, keyed by + * request frame id. The effective bound is the larger of the configured bound + * and the floor, so bounding a request never aborts an answer the host is + * still allowed to send. A method absent from this table takes the configured + * bound; a per-request `timeoutMs` overrides both. + * + * Every request frame id is classified here or as prompt-free in + * `client.test.ts`, so a generated method added without a floor fails that + * test rather than silently inheriting the default. + */ +export const REQUEST_TIMEOUT_FLOOR_MS: ReadonlyMap = new Map([ + [W.ACCOUNT_GET_ACCOUNT.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.ACCOUNT_GET_ACCOUNT_ALIAS.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.ACCOUNT_CREATE_ACCOUNT_PROOF.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.ACCOUNT_SIGN_VRF.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [ + W.ACCOUNT_REGISTER_RING_VRF_KEY.request, + REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS, + ], + [W.ACCOUNT_LIST_RING_VRF_KEYS.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.ACCOUNT_RING_VRF_SIGN.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.SIGNING_SIGN_PAYLOAD.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [W.SIGNING_SIGN_RAW.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [ + W.SIGNING_SIGN_PAYLOAD_WITH_LEGACY_ACCOUNT.request, + REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS, + ], + [ + W.SIGNING_SIGN_RAW_WITH_LEGACY_ACCOUNT.request, + REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS, + ], + [W.SIGNING_CREATE_TRANSACTION.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [ + W.SIGNING_CREATE_TRANSACTION_WITH_LEGACY_ACCOUNT.request, + REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS, + ], + [W.STATEMENT_STORE_CREATE_PROOF.request, REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS], + [ + W.STATEMENT_STORE_CREATE_PROOF_AUTHORIZED.request, + REMOTE_AUTHORITY_REQUEST_TIMEOUT_MS, + ], + [W.ACCOUNT_REQUEST_LOGIN.request, USER_APPROVAL_REQUEST_TIMEOUT_MS], + [ + W.PERMISSIONS_REQUEST_DEVICE_PERMISSION.request, + USER_APPROVAL_REQUEST_TIMEOUT_MS, + ], + [ + W.PERMISSIONS_REQUEST_REMOTE_PERMISSION.request, + USER_APPROVAL_REQUEST_TIMEOUT_MS, + ], + [W.PAYMENT_REQUEST.request, USER_APPROVAL_REQUEST_TIMEOUT_MS], + [W.PAYMENT_TOP_UP.request, USER_APPROVAL_REQUEST_TIMEOUT_MS], + [W.RESOURCE_ALLOCATION_REQUEST.request, LIVE_ALLOCATION_REQUEST_TIMEOUT_MS], + [W.PREIMAGE_SUBMIT.request, LIVE_ALLOCATION_REQUEST_TIMEOUT_MS], + [W.STATEMENT_STORE_SUBMIT.request, LIVE_ALLOCATION_REQUEST_TIMEOUT_MS], +]); + +/** + * Validate a caller-supplied request bound, rejecting the values `setTimeout` + * would silently collapse into an immediate fire. + */ +function checkRequestTimeoutMs(value: number): number { + if ( + !Number.isSafeInteger(value) || + value < 1 || + value > MAX_REQUEST_TIMEOUT_MS + ) { + throw new Error( + `Invalid TrUAPI request timeout: ${value}. Expected an integer between 1 and ${MAX_REQUEST_TIMEOUT_MS}.`, + ); + } + return value; +} + +/** + * Resolve the bound one request is armed with: a per-request `timeoutMs` wins + * outright, otherwise the larger of the transport's bound and the method's + * floor, so a product that deliberately configures a long bound keeps it and + * one that configures a short bound still cannot abort an answer the host is + * allowed to send. + */ +export function resolveRequestTimeoutMs( + requestFrameId: number, + transportTimeoutMs: number, + perRequestTimeoutMs: number | undefined, +): number { + if (perRequestTimeoutMs !== undefined) { + return checkRequestTimeoutMs(perRequestTimeoutMs); + } + return Math.max( + transportTimeoutMs, + REQUEST_TIMEOUT_FLOOR_MS.get(requestFrameId) ?? 0, + ); } /** @@ -154,6 +301,10 @@ export function createTransport( options: CreateTransportOptions = {}, ): TrUApiTransport { const codecVersion = options.codecVersion ?? TRUAPI_CODEC_VERSION; + const requestTimeoutMs = + options.requestTimeoutMs === undefined + ? DEFAULT_REQUEST_TIMEOUT_MS + : checkRequestTimeoutMs(options.requestTimeoutMs); let idCounter = 0; let closedError: Error | null = null; const pending = new Map< @@ -162,6 +313,7 @@ export function createTransport( ids: RequestFrameIds; resolve: (value: Uint8Array) => void; reject: (error: Error) => void; + timer: ReturnType; } >(); const subscriptions = new Map< @@ -193,6 +345,19 @@ export function createTransport( return error instanceof Error ? error : new Error(String(error)); } + /** + * Remove a pending request and cancel its timeout timer. Every settle path + * goes through here, so a settled request never leaves a live timer and a + * frame arriving after the bound fired finds no entry to resolve. + */ + function takePending(requestId: string) { + const entry = pending.get(requestId); + if (!entry) return undefined; + pending.delete(requestId); + clearTimeout(entry.timer); + return entry; + } + /** * Close the transport once, rejecting pending requests and notifying live * subscriptions. @@ -206,7 +371,7 @@ export function createTransport( closedError = nextError; for (const [requestId, entry] of pending) { - pending.delete(requestId); + takePending(requestId); entry.reject(nextError); } @@ -302,7 +467,7 @@ export function createTransport( if (payload.id !== p.ids.response) { return; } - pending.delete(requestId); + takePending(requestId); try { p.resolve(payload.value); } catch (error) { @@ -447,7 +612,13 @@ export function createTransport( ids, payload, decodeResponse, + timeoutMs, }: RequestParams): ResultAsync { + const bound = resolveRequestTimeoutMs( + ids.request, + requestTimeoutMs, + timeoutMs, + ); const promise = new Promise>((resolve, reject) => { if (closedError) { reject(closedError); @@ -455,10 +626,15 @@ export function createTransport( } const requestId = `p:${++idCounter}`; + const timer = setTimeout(() => { + takePending(requestId); + reject(new RequestTimeoutError(bound)); + }, bound); pending.set(requestId, { ids, resolve: (response) => resolve(decodeResponse(response)), reject, + timer, }); try { send({ @@ -469,7 +645,7 @@ export function createTransport( }, }); } catch (error) { - pending.delete(requestId); + takePending(requestId); reject(toError(error)); } }); @@ -543,7 +719,9 @@ export function createTransport( bufferCapacity, }: RegisterHostInitiatedSubscriptionParams) { if (hostRoutes.has(ids.start)) { - throw new Error(`host-initiated subscription ${ids.start} is already registered`); + throw new Error( + `host-initiated subscription ${ids.start} is already registered`, + ); } const route: HostRoute = { ids, diff --git a/js/packages/truapi/src/index.ts b/js/packages/truapi/src/index.ts index a631ba2ea..e680ce4d2 100644 --- a/js/packages/truapi/src/index.ts +++ b/js/packages/truapi/src/index.ts @@ -14,6 +14,7 @@ export type { } from "./transport.js"; export type { CreateTransportOptions } from "./client.js"; export { + RequestTimeoutError, SubscriptionError, createIframeProvider, createMessagePortProvider, diff --git a/js/packages/truapi/src/transport.ts b/js/packages/truapi/src/transport.ts index 65e3fcdbc..9cd20d940 100644 --- a/js/packages/truapi/src/transport.ts +++ b/js/packages/truapi/src/transport.ts @@ -54,6 +54,27 @@ export class SubscriptionError extends Error { } } +/** + * Rejection delivered when a request outlives its bound: the peer accepted the + * frame and sent no response while the channel stayed open. + * + * Distinct from the plain `Error` a transport or provider close rejects with, + * so a caller can tell "host went silent, retry" from "channel is gone, + * re-establish". Discriminate with `error instanceof RequestTimeoutError`. + **/ +export class RequestTimeoutError extends Error { + /** + * Bound, in milliseconds, that the request outlived. + **/ + readonly timeoutMs: number; + + constructor(timeoutMs: number) { + super(`TrUAPI request timed out after ${timeoutMs}ms`); + this.name = "RequestTimeoutError"; + this.timeoutMs = timeoutMs; + } +} + /** * Minimal Observable-compatible observer shape used by generated subscription * APIs without depending on RxJS. @@ -176,6 +197,13 @@ export interface RequestParams { * envelope. The transport unwraps the envelope into `ResultAsync`. **/ decodeResponse: (payload: Uint8Array) => ResultPayload; + + /** + * Bound for this one request, in milliseconds, overriding both the + * transport's `requestTimeoutMs` and the per-method floor a slow-answering + * method would otherwise take. Must be an integer between 1 and 2147483647. + **/ + timeoutMs?: number; } /**