Skip to content

Protocol request timeout starts after the frame-ready wait, not at the call #166

Description

@ryanleecode

Problem Statement

postRequest applies a per-method timeout, 30 seconds by default, and callers reasonably read that as the bound on the call. The timer is only created after it awaits the shared protocol iframe becoming ready, and that wait carries its own 240-second budget. So the first request after a cold or wedged protocol frame can block for roughly four and a half minutes while reporting a 30-second contract, and the eventual rejection does not say whether the time went into booting the frame or waiting for a reply.

Goal

A protocol request rejects within its own documented per-method budget measured from the moment it is called, including any time spent waiting for the host or protocol frame to become ready.

Evidence: the await precedes the timer

packages/protocol/src/client.ts:497-521

async function postRequest<M extends ProtocolRequestMethod>(
  method: M,
  payload: ProtocolRequestMap[M],
  onProgress?: (message: string) => void,
  needsProtocolReady = !isSharedAuthRequestMethod(method) &&
    !isSharedModeRequestMethod(method),
): Promise<unknown> {
  await (needsProtocolReady ? ensureProtocolFrame() : ensureHostFrame());
  // ...
  const timeoutMs = UNTIMED_METHODS.has(method)
    ? null
    : (METHOD_TIMEOUTS[method] ?? DEFAULT_TIMEOUT_MS);

The setTimeout that enforces timeoutMs is created at :528, inside the new Promise at :524 — after the await on line 504 has already returned.

Evidence: the budgets that stack

packages/protocol/src/client.ts:317, :321, :481, :487-488

const IFRAME_LOAD_TIMEOUT_MS = 30_000;
const IFRAME_READY_TIMEOUT_MS = 240_000;   // must exceed TIMEOUTS.SHARED_WORKER_READY
const DEFAULT_TIMEOUT_MS = 30_000;
const UNTIMED_METHODS: ReadonlySet<ProtocolRequestMethod> =
  new Set<ProtocolRequestMethod>(["warmup"]);

Worst case for a needsProtocolReady method is the 240s ready wait plus the 30s request budget. warmup is the only method exempted from a request timeout, so no account, session, or auth method is exempt by design.

Orientation

  • packages/protocol/src/client.tspostRequest, ensureProtocolFrame, ensureHostFrame, the ready-waiter list pendingReadyResolvers (:440), and the load/ready rejections at :352-353 and :437-438.
  • There is already a fast-fail path that rejects ready-waiters immediately when the chain is known dead rather than letting them run the full budget (:144-145, :242-246) — the same idea, applied to one cause.
  • Shared-auth and shared-mode methods pass needsProtocolReady=false and await only ensureHostFrame(), so they are exposed to the 30s load bound rather than the 240s one. Both paths need accounting.
  • METHOD_TIMEOUTS (:489) already carries per-method overrides, e.g. chainConnect: 30_000.

Non-Counting Outcomes

  • Shrinking IFRAME_READY_TIMEOUT_MS instead of making the request budget cover the wait. The comment at :318-321 says that budget must exceed TIMEOUTS.SHARED_WORKER_READY, so cutting it breaks a legitimate cold boot to make a metric look better.
  • Starting the timer before the await but leaving the ensure path to reject with its own unrelated error, so a caller still cannot attribute the elapsed time.
  • Fixing only the needsProtocolReady=true branch and leaving shared-auth and shared-mode requests unaccounted, since those are exactly the session and preference reads that run at boot.
  • A test using fake timers that asserts rejection after the budget without ever driving a frame that is slow to signal ready — it verifies arithmetic, not the ordering defect.
  • Making warmup timed to simplify the change; :482-486 gives a stated reason it is exempt.

Acceptance Criteria

  • (gatekeeper) A test drives a protocol frame that never signals ready and asserts a non-warmup request rejects within its per-method budget measured from call time; it fails when the change is reverted. Name the test file in packages/protocol.
  • pnpm test in packages/protocol exits 0.
  • The rejection distinguishes "budget spent waiting for the frame to become ready" from "budget spent waiting for a reply".
  • Both branches of needsProtocolReady are covered by tests.
  • warmup remains exempt from a request timeout.
  • No existing timeout constant is reduced to satisfy the bound.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions