From f7a4a145c4613e8cc15ca93b59290f5b0396821b Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Thu, 6 Aug 2026 17:23:14 -0500 Subject: [PATCH 1/6] Add RPC error classifier for DO resets and connection failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reads the structured flags enhanced_error_serialization already delivers to the browser (durableObjectReset, retryable, overloaded, durableObjectId; semantics per workerd jsg/util.c++ — see MR 238), with message matching as fallback. Nothing consumed these before: every call site treated a transient DO reset like a terminal error. --- .../workshop-frontend/src/rpcErrors.test.ts | 99 +++++++++++++++++++ packages/workshop-frontend/src/rpcErrors.ts | 64 ++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 packages/workshop-frontend/src/rpcErrors.test.ts create mode 100644 packages/workshop-frontend/src/rpcErrors.ts diff --git a/packages/workshop-frontend/src/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts new file mode 100644 index 00000000..37b4210c --- /dev/null +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('./errorReporting', () => ({ reportIssue: vi.fn() })) + +import { reportIssue } from './errorReporting' +import { + classifyRpcError, getDurableObjectId, isDurableObjectResetError, isOverloadedError, + isTransientRpcError, reportDoResetError, +} from './rpcErrors' + +// The reject frame observed in prod for a DO storage-timeout reset. +function storageTimeoutReset() { + return Object.assign( + new Error('Durable Object storage operation exceeded timeout which caused object to be reset.'), + { remote: true, overloaded: true, durableObjectReset: true, durableObjectId: 'eed0859e' }, + ) +} + +describe('classifyRpcError', () => { + it('classifies reset flags as do-reset', () => { + expect(classifyRpcError(storageTimeoutReset())).toBe('do-reset') + }) + + it('trusts the durableObjectReset flag over an unrecognized message', () => { + const err = Object.assign(new Error('internal error'), { durableObjectReset: true }) + expect(classifyRpcError(err)).toBe('do-reset') + }) + + it('falls back to known reset messages without flags', () => { + for (const message of [ + 'Durable Object reset because its code was updated.', + "Durable Object's isolate exceeded its memory limit and was reset.", + 'Durable Object exceeded its CPU time limit and was reset.', + ]) { + expect(classifyRpcError(new Error(message))).toBe('do-reset') + } + }) + + it('prefers do-reset when both reset and retryable flags are set', () => { + const err = Object.assign(new Error('x'), { durableObjectReset: true, retryable: true }) + expect(classifyRpcError(err)).toBe('do-reset') + }) + + it('classifies the retryable flag as connection', () => { + expect(classifyRpcError(Object.assign(new Error('x'), { retryable: true }))).toBe('connection') + }) + + it('classifies capnweb transport messages as connection', () => { + expect(classifyRpcError(new Error('Peer closed WebSocket: 1006 '))).toBe('connection') + expect(classifyRpcError(new Error('WebSocket connection failed.'))).toBe('connection') + expect(classifyRpcError(new Error('RPC session was shut down by disposing the main stub'))) + .toBe('connection') + }) + + it('classifies auth failures, which must never be retried or quieted', () => { + expect(classifyRpcError(new Error('invalid session token'))).toBe('auth') + expect(classifyRpcError(new Error('Not authenticated with Access.'))).toBe('auth') + }) + + it('classifies everything else as other', () => { + expect(classifyRpcError(new Error('Workspace not found.'))).toBe('other') + expect(classifyRpcError('boom')).toBe('other') + expect(classifyRpcError(null)).toBe('other') + expect(classifyRpcError(undefined)).toBe('other') + }) +}) + +describe('isTransientRpcError', () => { + it('is true for do-reset and connection, false otherwise', () => { + expect(isTransientRpcError(storageTimeoutReset())).toBe(true) + expect(isTransientRpcError(new Error('Peer closed WebSocket: 1006 '))).toBe(true) + expect(isTransientRpcError(new Error('invalid session token'))).toBe(false) + expect(isTransientRpcError(new Error('Workspace not found.'))).toBe(false) + }) +}) + +describe('flag accessors', () => { + it('reads reset, overload, and DO id from the enriched error', () => { + const err = storageTimeoutReset() + expect(isDurableObjectResetError(err)).toBe(true) + expect(isOverloadedError(err)).toBe(true) + expect(getDurableObjectId(err)).toBe('eed0859e') + }) + + it('handles errors without flags', () => { + expect(isOverloadedError(new Error('x'))).toBe(false) + expect(getDurableObjectId(new Error('x'))).toBeUndefined() + expect(getDurableObjectId(null)).toBeUndefined() + }) +}) + +describe('reportDoResetError', () => { + it('forwards to reportIssue with a namespaced site', () => { + const err = storageTimeoutReset() + reportDoResetError('chat.send', err, { gadgetId: 'g1' }) + expect(reportIssue).toHaveBeenCalledWith('do-reset.chat.send', err, + { severity: 'warning', handled: true, gadgetId: 'g1' }) + }) +}) diff --git a/packages/workshop-frontend/src/rpcErrors.ts b/packages/workshop-frontend/src/rpcErrors.ts new file mode 100644 index 00000000..b50475b3 --- /dev/null +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -0,0 +1,64 @@ +import { reportIssue } from './errorReporting' + +// Classifies errors surfaced through capnweb RPC. The backend runs with +// `enhanced_error_serialization`, so remote failures carry structured flags (workerd +// jsg/util.c++): `retryable` ⇔ the connection was lost, `overloaded` ⇔ the target pushed +// back, `durableObjectReset` ⇔ the target Durable Object was reset. Flags are authoritative; +// message matching is a fallback for errors that lose them in transit. + +export type RpcErrorClass = 'do-reset' | 'connection' | 'auth' | 'other' + +const DO_RESET_MESSAGES = [ + 'Durable Object reset because its code was updated', + 'Durable Object storage operation exceeded timeout', + "Durable Object's isolate exceeded its memory limit", + 'Durable Object exceeded its CPU time limit', +] + +// Transport failures raised locally by capnweb, plus its own-session teardown message. +const CONNECTION_MESSAGES = [ + 'Peer closed WebSocket', + 'WebSocket connection failed.', + 'RPC session was shut down by disposing the main stub', +] + +const AUTH_MESSAGES = ['invalid session token', 'Not authenticated with Access'] + +const messageOf = (err: unknown) => (err instanceof Error ? err.message : String(err)) + +const flag = (err: unknown, name: string) => + (err as Record | null | undefined)?.[name] === true + +export function isDurableObjectResetError(err: unknown): boolean { + return flag(err, 'durableObjectReset') || DO_RESET_MESSAGES.some(m => messageOf(err).includes(m)) +} + +export function isOverloadedError(err: unknown): boolean { + return flag(err, 'overloaded') +} + +export function getDurableObjectId(err: unknown): string | undefined { + const id = (err as { durableObjectId?: unknown } | null | undefined)?.durableObjectId + return typeof id === 'string' ? id : undefined +} + +export function classifyRpcError(err: unknown): RpcErrorClass { + if (isDurableObjectResetError(err)) return 'do-reset' + const message = messageOf(err) + if (flag(err, 'retryable') || CONNECTION_MESSAGES.some(m => message.includes(m))) { + return 'connection' + } + if (AUTH_MESSAGES.some(m => message.includes(m))) return 'auth' + return 'other' +} + +// True for failures that a healthy retry or reconnect is expected to cure. +export function isTransientRpcError(err: unknown): boolean { + const cls = classifyRpcError(err) + return cls === 'do-reset' || cls === 'connection' +} + +/** Reports a DO-reset error to the client-errors endpoint (no-op unless reporting is enabled). */ +export function reportDoResetError(site: string, err: unknown, options?: { gadgetId?: string }) { + reportIssue(`do-reset.${site}`, err, { severity: 'warning', handled: true, ...options }) +} From 364d109c6d441c1bab783b4cdf24af01c4099456 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Thu, 6 Aug 2026 17:40:28 -0500 Subject: [PATCH 2/6] Retry idempotent reads once after a Durable Object reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reset DO rejects in-flight RPCs while the WebSocket stays healthy, and nothing ever re-fetched — the sidebar, model list, onboarding check, vendor branding, and connected-account subscriptions stayed broken until reload. The object restarts on its next request, so withDoResetRetry retries once after a jittered delay. Reads only; writes are never retried (a reset after commit would double-apply). --- .../__integration__/open-gadget-rpc.test.ts | 24 +++++++ packages/workshop-backend/src/server.ts | 13 +++- .../src/BlueprintLandingPage.tsx | 3 +- .../workshop-frontend/src/GatekeeperModal.tsx | 3 +- .../src/ObserverConfigModal.tsx | 9 +-- .../src/OnboardingWizard.tsx | 8 +-- .../src/ResourcePicker.test.tsx | 62 +++++++++++++++++++ .../workshop-frontend/src/ResourcePicker.tsx | 11 +++- .../components/AppShell/SidebarWorkspaces.tsx | 4 +- .../src/components/ConnectionChips.tsx | 3 +- .../workshop-frontend/src/routes/__root.tsx | 3 +- .../src/routes/gatekeepers.tsx | 10 ++- .../workshop-frontend/src/routes/index.tsx | 4 +- .../workshop-frontend/src/rpcErrors.test.ts | 37 ++++++++++- packages/workshop-frontend/src/rpcErrors.ts | 14 +++++ .../src/useVendorBranding.ts | 3 +- 16 files changed, 181 insertions(+), 30 deletions(-) create mode 100644 packages/workshop-frontend/src/ResourcePicker.test.tsx diff --git a/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts b/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts index 5df6aa53..fff93c25 100644 --- a/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts +++ b/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts @@ -1,3 +1,4 @@ +import { abortAllDurableObjects } from "cloudflare:test"; import { exports } from "cloudflare:workers"; import { newWebSocketRpcSession, type RpcStub } from "capnweb"; import { @@ -133,3 +134,26 @@ describe.skip("openGadget errors across native RPC and Cap'n Web", () => { expectRpcCode(browserError, OPEN_GADGET_ERROR_CODES.workspaceAccessDenied); }); }); + +// The DO-reset recovery contract: a user-DO reset must not poison the API session. A stub is +// bound to one incarnation of the object and is permanently broken by a reset, so the +// authenticated API re-resolves its user-DO stub per call — the same session recovers on the +// next request with no reconnect. abortAllDurableObjects() is the non-graceful teardown, the +// local stand-in for the storage-timeout/overload resets observed in production. (Deliberately +// not evictDurableObject(): eviction is graceful — it drains in-flight work and never breaks a +// stub — so it cannot reproduce this failure.) +describe("user-DO reset recovery", () => { + it("recovers on the same session after the user DO is reset", async () => { + using publicApi = await connect(); + const account = await createAccount(publicApi, "reset"); + using authenticated = await publicApi.authenticate(account.token); + + expect(await authenticated.listModels()).toBeInstanceOf(Array); + + await abortAllDurableObjects(); + + // Same socket, same AuthenticatedApiImpl. With the per-call stub getter this reaches the + // restarted object; with a session-cached stub it would reject with the abort error forever. + expect(await authenticated.listModels()).toBeInstanceOf(Array); + }); +}); diff --git a/packages/workshop-backend/src/server.ts b/packages/workshop-backend/src/server.ts index 6ffb384c..2f054fcc 100644 --- a/packages/workshop-backend/src/server.ts +++ b/packages/workshop-backend/src/server.ts @@ -74,7 +74,7 @@ type Env = Cloudflare.Env & { @validateRpc() class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { constructor(private ctx: ExecutionContext, private env: Env, - private user: DurableObjectStub, + private userId: DurableObjectId, private abortSession: (reason: Error) => void) { super(); @@ -87,6 +87,13 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { private adminSettings: DurableObjectNamespace; private users: DurableObjectNamespace; + // A stub is bound to one incarnation of the DO and is poisoned once that incarnation resets, + // so re-resolve per call instead of caching one for the session (per the DO error-handling + // docs: create a fresh stub per attempt). Stub creation is local and lazy — not a network call. + private get user(): DurableObjectStub { + return this.users.get(this.userId); + } + #isAdmin(): boolean { let name = this.user.id.name; let admins = this.env.ADMINS; @@ -675,7 +682,7 @@ class PublicApiImpl extends RpcTarget implements PublicApi { user_id: userId.toString(), source: "session_token", }); - return new AuthenticatedApiImpl(this.ctx, this.env, stub, this.abortSession); + return new AuthenticatedApiImpl(this.ctx, this.env, userId, this.abortSession); } async authenticateFromCfAccess(): Promise { @@ -700,7 +707,7 @@ class PublicApiImpl extends RpcTarget implements PublicApi { user_id: userId.toString(), source: "cf_access", }); - return new AuthenticatedApiImpl(this.ctx, this.env, stub, this.abortSession); + return new AuthenticatedApiImpl(this.ctx, this.env, userId, this.abortSession); } async login(username: string, passwordHash: Uint8Array): Promise { diff --git a/packages/workshop-frontend/src/BlueprintLandingPage.tsx b/packages/workshop-frontend/src/BlueprintLandingPage.tsx index 16226ae2..b501a345 100644 --- a/packages/workshop-frontend/src/BlueprintLandingPage.tsx +++ b/packages/workshop-frontend/src/BlueprintLandingPage.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useState, useEffect, useCallback, useMemo, useRef, type ReactNode } from 'react' import { useNavigate, useParams, useRouter } from '@tanstack/react-router' import { RpcStub, RpcTarget } from 'capnweb' @@ -181,7 +182,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) { ready() {} } - authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber()) + withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber())) .then(stub => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/GatekeeperModal.tsx b/packages/workshop-frontend/src/GatekeeperModal.tsx index 0706d33e..edcd5fb3 100644 --- a/packages/workshop-frontend/src/GatekeeperModal.tsx +++ b/packages/workshop-frontend/src/GatekeeperModal.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Dialog, useKumoToastManager, type PortalContainer } from '@cloudflare/kumo' import { @@ -431,7 +432,7 @@ export default function GatekeeperModal({ } const subscriber = new AccountsSubscriber() - authenticatedApi.subscribeConnectedAccounts(subscriber) + withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) .then(stub => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/ObserverConfigModal.tsx b/packages/workshop-frontend/src/ObserverConfigModal.tsx index 22f5755f..9a8d3ceb 100644 --- a/packages/workshop-frontend/src/ObserverConfigModal.tsx +++ b/packages/workshop-frontend/src/ObserverConfigModal.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef } from 'react' import { Dialog, Select, Loader, Text, useKumoToastManager } from '@cloudflare/kumo' import { Warning, Plus, ArrowClockwise, CheckCircle } from '@phosphor-icons/react' @@ -143,8 +144,8 @@ export default function ObserverConfigModal({ } } - authenticatedApi - .subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true }) + withDoResetRetry(() => authenticatedApi + .subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true })) .then(stub => { if (cancelled) { stub[Symbol.dispose](); return } subStub = stub @@ -163,10 +164,10 @@ export default function ObserverConfigModal({ // ── load vendor metadata for display and resource-scope resolution ───────────── useEffect(() => { let cancelled = false - Promise.all([ + withDoResetRetry(() => Promise.all([ authenticatedApi.listGatekeeperVendors(), authenticatedApi.listAddableGatekeepers(), - ]) + ])) .then(([vendors, addable]) => { if (cancelled) return const map = new Map() diff --git a/packages/workshop-frontend/src/OnboardingWizard.tsx b/packages/workshop-frontend/src/OnboardingWizard.tsx index 6e305c21..e0c3b5ff 100644 --- a/packages/workshop-frontend/src/OnboardingWizard.tsx +++ b/packages/workshop-frontend/src/OnboardingWizard.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef, useCallback } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' import { RpcTarget } from 'capnweb' @@ -119,10 +120,10 @@ export default function OnboardingWizard({ // Load models + AI config const fetchModels = useCallback(async () => { try { - const [modelList, cfg] = await Promise.all([ + const [modelList, cfg] = await withDoResetRetry(() => Promise.all([ authenticatedApi.listModels(), authenticatedApi.getAiConfig(), - ]) + ])) setModels(modelList) setAiConfig(cfg) // Default to the first model in the list @@ -220,8 +221,7 @@ export default function OnboardingWizard({ const subscriber = new AccountsSubscriber() let subscriptionStub: { [Symbol.dispose](): void } | null = null - authenticatedApi - .subscribeConnectedAccounts(subscriber) + withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/ResourcePicker.test.tsx b/packages/workshop-frontend/src/ResourcePicker.test.tsx new file mode 100644 index 00000000..ad77ee41 --- /dev/null +++ b/packages/workshop-frontend/src/ResourcePicker.test.tsx @@ -0,0 +1,62 @@ +// @vitest-environment jsdom +/* eslint-disable react/react-in-jsx-scope */ + +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RpcStub } from 'capnweb' +import type { AuthenticatedApi } from '@gadgets/workshop-shared/api' +import ResourcePicker from './ResourcePicker' + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +vi.mock('@cloudflare/kumo', () => ({ + Tooltip: ({ children }: { children: ReactNode }) => children, + useKumoToastManager: () => ({ add: vi.fn<(toast: unknown) => void>() }), +})) + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(next => { resolve = next }) + return { promise, resolve } +} + +describe('ResourcePicker', () => { + let root: Root | undefined + let container: HTMLDivElement | undefined + + afterEach(() => { + act(() => root?.unmount()) + container?.remove() + vi.restoreAllMocks() + }) + + it('disposes a connected-account subscription that resolves after unmount', async () => { + const pendingSubscription = deferred<{ [Symbol.dispose](): void }>() + const dispose = vi.fn<() => void>() + const authenticatedApi = { + subscribeConnectedAccounts: () => pendingSubscription.promise, + listGatekeeperVendors: async () => [], + } as unknown as RpcStub + + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + await act(async () => root!.render( + {}} + />, + )) + + act(() => root!.unmount()) + root = undefined + await act(async () => { + pendingSubscription.resolve({ [Symbol.dispose]: dispose }) + await Promise.resolve() + }) + + expect(dispose).toHaveBeenCalledOnce() + }) +}) diff --git a/packages/workshop-frontend/src/ResourcePicker.tsx b/packages/workshop-frontend/src/ResourcePicker.tsx index d7071989..27f2f01c 100644 --- a/packages/workshop-frontend/src/ResourcePicker.tsx +++ b/packages/workshop-frontend/src/ResourcePicker.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef, useMemo, useCallback, type MutableRefObject } from 'react' import { Tooltip, useKumoToastManager } from '@cloudflare/kumo' import { Plus, CaretRight, Warning } from '@phosphor-icons/react' @@ -111,6 +112,7 @@ export default function ResourcePicker({ // Subscribe to connected accounts on mount. useEffect(() => { + let cancelled = false seenAccountIdsRef.current = new Set() class AccountsSubscriber extends RpcTarget implements ConnectedAccountsSubscriber { @@ -157,17 +159,20 @@ export default function ResourcePicker({ const subscriber = new AccountsSubscriber() const subscribe = async () => { try { - const stub = await authenticatedApi.subscribeConnectedAccounts(subscriber) - subscriptionRef.current = { stub } + const stub = await withDoResetRetry( + () => authenticatedApi.subscribeConnectedAccounts(subscriber)) + if (cancelled) stub[Symbol.dispose]() + else subscriptionRef.current = { stub } } catch (error) { console.error('Failed to subscribe to connected accounts:', error) // Nothing more is coming, so show what we have rather than hiding forever. - setAccountsLoaded(true) + if (!cancelled) setAccountsLoaded(true) } } subscribe() return () => { + cancelled = true if (subscriptionRef.current) { subscriptionRef.current.stub[Symbol.dispose]() subscriptionRef.current = null diff --git a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx index b5c363c5..7a274103 100644 --- a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx +++ b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from '../../rpcErrors' import { createContext, useCallback, @@ -89,8 +90,7 @@ export function SidebarWorkspacesProvider({ children }: { children: ReactNode }) useEffect(() => { let cancelled = false setGadgetsLoading(true) - authenticatedApi - .listGadgets() + withDoResetRetry(() => authenticatedApi.listGadgets()) .then((list) => { if (cancelled) return setGadgets(list) diff --git a/packages/workshop-frontend/src/components/ConnectionChips.tsx b/packages/workshop-frontend/src/components/ConnectionChips.tsx index dec3b835..4af6daf8 100644 --- a/packages/workshop-frontend/src/components/ConnectionChips.tsx +++ b/packages/workshop-frontend/src/components/ConnectionChips.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from '../rpcErrors' import { Plus } from '@phosphor-icons/react' import { Link } from '@tanstack/react-router' import { useAuthenticatedApi } from '../AuthContext' @@ -45,7 +46,7 @@ export default function ConnectionChips() { } const subscriber = new ChipsSubscriber() - const subPromise = authenticatedApi.subscribeConnectedAccounts(subscriber) + const subPromise = withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) subPromise.then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/routes/__root.tsx b/packages/workshop-frontend/src/routes/__root.tsx index 97eade90..4ba8c9ea 100644 --- a/packages/workshop-frontend/src/routes/__root.tsx +++ b/packages/workshop-frontend/src/routes/__root.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from '../rpcErrors' import { useState, useEffect } from 'react' import { createRootRoute, Outlet, useRouterState } from '@tanstack/react-router' import { TooltipProvider, Toasty } from '@cloudflare/kumo' @@ -151,7 +152,7 @@ function AuthenticatedShell({ useEffect(() => { let cancelled = false - authenticatedApi.isOnboardingCompleted().then((completed) => { + withDoResetRetry(() => authenticatedApi.isOnboardingCompleted()).then((completed) => { if (!cancelled) setOnboardingNeeded(!completed) }).catch((err) => { console.error('Failed to check onboarding status:', err) diff --git a/packages/workshop-frontend/src/routes/gatekeepers.tsx b/packages/workshop-frontend/src/routes/gatekeepers.tsx index 85a22090..7b0a166e 100644 --- a/packages/workshop-frontend/src/routes/gatekeepers.tsx +++ b/packages/workshop-frontend/src/routes/gatekeepers.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from '../rpcErrors' import { createFileRoute } from '@tanstack/react-router' import { useEffect, useMemo, useRef, useState } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' @@ -482,8 +483,7 @@ function ConnectorsPage() { setAccountsLoaded(false) setVendorsLoaded(false) - authenticatedApi - .listAddableGatekeepers() + withDoResetRetry(() => authenticatedApi.listAddableGatekeepers()) .then((list) => { if (!cancelled) setAddable(list) }) @@ -491,8 +491,7 @@ function ConnectorsPage() { console.error('Failed to load addable gatekeepers:', err) }) - authenticatedApi - .listGatekeeperVendors() + withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()) .then((vendorList) => { if (cancelled) return const unavailable = vendorList.filter((v) => v.unavailable) @@ -552,8 +551,7 @@ function ConnectorsPage() { const subscriber = new AccountsSubscriber() - authenticatedApi - .subscribeConnectedAccounts(subscriber) + withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/routes/index.tsx b/packages/workshop-frontend/src/routes/index.tsx index 9506b75b..9cac5355 100644 --- a/packages/workshop-frontend/src/routes/index.tsx +++ b/packages/workshop-frontend/src/routes/index.tsx @@ -1,3 +1,4 @@ +import { withDoResetRetry } from "../rpcErrors"; import { useState, useEffect, useRef, useCallback } from "react"; import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { useKumoToastManager } from "@cloudflare/kumo"; @@ -57,8 +58,7 @@ export function HomePageContent({ prompt }: HomeSearch) { useEffect(() => { let cancelled = false; - authenticatedApi - .listModels() + withDoResetRetry(() => authenticatedApi.listModels()) .then((list) => { if (cancelled) return; setModels(list); diff --git a/packages/workshop-frontend/src/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts index 37b4210c..c5f79d49 100644 --- a/packages/workshop-frontend/src/rpcErrors.test.ts +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -5,7 +5,7 @@ vi.mock('./errorReporting', () => ({ reportIssue: vi.fn() })) import { reportIssue } from './errorReporting' import { classifyRpcError, getDurableObjectId, isDurableObjectResetError, isOverloadedError, - isTransientRpcError, reportDoResetError, + isTransientRpcError, reportDoResetError, withDoResetRetry, } from './rpcErrors' // The reject frame observed in prod for a DO storage-timeout reset. @@ -97,3 +97,38 @@ describe('reportDoResetError', () => { { severity: 'warning', handled: true, gadgetId: 'g1' }) }) }) + +describe('withDoResetRetry', () => { + it('retries once after a reset error', async () => { + vi.useFakeTimers() + try { + const fn = vi.fn().mockRejectedValueOnce(storageTimeoutReset()).mockResolvedValueOnce('ok') + const result = withDoResetRetry(fn) + await vi.advanceTimersByTimeAsync(2000) + expect(await result).toBe('ok') + expect(fn).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('does not retry non-reset errors', async () => { + const fn = vi.fn().mockRejectedValue(new Error('Workspace not found.')) + await expect(withDoResetRetry(fn)).rejects.toThrow('Workspace not found.') + expect(fn).toHaveBeenCalledTimes(1) + }) + + it('gives up after the second failure', async () => { + vi.useFakeTimers() + try { + const fn = vi.fn().mockRejectedValue(storageTimeoutReset()) + const result = withDoResetRetry(fn) + result.catch(() => {}) + await vi.advanceTimersByTimeAsync(2000) + await expect(result).rejects.toThrow('exceeded timeout') + expect(fn).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/packages/workshop-frontend/src/rpcErrors.ts b/packages/workshop-frontend/src/rpcErrors.ts index b50475b3..3f03ce49 100644 --- a/packages/workshop-frontend/src/rpcErrors.ts +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -58,6 +58,20 @@ export function isTransientRpcError(err: unknown): boolean { return cls === 'do-reset' || cls === 'connection' } +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + +// Retries an idempotent call once after a DO reset: the object restarts on its next request, +// so a single delayed attempt usually succeeds. Never use for writes. +export async function withDoResetRetry(fn: () => Promise, delayMs = 1500): Promise { + try { + return await fn() + } catch (err) { + if (!isDurableObjectResetError(err)) throw err + await sleep(delayMs * (0.75 + Math.random() * 0.5)) + return fn() + } +} + /** Reports a DO-reset error to the client-errors endpoint (no-op unless reporting is enabled). */ export function reportDoResetError(site: string, err: unknown, options?: { gadgetId?: string }) { reportIssue(`do-reset.${site}`, err, { severity: 'warning', handled: true, ...options }) diff --git a/packages/workshop-frontend/src/useVendorBranding.ts b/packages/workshop-frontend/src/useVendorBranding.ts index d9c39e7b..8b4bcbc2 100644 --- a/packages/workshop-frontend/src/useVendorBranding.ts +++ b/packages/workshop-frontend/src/useVendorBranding.ts @@ -1,3 +1,4 @@ +import { withDoResetRetry } from './rpcErrors' import { useEffect, useState } from 'react' import { RpcStub } from 'capnweb' import { AuthenticatedApi } from '@gadgets/workshop-shared/api' @@ -31,7 +32,7 @@ export function useVendorBranding( if (!promise) { const api = authenticatedApi promise = (async () => { - const vendors = await api.listGatekeeperVendors() + const vendors = await withDoResetRetry(() => api.listGatekeeperVendors()) const map = new Map() for (const vendor of vendors) { const { logo, color } = vendor.description From a29aba0a81d33cbf771886ca74c2d5d2782d59a6 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Thu, 6 Aug 2026 17:52:29 -0500 Subject: [PATCH 3/6] Quiet transient RPC errors and surface chat-send hiccups inline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transient failures (DO reset, connection loss) were logged as scary console errors with dead-end toasts at every load site. Route them through logRpcFailure — debug-level for transient, loud otherwise — and skip the toasts, since a reconnect or retry is expected to cure them. Failed chat sends now show an inline composer hint instead of a toast; the wording is hedged because a reset after commit means the message may have landed. DO resets on the send path report through reportDoResetError for telemetry. --- .../src/BlueprintLandingPage.tsx | 8 ++-- .../workshop-frontend/src/ChatInterface.tsx | 41 +++++++++++++++---- .../src/ConnectAccountModal.tsx | 3 +- .../workshop-frontend/src/Connections.tsx | 2 + .../workshop-frontend/src/GatekeeperModal.tsx | 8 ++-- .../src/ObserverConfigModal.tsx | 2 + .../src/OnboardingWizard.tsx | 4 +- .../workshop-frontend/src/ResourcePicker.tsx | 4 +- .../components/AppShell/SidebarWorkspaces.tsx | 4 +- .../src/homePromptFlow.test.tsx | 20 +++++++++ .../workshop-frontend/src/routes/__root.tsx | 4 +- .../src/routes/gatekeepers.tsx | 8 ++-- .../workshop-frontend/src/routes/index.tsx | 15 ++++--- .../src/routes/providers.tsx | 5 ++- .../workshop-frontend/src/rpcErrors.test.ts | 20 ++++++++- packages/workshop-frontend/src/rpcErrors.ts | 9 ++++ .../src/useVendorBranding.ts | 4 +- 17 files changed, 123 insertions(+), 38 deletions(-) diff --git a/packages/workshop-frontend/src/BlueprintLandingPage.tsx b/packages/workshop-frontend/src/BlueprintLandingPage.tsx index b501a345..b44c78eb 100644 --- a/packages/workshop-frontend/src/BlueprintLandingPage.tsx +++ b/packages/workshop-frontend/src/BlueprintLandingPage.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from './rpcErrors' +import { logRpcFailure, withDoResetRetry } from './rpcErrors' import { useState, useEffect, useCallback, useMemo, useRef, type ReactNode } from 'react' import { useNavigate, useParams, useRouter } from '@tanstack/react-router' import { RpcStub, RpcTarget } from 'capnweb' @@ -121,7 +121,9 @@ export default function BlueprintLandingPage({ rpcStub }: Props) { // When authenticated, fetch models for binding assignment. useEffect(() => { if (isAuthenticated && authenticatedApi) { - authenticatedApi.listModels().then(setModels).catch(console.error) + withDoResetRetry(() => authenticatedApi.listModels()) + .then(setModels) + .catch(err => logRpcFailure('Failed to load models:', err)) } else { setModels([]) } @@ -191,7 +193,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) { } }) .catch(err => { - console.error('Failed to subscribe to connected accounts:', err) + logRpcFailure('Failed to subscribe to connected accounts:', err) }) return () => { diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index d5dac25a..4036c284 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -1,3 +1,4 @@ +import { isDurableObjectResetError, isTransientRpcError, logRpcFailure, reportDoResetError } from "./rpcErrors"; import { Fragment, memo, @@ -1780,6 +1781,7 @@ export const ChatInput = ({ attachLabel, draftUpdateBanner, blockedReason, + chatKey, onStop, showThinkingTraces = true, onToggleThinkingTraces, @@ -1825,6 +1827,8 @@ export const ChatInput = ({ /** When set, the composer is disabled and shows this message — the user must resolve something * (e.g. accept/deny a pending connection request) before they can type or send. */ blockedReason?: string; + /** Identity of the chat the composer is bound to; a change clears chat-scoped hints. */ + chatKey?: number | null; onStop?: () => void; showThinkingTraces?: boolean; onToggleThinkingTraces?: () => void; @@ -1838,6 +1842,11 @@ export const ChatInput = ({ const [capsules, setCapsules] = useState([]); const [pendingAttachments, setPendingAttachments] = useState([]); const [isSending, setIsSending] = useState(false); + const [sendHiccup, setSendHiccup] = useState(false); + const chatKeyRef = useRef(chatKey); + chatKeyRef.current = chatKey; + + useEffect(() => setSendHiccup(false), [chatKey]); const [isAttachmentDragActive, setIsAttachmentDragActive] = useState(false); const [selectedSlashCommand, setSelectedSlashCommand] = useState(null); // The caret the slash command picker parses at. Deliberately updated only when it moves to a @@ -2276,6 +2285,7 @@ export const ChatInput = ({ const handleSend = async () => { if (sendInFlightRef.current || isSending || isBlocked) return; + setSendHiccup(false); const attachmentsSnapshot = pendingAttachments; const readyAttachments = attachmentsSnapshot .filter((attachment) => attachment.uploadState === "ready" && attachment.ref) @@ -2454,8 +2464,12 @@ export const ChatInput = ({ }; const submitMessage = () => { + const submittedChatKey = chatKey; void handleSend().catch((err) => { - console.error("Failed to send chat message:", err); + // The onSend handlers already log; the composer only needs the hint state. + if (isTransientRpcError(err) && chatKeyRef.current === submittedChatKey) { + setSendHiccup(true); + } }); }; @@ -3050,6 +3064,11 @@ export const ChatInput = ({ )} {draftUpdateBanner} + {sendHiccup && ( +
+ Connection hiccup — your message may not have been sent. Check the thread, then try again. +
+ )} {/* Textarea */}
{slashCommandPicker.popup} @@ -5213,9 +5232,10 @@ function ChatInterface({ forceUpdate(); } } catch (err) { - console.error("Failed to subscribe to chats:", err); - reportIssue('chat.subscription-load', err) - toasts.add({ title: "Unable to load conversations", variant: "error" }); + if (!logRpcFailure("Failed to subscribe to chats:", err)) { + reportIssue('chat.subscription-load', err) + toasts.add({ title: "Unable to load conversations", variant: "error" }); + } } }; @@ -5366,8 +5386,10 @@ function ChatInterface({ ); } } catch (err) { - console.error("Failed to send message:", err); - toasts.add({ title: "Failed to send message", variant: "error" }); + if (isDurableObjectResetError(err)) reportDoResetError("chat.send", err); + if (!logRpcFailure("Failed to send message:", err)) { + toasts.add({ title: "Failed to send message", variant: "error" }); + } throw err; } }; @@ -5388,8 +5410,10 @@ function ChatInterface({ message, model, capsules, attachments, formats); onNavigateToChatRef.current(newChatId); } catch (err) { - console.error("Failed to create new chat:", err); - toasts.add({ title: "Failed to start conversation", variant: "error" }); + if (isDurableObjectResetError(err)) reportDoResetError("chat.new", err); + if (!logRpcFailure("Failed to create new chat:", err)) { + toasts.add({ title: "Failed to start conversation", variant: "error" }); + } throw err; } }; @@ -7576,6 +7600,7 @@ function ChatInterface({
overseer.newGatekeeper(accountId, url) } diff --git a/packages/workshop-frontend/src/ConnectAccountModal.tsx b/packages/workshop-frontend/src/ConnectAccountModal.tsx index 7afeb75a..7be36f61 100644 --- a/packages/workshop-frontend/src/ConnectAccountModal.tsx +++ b/packages/workshop-frontend/src/ConnectAccountModal.tsx @@ -4,6 +4,7 @@ import { RpcStub } from 'capnweb' import { AuthenticatedApi, GatekeeperVendorFilter } from '@gadgets/workshop-shared/api' import { VendorDescription } from '@gadgets/workshop-shared/gatekeeper' import VendorCard from './VendorCard' +import { withDoResetRetry } from './rpcErrors' interface ConnectAccountModalProps { visible: boolean @@ -41,7 +42,7 @@ export default function ConnectAccountModal({ const fetchVendors = async () => { setVendorsLoading(true) try { - const vendorList = await authenticatedApi.listGatekeeperVendors(filter) + const vendorList = await withDoResetRetry(() => authenticatedApi.listGatekeeperVendors(filter)) const unavailable = vendorList.filter(v => v.unavailable) if (unavailable.length > 0) { toasts.add({ diff --git a/packages/workshop-frontend/src/Connections.tsx b/packages/workshop-frontend/src/Connections.tsx index 1a687067..1700fb9e 100644 --- a/packages/workshop-frontend/src/Connections.tsx +++ b/packages/workshop-frontend/src/Connections.tsx @@ -70,6 +70,8 @@ export default function Connections({ overseer, gadget, chatId, authenticatedApi setHooks(hookList.filter((hook) => hook.gadgetId === id)) onHasGatekeepersChange?.(bindingList.length > 0) } catch (err) { + // Loud on purpose: this panel has no retry path, so a quieted transient failure would + // silently render "no connected resources". console.error('Failed to load gatekeepers:', err) reportIssue('connections.load', err) toasts.add({ title: 'Failed to load connections', variant: 'error' }) diff --git a/packages/workshop-frontend/src/GatekeeperModal.tsx b/packages/workshop-frontend/src/GatekeeperModal.tsx index edcd5fb3..8e57587b 100644 --- a/packages/workshop-frontend/src/GatekeeperModal.tsx +++ b/packages/workshop-frontend/src/GatekeeperModal.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from './rpcErrors' +import { logRpcFailure, withDoResetRetry } from './rpcErrors' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Dialog, useKumoToastManager, type PortalContainer } from '@cloudflare/kumo' import { @@ -369,7 +369,7 @@ export default function GatekeeperModal({ setSpawnerEnv( (spawnerEnvCandidatesRef.current ?? []).map(entry => ({ ...entry, enabled: true }))) - authenticatedApi.listModels().then(models => { + withDoResetRetry(() => authenticatedApi.listModels()).then(models => { if (cancelled) return setAvailableModels(models) if (models.length > 0) { @@ -388,7 +388,7 @@ export default function GatekeeperModal({ toasts.add({ title: "Couldn't load AI models", variant: 'error' }) }) - authenticatedApi.listGatekeeperVendors().then(vendors => { + withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()).then(vendors => { if (cancelled) return setVendors(vendors) }).catch(err => { @@ -441,7 +441,7 @@ export default function GatekeeperModal({ } }) .catch(error => { - console.error('Failed to subscribe to connected accounts:', error) + logRpcFailure('Failed to subscribe to connected accounts:', error) }) return () => { diff --git a/packages/workshop-frontend/src/ObserverConfigModal.tsx b/packages/workshop-frontend/src/ObserverConfigModal.tsx index 9a8d3ceb..32b5c716 100644 --- a/packages/workshop-frontend/src/ObserverConfigModal.tsx +++ b/packages/workshop-frontend/src/ObserverConfigModal.tsx @@ -151,6 +151,8 @@ export default function ObserverConfigModal({ subStub = stub }) .catch(err => { + // Loud on purpose: the modal has no retry path, so a quieted transient failure would + // strand the user on a permanent loader. console.error('Failed to subscribe to connected accounts:', err) toasts.add({ title: 'Failed to load your connected accounts', variant: 'error' }) }) diff --git a/packages/workshop-frontend/src/OnboardingWizard.tsx b/packages/workshop-frontend/src/OnboardingWizard.tsx index e0c3b5ff..6bc6d41e 100644 --- a/packages/workshop-frontend/src/OnboardingWizard.tsx +++ b/packages/workshop-frontend/src/OnboardingWizard.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from './rpcErrors' +import { logRpcFailure, withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef, useCallback } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' import { RpcTarget } from 'capnweb' @@ -230,7 +230,7 @@ export default function OnboardingWizard({ } }) .catch((err) => { - console.error('Failed to subscribe to connected accounts:', err) + logRpcFailure('Failed to subscribe to connected accounts:', err) }) return () => { diff --git a/packages/workshop-frontend/src/ResourcePicker.tsx b/packages/workshop-frontend/src/ResourcePicker.tsx index 27f2f01c..1340bb18 100644 --- a/packages/workshop-frontend/src/ResourcePicker.tsx +++ b/packages/workshop-frontend/src/ResourcePicker.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from './rpcErrors' +import { logRpcFailure, withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef, useMemo, useCallback, type MutableRefObject } from 'react' import { Tooltip, useKumoToastManager } from '@cloudflare/kumo' import { Plus, CaretRight, Warning } from '@phosphor-icons/react' @@ -164,7 +164,7 @@ export default function ResourcePicker({ if (cancelled) stub[Symbol.dispose]() else subscriptionRef.current = { stub } } catch (error) { - console.error('Failed to subscribe to connected accounts:', error) + logRpcFailure('Failed to subscribe to connected accounts:', error) // Nothing more is coming, so show what we have rather than hiding forever. if (!cancelled) setAccountsLoaded(true) } diff --git a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx index 7a274103..2d6ea839 100644 --- a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx +++ b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from '../../rpcErrors' +import { logRpcFailure, withDoResetRetry } from '../../rpcErrors' import { createContext, useCallback, @@ -97,7 +97,7 @@ export function SidebarWorkspacesProvider({ children }: { children: ReactNode }) setGadgetsLoading(false) }) .catch((err) => { - console.error('Failed to load workspaces for sidebar:', err) + logRpcFailure('Failed to load workspaces for sidebar:', err) if (!cancelled) setGadgetsLoading(false) }) return () => { cancelled = true } diff --git a/packages/workshop-frontend/src/homePromptFlow.test.tsx b/packages/workshop-frontend/src/homePromptFlow.test.tsx index 577934ec..337d356a 100644 --- a/packages/workshop-frontend/src/homePromptFlow.test.tsx +++ b/packages/workshop-frontend/src/homePromptFlow.test.tsx @@ -57,7 +57,9 @@ describe("Home prompt route flow", () => { container?.remove(); localStorage.clear(); testState.seeds.length = 0; + testState.listModels.mockReset().mockResolvedValue([]); vi.clearAllMocks(); + vi.useRealTimers(); }); it("seeds the composer once, clears route state, and does not create a workspace", async () => { @@ -73,4 +75,22 @@ describe("Home prompt route flow", () => { expect(testState.navigate).toHaveBeenCalledWith({ to: "/", search: {}, replace: true }); expect(testState.newGadget).not.toHaveBeenCalled(); }); + + it("surfaces a Durable Object reset after the model retry is exhausted", async () => { + vi.useFakeTimers(); + const reset = Object.assign(new Error("reset"), { durableObjectReset: true }); + testState.listModels.mockRejectedValue(reset); + container = document.createElement("div"); + document.body.append(container); + root = createRoot(container); + + await act(async () => root!.render()); + await act(async () => vi.runAllTimersAsync()); + + expect(testState.listModels).toHaveBeenCalledTimes(2); + expect(testState.addToast).toHaveBeenCalledWith({ + title: "Couldn't load AI models", + variant: "error", + }); + }); }); diff --git a/packages/workshop-frontend/src/routes/__root.tsx b/packages/workshop-frontend/src/routes/__root.tsx index 4ba8c9ea..ba508712 100644 --- a/packages/workshop-frontend/src/routes/__root.tsx +++ b/packages/workshop-frontend/src/routes/__root.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from '../rpcErrors' +import { logRpcFailure, withDoResetRetry } from '../rpcErrors' import { useState, useEffect } from 'react' import { createRootRoute, Outlet, useRouterState } from '@tanstack/react-router' import { TooltipProvider, Toasty } from '@cloudflare/kumo' @@ -155,7 +155,7 @@ function AuthenticatedShell({ withDoResetRetry(() => authenticatedApi.isOnboardingCompleted()).then((completed) => { if (!cancelled) setOnboardingNeeded(!completed) }).catch((err) => { - console.error('Failed to check onboarding status:', err) + logRpcFailure('Failed to check onboarding status:', err) // If the check fails, skip onboarding to avoid blocking the user if (!cancelled) setOnboardingNeeded(false) }) diff --git a/packages/workshop-frontend/src/routes/gatekeepers.tsx b/packages/workshop-frontend/src/routes/gatekeepers.tsx index 7b0a166e..ca323b46 100644 --- a/packages/workshop-frontend/src/routes/gatekeepers.tsx +++ b/packages/workshop-frontend/src/routes/gatekeepers.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from '../rpcErrors' +import { logRpcFailure, withDoResetRetry } from '../rpcErrors' import { createFileRoute } from '@tanstack/react-router' import { useEffect, useMemo, useRef, useState } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' @@ -488,7 +488,7 @@ function ConnectorsPage() { if (!cancelled) setAddable(list) }) .catch((err) => { - console.error('Failed to load addable gatekeepers:', err) + logRpcFailure('Failed to load addable gatekeepers:', err) }) withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()) @@ -513,7 +513,7 @@ function ConnectorsPage() { setVendorsLoaded(true) }) .catch((err) => { - console.error('Failed to load available services:', err) + logRpcFailure('Failed to load available services:', err) if (!cancelled) setLoadError(true) }) @@ -560,7 +560,7 @@ function ConnectorsPage() { } }) .catch((err) => { - console.error('Failed to subscribe to connected accounts:', err) + logRpcFailure('Failed to subscribe to connected accounts:', err) if (!cancelled) setLoadError(true) }) diff --git a/packages/workshop-frontend/src/routes/index.tsx b/packages/workshop-frontend/src/routes/index.tsx index 9cac5355..a6a37560 100644 --- a/packages/workshop-frontend/src/routes/index.tsx +++ b/packages/workshop-frontend/src/routes/index.tsx @@ -1,4 +1,4 @@ -import { withDoResetRetry } from "../rpcErrors"; +import { isDurableObjectResetError, logRpcFailure, reportDoResetError, withDoResetRetry } from "../rpcErrors"; import { useState, useEffect, useRef, useCallback } from "react"; import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { useKumoToastManager } from "@cloudflare/kumo"; @@ -65,8 +65,10 @@ export function HomePageContent({ prompt }: HomeSearch) { setSelectedModel(getStoredSelectedModel(list)); }) .catch((err) => { - console.error("Failed to fetch models:", err); - toasts.add({ title: "Couldn't load AI models", variant: "error" }); + const transient = logRpcFailure("Failed to fetch models:", err); + if (!transient || isDurableObjectResetError(err)) { + toasts.add({ title: "Couldn't load AI models", variant: "error" }); + } }); return () => { cancelled = true; @@ -117,13 +119,16 @@ export function HomePageContent({ prompt }: HomeSearch) { // Open the conversation we just started. navigate({ to: "/workspace/$id", params: { id }, search: { chat } }); } catch (err) { - console.error("Failed to create gadget:", err); + if (isDurableObjectResetError(err)) reportDoResetError("workspace.create", err); + const transient = logRpcFailure("Failed to create gadget:", err); // A retry reuses the provisional gadget while the draft contains gadget-scoped references. if (!attachments?.length && !capsules?.length) { provisionalOverseerRef.current?.stub[Symbol.dispose](); provisionalOverseerRef.current = null; } - toasts.add({ title: "Failed to create workspace", variant: "error" }); + if (!transient) { + toasts.add({ title: "Failed to create workspace", variant: "error" }); + } throw err; } }, diff --git a/packages/workshop-frontend/src/routes/providers.tsx b/packages/workshop-frontend/src/routes/providers.tsx index 16e16e04..ef6e1c95 100644 --- a/packages/workshop-frontend/src/routes/providers.tsx +++ b/packages/workshop-frontend/src/routes/providers.tsx @@ -18,6 +18,7 @@ import { import AddModelModal from '../AddModelModal' import { useDocumentTitle } from '../useDocumentTitle' import { MENU_CONTENT, MENU_ITEM, MENU_ITEM_DANGER } from '../components/menuStyles' +import { withDoResetRetry } from '../rpcErrors' export const Route = createFileRoute('/providers')({ component: ProvidersPage }) @@ -148,8 +149,8 @@ function ProvidersPage() { setLoadError(false) try { const [modelList, qm, cfg] = await Promise.all([ - authenticatedApi.listModels(), - authenticatedApi.getQuickModel(), + withDoResetRetry(() => authenticatedApi.listModels()), + withDoResetRetry(() => authenticatedApi.getQuickModel()), authenticatedApi.getAiConfig(), ]) setModels(modelList) diff --git a/packages/workshop-frontend/src/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts index c5f79d49..07f1cb22 100644 --- a/packages/workshop-frontend/src/rpcErrors.test.ts +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -5,7 +5,7 @@ vi.mock('./errorReporting', () => ({ reportIssue: vi.fn() })) import { reportIssue } from './errorReporting' import { classifyRpcError, getDurableObjectId, isDurableObjectResetError, isOverloadedError, - isTransientRpcError, reportDoResetError, withDoResetRetry, + isTransientRpcError, logRpcFailure, reportDoResetError, withDoResetRetry, } from './rpcErrors' // The reject frame observed in prod for a DO storage-timeout reset. @@ -132,3 +132,21 @@ describe('withDoResetRetry', () => { } }) }) + +describe('logRpcFailure', () => { + it('logs transient errors at debug level and returns true', () => { + const debug = vi.spyOn(console, 'debug').mockImplementation(() => {}) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + expect(logRpcFailure('failed:', storageTimeoutReset())).toBe(true) + expect(debug).toHaveBeenCalledOnce() + expect(error).not.toHaveBeenCalled() + + expect(logRpcFailure('failed:', new Error('Workspace not found.'))).toBe(false) + expect(error).toHaveBeenCalledOnce() + } finally { + debug.mockRestore() + error.mockRestore() + } + }) +}) diff --git a/packages/workshop-frontend/src/rpcErrors.ts b/packages/workshop-frontend/src/rpcErrors.ts index 3f03ce49..5fe663b7 100644 --- a/packages/workshop-frontend/src/rpcErrors.ts +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -58,6 +58,15 @@ export function isTransientRpcError(err: unknown): boolean { return cls === 'do-reset' || cls === 'connection' } +// Logs an RPC failure: quietly for transient errors (a retry or reconnect is expected to cure +// them), loudly otherwise. Returns true when transient so call sites can skip their toasts. +export function logRpcFailure(message: string, err: unknown): boolean { + const transient = isTransientRpcError(err) + if (transient) console.debug(message, err) + else console.error(message, err) + return transient +} + const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) // Retries an idempotent call once after a DO reset: the object restarts on its next request, diff --git a/packages/workshop-frontend/src/useVendorBranding.ts b/packages/workshop-frontend/src/useVendorBranding.ts index 8b4bcbc2..bfdc41ed 100644 --- a/packages/workshop-frontend/src/useVendorBranding.ts +++ b/packages/workshop-frontend/src/useVendorBranding.ts @@ -1,4 +1,4 @@ -import { withDoResetRetry } from './rpcErrors' +import { logRpcFailure, withDoResetRetry } from './rpcErrors' import { useEffect, useState } from 'react' import { RpcStub } from 'capnweb' import { AuthenticatedApi } from '@gadgets/workshop-shared/api' @@ -51,7 +51,7 @@ export function useVendorBranding( promise .then((map) => { if (!cancelled) setBranding(map) }) .catch((err) => { - console.error('Failed to load vendor branding:', err) + logRpcFailure('Failed to load vendor branding:', err) }) return () => { cancelled = true } From 7d776dfc521c1f6f68715e15437629ca504d32cd Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Thu, 6 Aug 2026 18:01:09 -0500 Subject: [PATCH 4/6] Harden rpcErrors against message drift Per the DO error-handling docs, flags are the supported contract: - auth strings move to a shared AUTH_ERROR_MESSAGES constant thrown by the backend and imported by the classifier, so they cannot drift - a canary test pins the flagless capnweb transport messages to the installed build, so an upgrade fails in CI rather than in the UX - the workerd reset strings are documented as re-wrap fallback only - withDoResetRetry documents why one jittered retry is safe despite the overloaded flag accompanying reset errors --- packages/workshop-backend/src/server.ts | 6 +- packages/workshop-backend/src/user.ts | 13 +++- .../workshop-frontend/src/rpcErrors.test.ts | 77 +++++++++++++++++-- packages/workshop-frontend/src/rpcErrors.ts | 37 +++++++-- packages/workshop-shared/src/api.ts | 34 ++++++++ 5 files changed, 149 insertions(+), 18 deletions(-) diff --git a/packages/workshop-backend/src/server.ts b/packages/workshop-backend/src/server.ts index 2f054fcc..ca7609ad 100644 --- a/packages/workshop-backend/src/server.ts +++ b/packages/workshop-backend/src/server.ts @@ -1,7 +1,7 @@ import { RpcStub, RpcTarget, newWorkersRpcResponse } from "capnweb"; import { validateRpc } from "capnweb-validate"; import type { JWTPayload } from "jose"; -import { PublicApi, AuthenticatedApi, Overseer, GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, AiGatewayInfo, AiModelProvider, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, ObserverConfigCallback, BlueprintLibrarySummary, BlueprintPublicInfo, BlueprintUserSummary, BlueprintBindingAssignment, AgentSpawnerConfig, WorkpieceId, BLUEPRINT_SCREENSHOT_PATH_PREFIX, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ServerConfig, CloudflareUsageInfo, CloudflareAccountOption, LoginAttempt, GatekeeperAppInfo, AdminApi, GatekeeperVendorInfo, OutputFormatOffer, ListOutputsResult, createOpenGadgetError, getOpenGadgetErrorCode, OPEN_GADGET_ERROR_CODES } from '@gadgets/workshop-shared/api'; +import { PublicApi, AuthenticatedApi, Overseer, GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, AiGatewayInfo, AiModelProvider, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, ObserverConfigCallback, BlueprintLibrarySummary, BlueprintPublicInfo, BlueprintUserSummary, BlueprintBindingAssignment, AgentSpawnerConfig, WorkpieceId, BLUEPRINT_SCREENSHOT_PATH_PREFIX, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ServerConfig, CloudflareUsageInfo, CloudflareAccountOption, LoginAttempt, GatekeeperAppInfo, AdminApi, GatekeeperVendorInfo, OutputFormatOffer, ListOutputsResult, createOpenGadgetError, getOpenGadgetErrorCode, OPEN_GADGET_ERROR_CODES, AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api'; import type { UiFeatureFlags } from "@gadgets/workshop-shared/feature-flags"; import { getServerConfig } from "./deployment-config.js"; import { isPasswordAuthEnabled, getAuthGatekeeperAllowlist } from "./auth/config.js"; @@ -671,7 +671,7 @@ class PublicApiImpl extends RpcTarget implements PublicApi { async authenticate(token: string): Promise { let split = token.split(':'); if (split.length !== 2) { - throw new Error("Invalid session token."); + throw createAuthError(AUTH_ERROR_CODES.invalidSessionToken); } let userId = this.users.idFromName(split[0]); @@ -687,7 +687,7 @@ class PublicApiImpl extends RpcTarget implements PublicApi { async authenticateFromCfAccess(): Promise { if (!this.accessPayload) { - throw new Error("Not authenticated with Access."); + throw createAuthError(AUTH_ERROR_CODES.notAuthenticatedWithAccess); } let email = this.accessPayload.email as string; diff --git a/packages/workshop-backend/src/user.ts b/packages/workshop-backend/src/user.ts index 727d0898..e0fa3891 100644 --- a/packages/workshop-backend/src/user.ts +++ b/packages/workshop-backend/src/user.ts @@ -1,5 +1,5 @@ import { RpcStub } from "capnweb"; -import { GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, SUGGESTED_MODELS, CollaboratorRole, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, GadgetMetadata, BlueprintMetadata, BlueprintLibrarySummary, BlueprintSource, BlueprintUserSummary, BLUEPRINT_SCREENSHOT_R2_PREFIX, GatekeeperVendorInfo, BlueprintOutput, OutputSummary, WorkpieceId, ListOutputsResult } from '@gadgets/workshop-shared/api'; +import { GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, SUGGESTED_MODELS, CollaboratorRole, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, GadgetMetadata, BlueprintMetadata, BlueprintLibrarySummary, BlueprintSource, BlueprintUserSummary, BLUEPRINT_SCREENSHOT_R2_PREFIX, GatekeeperVendorInfo, BlueprintOutput, OutputSummary, WorkpieceId, ListOutputsResult, AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api'; import { Gatekeeper, GatekeeperUser, GatekeeperUserVerifier, GatekeeperVendor, AccountDescription, VendorDescription, GatekeeperConnectCallback, SupportedResource, ResourceConfiguratorFrame, AppUiContext, GatekeeperUiFrame } from "@gadgets/workshop-shared/gatekeeper"; import { shouldAutoProvisionAccount, ambientGatekeeperMode } from "./provisioning-policy.js"; import { CloudflareGatekeeperUser } from "@gadgets/workshop-shared/cloudflare-gatekeeper"; @@ -296,12 +296,19 @@ export class UserDurableObject extends DurableObject { } async authenticate(token: string): Promise { - let tokenBytes = Uint8Array.fromBase64(token); + let tokenBytes: Uint8Array; + try { + tokenBytes = Uint8Array.fromBase64(token); + } catch { + // A corrupt (non-Base64) token must classify as an auth failure like any other bad token, + // not surface as the decoder's SyntaxError. + throw createAuthError(AUTH_ERROR_CODES.invalidSessionToken); + } let hash = await crypto.subtle.digest('SHA-256', tokenBytes); let tokenId = new Uint8Array(hash).toHex(); let session = this.storage.sessions.get(tokenId); if (!session) { - throw new Error("invalid session token"); + throw createAuthError(AUTH_ERROR_CODES.invalidSessionToken); } } diff --git a/packages/workshop-frontend/src/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts index 07f1cb22..f3e45739 100644 --- a/packages/workshop-frontend/src/rpcErrors.test.ts +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -1,11 +1,18 @@ +// Vitest runs under node, but the src/ tsconfig only has browser types — hence the suppressions. +// @ts-expect-error node builtin without @types/node +import { readFileSync } from 'node:fs' +// @ts-expect-error node builtin without @types/node +import { createRequire } from 'node:module' import { describe, expect, it, vi } from 'vitest' +import { deserialize, serialize } from 'capnweb' +import { AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api' -vi.mock('./errorReporting', () => ({ reportIssue: vi.fn() })) +vi.mock('./errorReporting', () => ({ reportIssue: vi.fn<(site: string, err: unknown, options?: object) => void>() })) import { reportIssue } from './errorReporting' import { classifyRpcError, getDurableObjectId, isDurableObjectResetError, isOverloadedError, - isTransientRpcError, logRpcFailure, reportDoResetError, withDoResetRetry, + CONNECTION_MESSAGES, isTransientRpcError, logRpcFailure, reportDoResetError, withDoResetRetry, } from './rpcErrors' // The reject frame observed in prod for a DO storage-timeout reset. @@ -31,6 +38,8 @@ describe('classifyRpcError', () => { 'Durable Object reset because its code was updated.', "Durable Object's isolate exceeded its memory limit and was reset.", 'Durable Object exceeded its CPU time limit and was reset.', + // What later calls on an already-dead capability reject with (flagless). + 'The execution context which hosts this callback is no longer running.', ]) { expect(classifyRpcError(new Error(message))).toBe('do-reset') } @@ -50,9 +59,15 @@ describe('classifyRpcError', () => { expect(classifyRpcError(new Error('WebSocket connection failed.'))).toBe('connection') expect(classifyRpcError(new Error('RPC session was shut down by disposing the main stub'))) .toBe('connection') + expect(classifyRpcError(new Error('Attempted to use RPC stub after it has been disposed.'))) + .toBe('connection') }) it('classifies auth failures, which must never be retried or quieted', () => { + // Coded errors are authoritative; bare messages are the fallback for older deployments. + expect(classifyRpcError(createAuthError(AUTH_ERROR_CODES.invalidSessionToken))).toBe('auth') + expect(classifyRpcError(Object.assign(new Error('nope'), { code: 'INVALID_SESSION_TOKEN' }))) + .toBe('auth') expect(classifyRpcError(new Error('invalid session token'))).toBe('auth') expect(classifyRpcError(new Error('Not authenticated with Access.'))).toBe('auth') }) @@ -102,7 +117,7 @@ describe('withDoResetRetry', () => { it('retries once after a reset error', async () => { vi.useFakeTimers() try { - const fn = vi.fn().mockRejectedValueOnce(storageTimeoutReset()).mockResolvedValueOnce('ok') + const fn = vi.fn<() => Promise>().mockRejectedValueOnce(storageTimeoutReset()).mockResolvedValueOnce('ok') const result = withDoResetRetry(fn) await vi.advanceTimersByTimeAsync(2000) expect(await result).toBe('ok') @@ -113,15 +128,38 @@ describe('withDoResetRetry', () => { }) it('does not retry non-reset errors', async () => { - const fn = vi.fn().mockRejectedValue(new Error('Workspace not found.')) + const fn = vi.fn<() => Promise>().mockRejectedValue(new Error('Workspace not found.')) await expect(withDoResetRetry(fn)).rejects.toThrow('Workspace not found.') expect(fn).toHaveBeenCalledTimes(1) }) + it('retries once on a retryable-flagged invocation failure', async () => { + vi.useFakeTimers() + try { + const fn = vi.fn<() => Promise>() + .mockRejectedValueOnce(Object.assign(new Error('internal error'), { remote: true, retryable: true })) + .mockResolvedValueOnce('ok') + const result = withDoResetRetry(fn) + await vi.advanceTimersByTimeAsync(2000) + expect(await result).toBe('ok') + expect(fn).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + // Local transport errors carry no flags; their recovery belongs to the connection manager, + // so the retry must refuse them even though they classify as transient. + it('does not retry flagless transport errors', async () => { + const fn = vi.fn<() => Promise>().mockRejectedValue(new Error('Peer closed WebSocket')) + await expect(withDoResetRetry(fn)).rejects.toThrow('Peer closed WebSocket') + expect(fn).toHaveBeenCalledTimes(1) + }) + it('gives up after the second failure', async () => { vi.useFakeTimers() try { - const fn = vi.fn().mockRejectedValue(storageTimeoutReset()) + const fn = vi.fn<() => Promise>().mockRejectedValue(storageTimeoutReset()) const result = withDoResetRetry(fn) result.catch(() => {}) await vi.advanceTimersByTimeAsync(2000) @@ -150,3 +188,32 @@ describe('logRpcFailure', () => { } }) }) + +// Canary: these client-local errors carry no flags, so the classifier matches capnweb's message +// strings. Pin them to the installed build so an upgrade fails here, not silently in the UX. +describe('capnweb transport messages', () => { + it('still exist in the installed capnweb build', () => { + const require = createRequire(import.meta.url) + const source = readFileSync(require.resolve('capnweb'), 'utf8') + for (const message of CONNECTION_MESSAGES) { + expect(source, `capnweb no longer raises "${message}"`).toContain(message) + } + }) +}) + +// Canary: the classifier's primary path reads flags/codes off the deserialized error, so pin +// capnweb's custom-property round-trip too (serialize/deserialize use the same wire frame as +// RPC rejections). A regression here would silently demote every classification to the message +// fallback and drop auth codes entirely. +describe('capnweb error serialization', () => { + it('round-trips the custom properties the classifier reads', () => { + const sent = Object.assign(storageTimeoutReset(), { code: AUTH_ERROR_CODES.invalidSessionToken }) + const received = deserialize(serialize(sent)) as Error & Record + expect(received).toBeInstanceOf(Error) + expect(received.durableObjectReset).toBe(true) + expect(received.overloaded).toBe(true) + expect(received.durableObjectId).toBe('eed0859e') + expect(received.code).toBe(AUTH_ERROR_CODES.invalidSessionToken) + expect(classifyRpcError(received)).toBe('do-reset') + }) +}) diff --git a/packages/workshop-frontend/src/rpcErrors.ts b/packages/workshop-frontend/src/rpcErrors.ts index 5fe663b7..94d65633 100644 --- a/packages/workshop-frontend/src/rpcErrors.ts +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -1,3 +1,4 @@ +import { AUTH_ERROR_MESSAGES, getAuthErrorCode } from '@gadgets/workshop-shared/api' import { reportIssue } from './errorReporting' // Classifies errors surfaced through capnweb RPC. The backend runs with @@ -8,21 +9,33 @@ import { reportIssue } from './errorReporting' export type RpcErrorClass = 'do-reset' | 'connection' | 'auth' | 'other' +// Fallbacks: workerd errors normally arrive with `durableObjectReset` set (capnweb carries +// the flags in a dedicated slot); the first four strings only matter when something re-wrapped +// the error. The last is what LATER calls on an already-dead capability reject with — flagless +// (verified in a workerd probe); the flagged error only reaches calls in flight at reset time. +// Over our RPC surface a dead hosting context always means the capability needs reopening. const DO_RESET_MESSAGES = [ 'Durable Object reset because its code was updated', 'Durable Object storage operation exceeded timeout', "Durable Object's isolate exceeded its memory limit", 'Durable Object exceeded its CPU time limit', + 'The execution context which hosts this callback is no longer running', ] -// Transport failures raised locally by capnweb, plus its own-session teardown message. -const CONNECTION_MESSAGES = [ +// Transport failures raised locally by capnweb, plus its own-session teardown message. These +// carry no flags, so matching messages is all we have; a canary test pins them to the installed +// capnweb build so an upgrade fails loudly here instead of silently in the UX. +export const CONNECTION_MESSAGES = [ 'Peer closed WebSocket', 'WebSocket connection failed.', 'RPC session was shut down by disposing the main stub', + // What RPCs on an already-disposed stub reject with — e.g. the zombie the connection manager + // disposes while an outage is being recovered. + 'Attempted to use RPC stub after it has been disposed', ] -const AUTH_MESSAGES = ['invalid session token', 'Not authenticated with Access'] +// Fallback for auth errors thrown without a code (older deployments); codes are authoritative. +const AUTH_MESSAGES = Object.values(AUTH_ERROR_MESSAGES) const messageOf = (err: unknown) => (err instanceof Error ? err.message : String(err)) @@ -48,7 +61,11 @@ export function classifyRpcError(err: unknown): RpcErrorClass { if (flag(err, 'retryable') || CONNECTION_MESSAGES.some(m => message.includes(m))) { return 'connection' } - if (AUTH_MESSAGES.some(m => message.includes(m))) return 'auth' + // 'auth' is deliberately terminal — never quieted, never retried, and there is no missing + // re-auth handler: the session is invalid and only a fresh login cures it. + if (getAuthErrorCode(err) !== undefined || AUTH_MESSAGES.some(m => message.includes(m))) { + return 'auth' + } return 'other' } @@ -69,13 +86,19 @@ export function logRpcFailure(message: string, err: unknown): boolean { const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) -// Retries an idempotent call once after a DO reset: the object restarts on its next request, -// so a single delayed attempt usually succeeds. Never use for writes. +// Retries an idempotent call once after a backend-side transient failure: a DO reset (the object +// restarts on its next request, reached via a fresh stub) or a `retryable`-flagged invocation +// failure. Flags only survive on errors that round-tripped from the backend, so a flagged error +// proves the socket was healthy and a retry-in-place can succeed. Local transport errors carry no +// flags and are deliberately not retried: the connection manager owns that recovery, and a retry +// through the closure-captured dead stub could never succeed anyway. Deliberately retries even +// when `overloaded` is set alongside the reset — the reset destroyed the queue that was +// overloaded, and one jittered attempt is not a retry loop. Never use for writes. export async function withDoResetRetry(fn: () => Promise, delayMs = 1500): Promise { try { return await fn() } catch (err) { - if (!isDurableObjectResetError(err)) throw err + if (!isDurableObjectResetError(err) && !flag(err, 'retryable')) throw err await sleep(delayMs * (0.75 + Math.random() * 0.5)) return fn() } diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index 7cee2e78..14916a63 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -285,6 +285,40 @@ function isOpenGadgetErrorCode(value: unknown): value is OpenGadgetErrorCode { value === OPEN_GADGET_ERROR_CODES.workspaceAccessDenied; } +/** Stable error codes attached to authentication failures. */ +export const AUTH_ERROR_CODES = { + invalidSessionToken: "INVALID_SESSION_TOKEN", + notAuthenticatedWithAccess: "NOT_AUTHENTICATED_WITH_ACCESS", +} as const; + +/** An expected authentication failure code. */ +export type AuthErrorCode = typeof AUTH_ERROR_CODES[keyof typeof AUTH_ERROR_CODES]; + +/** Messages for auth failures thrown without a surviving code; clients match these only as a + * classification fallback, so changing one is a compatibility break with older deployments. */ +export const AUTH_ERROR_MESSAGES: Record = { + [AUTH_ERROR_CODES.invalidSessionToken]: "invalid session token", + [AUTH_ERROR_CODES.notAuthenticatedWithAccess]: "Not authenticated with Access.", +}; + +/** Creates an authentication failure with a machine-readable code. */ +export function createAuthError(code: AuthErrorCode): Error & { code: AuthErrorCode } { + return Object.assign(new Error(AUTH_ERROR_MESSAGES[code]), { code }); +} + +/** Reads the machine-readable code from an authentication failure. */ +export function getAuthErrorCode(error: unknown): AuthErrorCode | undefined { + if (typeof error !== "object" || error === null) return undefined; + + const candidate = "code" in error ? error.code : undefined; + return isAuthErrorCode(candidate) ? candidate : undefined; +} + +function isAuthErrorCode(value: unknown): value is AuthErrorCode { + return value === AUTH_ERROR_CODES.invalidSessionToken || + value === AUTH_ERROR_CODES.notAuthenticatedWithAccess; +} + // Top-level API exposed to the user after they have authenticated. export interface AuthenticatedApi extends RpcTarget { // Get profile info for the user who is logged in. From b2e88a9d3bc2aac57b138541f5a2b588a00b7de3 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sat, 8 Aug 2026 09:50:40 -0500 Subject: [PATCH 5/6] Restore e-order by caching the user-DO stub, with reset-safe invalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E-order is guaranteed per stub, so the per-access getter from the previous commit traded ordering for reset recovery. Cache one stub per session instead, and intercept rejections through a Proxy in the getter: flagged errors (durableObjectReset/retryable) drop the cache and rethrow — the call may have executed, so the frontend owns that recovery — while a flagless dead-capability rejection proves the call never reached the DO and is re-issued exactly once on a fresh stub, safe even for writes. Two JSRPC subtleties the tests caught: `.apply` on a stub method proxy is an RPC path segment (use Reflect.apply), and JsRpcPromise.then rejects a non-function first argument. The retry goes through the raw target (via a symbol escape hatch) so cascading resets cannot retry unboundedly. The reset-recovery integration test doubles as the canary for workerd's dead-capability message; a new case pins that the cache re-arms after recovery instead of churning or staying poisoned. --- .../__integration__/open-gadget-rpc.test.ts | 36 +++++++--- packages/workshop-backend/src/server.ts | 71 +++++++++++++++++-- 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts b/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts index fff93c25..df771951 100644 --- a/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts +++ b/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts @@ -135,12 +135,14 @@ describe.skip("openGadget errors across native RPC and Cap'n Web", () => { }); }); -// The DO-reset recovery contract: a user-DO reset must not poison the API session. A stub is -// bound to one incarnation of the object and is permanently broken by a reset, so the -// authenticated API re-resolves its user-DO stub per call — the same session recovers on the -// next request with no reconnect. abortAllDurableObjects() is the non-graceful teardown, the -// local stand-in for the storage-timeout/overload resets observed in production. (Deliberately -// not evictDurableObject(): eviction is graceful — it drains in-flight work and never breaks a +// The DO-reset recovery contract: a user-DO reset must not poison the API session. E-order is +// per-stub, so the authenticated API shares one user-DO stub per session while it's healthy; a +// reset permanently breaks that stub, so a stub-breaking rejection drops the cache, and a +// rejection that proves the call was never sent (the flagless dead-capability error — the +// flagged one only reaches calls in flight at reset time) is transparently re-issued once on a +// fresh stub. abortAllDurableObjects() is the non-graceful teardown, the local stand-in for the +// storage-timeout/overload resets observed in production. (Deliberately not +// evictDurableObject(): eviction is graceful — it drains in-flight work and never breaks a // stub — so it cannot reproduce this failure.) describe("user-DO reset recovery", () => { it("recovers on the same session after the user DO is reset", async () => { @@ -152,8 +154,26 @@ describe("user-DO reset recovery", () => { await abortAllDurableObjects(); - // Same socket, same AuthenticatedApiImpl. With the per-call stub getter this reaches the - // restarted object; with a session-cached stub it would reject with the abort error forever. + // Same socket, same AuthenticatedApiImpl. The cached stub is dead, so this call rejects + // locally without reaching the DO and is re-issued once on a fresh stub — it must succeed + // with no client-visible failure. This doubles as the canary for workerd's dead-capability + // message: if that string drifts, the never-sent retry stops firing and this fails loudly. expect(await authenticated.listModels()).toBeInstanceOf(Array); }); + + it("re-arms the cached stub after recovery instead of churning or staying poisoned", async () => { + using publicApi = await connect(); + const account = await createAccount(publicApi, "rearm"); + using authenticated = await publicApi.authenticate(account.token); + + expect(await authenticated.listModels()).toBeInstanceOf(Array); + + await abortAllDurableObjects(); + + // First call recovers via the never-sent retry; the calls after it must ride the re-armed + // cached stub (a poisoned or thrashing cache would reject here). + expect(await authenticated.listModels()).toBeInstanceOf(Array); + expect(await authenticated.isOnboardingCompleted()).toBeTypeOf("boolean"); + expect(await authenticated.whoami()).toBeTruthy(); + }); }); diff --git a/packages/workshop-backend/src/server.ts b/packages/workshop-backend/src/server.ts index ca7609ad..672c4e90 100644 --- a/packages/workshop-backend/src/server.ts +++ b/packages/workshop-backend/src/server.ts @@ -71,6 +71,10 @@ type Env = Cloudflare.Env & { // ======================================================================================= +// Escape hatch on the wrapped user-DO stub (see #wrapUserStub) exposing its raw target, so the +// never-sent retry path can re-issue a call without re-entering the retry wrapper. +const RAW_USER_STUB = Symbol("rawUserStub"); + @validateRpc() class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { constructor(private ctx: ExecutionContext, private env: Env, @@ -87,11 +91,70 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { private adminSettings: DurableObjectNamespace; private users: DurableObjectNamespace; - // A stub is bound to one incarnation of the DO and is poisoned once that incarnation resets, - // so re-resolve per call instead of caching one for the session (per the DO error-handling - // docs: create a fresh stub per attempt). Stub creation is local and lazy — not a network call. + private userStub?: DurableObjectStub; + + // E-order (in-order delivery to the DO) is guaranteed per stub, so the session shares one stub + // while it's healthy. But a stub is bound to one incarnation of the object and is permanently + // broken once that incarnation resets, so the wrapper below drops the cached stub the moment a + // call through it fails with a stub-breaking error, and the next call re-resolves a fresh one. private get user(): DurableObjectStub { - return this.users.get(this.userId); + return this.userStub ??= this.#wrapUserStub(this.users.get(this.userId)); + } + + // Calls on a stub whose DO already reset while idle reject flagless with the brokenness + // reason; the flagged (`durableObjectReset`) error only reaches calls in flight at reset time. + // Verified in a workerd probe — see DO_RESET_MESSAGES in workshop-frontend/src/rpcErrors.ts, + // which pins the first string. The distinction matters: a flagless rejection matching one of + // these proves the call never reached the DO, so re-issuing it on a fresh stub cannot + // double-execute — safe even for writes. + static readonly #DEAD_CAPABILITY_MESSAGES = [ + // What production resets leave as the brokenness reason. + "The execution context which hosts this callback is no longer running", + // What vitest-pool-workers' abortAllDurableObjects() leaves — never occurs in production; + // listed so integration tests exercise the real never-sent retry path. + "Application called abortAllDurableObjects().", + ]; + + // Intercepts every method call on the user-DO stub. On a stub-breaking rejection, drops the + // cached stub so the next call re-resolves; if the rejection proves the call was never sent + // (see #DEAD_CAPABILITY_MESSAGES), re-issues it once on the fresh stub. Flagged errors are + // rethrown untouched — the call may have executed, and the frontend owns that recovery. + #wrapUserStub(stub: DurableObjectStub): DurableObjectStub { + const wrapped: DurableObjectStub = new Proxy(stub, { + get: (target, prop) => { + if (prop === RAW_USER_STUB) return target; + const value = Reflect.get(target, prop); + if (typeof value !== "function") return value; + return (...args: unknown[]) => { + // Reflect.apply, not value.apply(): on a JSRPC method proxy, `.apply` is an RPC path + // segment (it would invoke a remote method named "apply"), not Function.prototype.apply. + const result = Reflect.apply(value, target, args); + if (typeof (result as PromiseLike | null)?.then !== "function") return result; + // JsRpcPromise.then validates its first parameter as a Function — no `undefined` slot. + return (result as Promise).then((v: unknown) => v, (err: unknown) => { + const flags = err as { durableObjectReset?: unknown, retryable?: unknown } | null; + const flagged = flags?.durableObjectReset === true || flags?.retryable === true; + const neverSent = !flagged && err instanceof Error && + AuthenticatedApiImpl.#DEAD_CAPABILITY_MESSAGES.some(m => err.message.includes(m)); + if (flagged || neverSent) { + // Guard against thrashing: concurrent failures on the same dead stub must not + // each discard the replacement the first one already resolved. + if (this.userStub === wrapped) this.userStub = undefined; + if (neverSent) { + // Retry exactly once, against the raw target of the re-armed stub — going + // through the proxy again would retry unboundedly under repeated resets. + const raw = (this.user as unknown as Record)[RAW_USER_STUB] as + DurableObjectStub; + return Reflect.apply(Reflect.get(raw, prop) as (...a: unknown[]) => unknown, + raw, args); + } + } + throw err; + }); + }; + }, + }); + return wrapped; } #isAdmin(): boolean { From 7998195fb0abff341cd358ce8b4bc78e0a8c1b56 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Sat, 8 Aug 2026 10:18:12 -0500 Subject: [PATCH 6/6] Simplification pass over the DO-reset resilience work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Architectural: the idempotent-read retry moves from 19 per-call-site withDoResetRetry wraps (which had already drifted — providers.tsx left getAiConfig unwrapped while OnboardingWizard wrapped it) to a single withReadRetries proxy installed where useAuth creates the stub, keyed by a method-level allowlist — idempotency is a property of the method, not the call site. Future reads get the policy for free; writes and unlisted methods pass through untouched. Mechanical: the backend Proxy's RAW_USER_STUB symbol escape hatch is replaced by caching the raw stub in a second field — no symbol, no double casts, and the thrash guard compares against the trap's own target. logRpcFailure now owns do-reset telemetry via a reportSite option, collapsing the classify-report-log ritual at three action sites. The workerd dead-capability message now lives once, in workshop-shared, referenced by the backend matcher and the frontend classifier, so the frontend canary guards both. Block/line: the two coded-error families in api.ts share one codedErrorFamily factory (membership derived from the message record instead of hand-enumerated); the composer's send-hiccup hint is one state value scoped by its render condition instead of a boolean, a ref mirrored during render, and a comparison at set time; fake-timer try/finally scaffolding in rpcErrors.test.ts becomes afterEach, twin retry tests merge into it.each, and error fixtures share an rpcError helper. New tests pin the chokepoint: listed reads retry once, writes never do. --- packages/workshop-backend/src/server.ts | 47 ++++----- .../src/BlueprintLandingPage.tsx | 6 +- .../workshop-frontend/src/ChatInterface.tsx | 25 ++--- .../src/ConnectAccountModal.tsx | 3 +- .../workshop-frontend/src/GatekeeperModal.tsx | 8 +- .../src/ObserverConfigModal.tsx | 9 +- .../src/OnboardingWizard.tsx | 8 +- .../workshop-frontend/src/ResourcePicker.tsx | 5 +- .../components/AppShell/SidebarWorkspaces.tsx | 4 +- .../src/components/ConnectionChips.tsx | 3 +- .../src/homePromptFlow.test.tsx | 18 +++- .../workshop-frontend/src/routes/__root.tsx | 4 +- .../src/routes/gatekeepers.tsx | 8 +- .../workshop-frontend/src/routes/index.tsx | 14 +-- .../src/routes/providers.tsx | 5 +- .../workshop-frontend/src/rpcErrors.test.ts | 99 ++++++++++--------- packages/workshop-frontend/src/rpcErrors.ts | 58 +++++++++-- packages/workshop-frontend/src/useAuth.ts | 6 +- .../src/useVendorBranding.ts | 4 +- packages/workshop-shared/src/api.ts | 64 ++++++------ 20 files changed, 224 insertions(+), 174 deletions(-) diff --git a/packages/workshop-backend/src/server.ts b/packages/workshop-backend/src/server.ts index 672c4e90..4234d441 100644 --- a/packages/workshop-backend/src/server.ts +++ b/packages/workshop-backend/src/server.ts @@ -1,7 +1,7 @@ import { RpcStub, RpcTarget, newWorkersRpcResponse } from "capnweb"; import { validateRpc } from "capnweb-validate"; import type { JWTPayload } from "jose"; -import { PublicApi, AuthenticatedApi, Overseer, GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, AiGatewayInfo, AiModelProvider, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, ObserverConfigCallback, BlueprintLibrarySummary, BlueprintPublicInfo, BlueprintUserSummary, BlueprintBindingAssignment, AgentSpawnerConfig, WorkpieceId, BLUEPRINT_SCREENSHOT_PATH_PREFIX, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ServerConfig, CloudflareUsageInfo, CloudflareAccountOption, LoginAttempt, GatekeeperAppInfo, AdminApi, GatekeeperVendorInfo, OutputFormatOffer, ListOutputsResult, createOpenGadgetError, getOpenGadgetErrorCode, OPEN_GADGET_ERROR_CODES, AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api'; +import { PublicApi, AuthenticatedApi, Overseer, GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, AiGatewayInfo, AiModelProvider, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, ObserverConfigCallback, BlueprintLibrarySummary, BlueprintPublicInfo, BlueprintUserSummary, BlueprintBindingAssignment, AgentSpawnerConfig, WorkpieceId, BLUEPRINT_SCREENSHOT_PATH_PREFIX, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ServerConfig, CloudflareUsageInfo, CloudflareAccountOption, LoginAttempt, GatekeeperAppInfo, AdminApi, GatekeeperVendorInfo, OutputFormatOffer, ListOutputsResult, createOpenGadgetError, getOpenGadgetErrorCode, OPEN_GADGET_ERROR_CODES, AUTH_ERROR_CODES, createAuthError, WORKERD_DEAD_CAPABILITY_MESSAGE } from '@gadgets/workshop-shared/api'; import type { UiFeatureFlags } from "@gadgets/workshop-shared/feature-flags"; import { getServerConfig } from "./deployment-config.js"; import { isPasswordAuthEnabled, getAuthGatekeeperAllowlist } from "./auth/config.js"; @@ -71,10 +71,6 @@ type Env = Cloudflare.Env & { // ======================================================================================= -// Escape hatch on the wrapped user-DO stub (see #wrapUserStub) exposing its raw target, so the -// never-sent retry path can re-issue a call without re-entering the retry wrapper. -const RAW_USER_STUB = Symbol("rawUserStub"); - @validateRpc() class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { constructor(private ctx: ExecutionContext, private env: Env, @@ -91,25 +87,26 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { private adminSettings: DurableObjectNamespace; private users: DurableObjectNamespace; + // One stub per session, because e-order (in-order delivery to the DO) is guaranteed per stub. + // A stub is permanently broken once its incarnation of the object resets, so the wrapper below + // drops both fields on a stub-breaking rejection and the next call re-resolves fresh ones. + // `rawUserStub` is the Proxy's target, kept so the never-sent retry can re-issue a call + // without re-entering the wrapper. private userStub?: DurableObjectStub; + private rawUserStub?: DurableObjectStub; - // E-order (in-order delivery to the DO) is guaranteed per stub, so the session shares one stub - // while it's healthy. But a stub is bound to one incarnation of the object and is permanently - // broken once that incarnation resets, so the wrapper below drops the cached stub the moment a - // call through it fails with a stub-breaking error, and the next call re-resolves a fresh one. private get user(): DurableObjectStub { - return this.userStub ??= this.#wrapUserStub(this.users.get(this.userId)); + return this.userStub ??= + this.#wrapUserStub(this.rawUserStub = this.users.get(this.userId)); } // Calls on a stub whose DO already reset while idle reject flagless with the brokenness // reason; the flagged (`durableObjectReset`) error only reaches calls in flight at reset time. - // Verified in a workerd probe — see DO_RESET_MESSAGES in workshop-frontend/src/rpcErrors.ts, - // which pins the first string. The distinction matters: a flagless rejection matching one of - // these proves the call never reached the DO, so re-issuing it on a fresh stub cannot - // double-execute — safe even for writes. + // The distinction matters: a flagless rejection matching one of these proves the call never + // reached the DO, so re-issuing it on a fresh stub cannot double-execute — safe even for writes. static readonly #DEAD_CAPABILITY_MESSAGES = [ - // What production resets leave as the brokenness reason. - "The execution context which hosts this callback is no longer running", + // What production resets leave as the brokenness reason; the frontend canary test pins it. + WORKERD_DEAD_CAPABILITY_MESSAGE, // What vitest-pool-workers' abortAllDurableObjects() leaves — never occurs in production; // listed so integration tests exercise the real never-sent retry path. "Application called abortAllDurableObjects().", @@ -120,9 +117,8 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { // (see #DEAD_CAPABILITY_MESSAGES), re-issues it once on the fresh stub. Flagged errors are // rethrown untouched — the call may have executed, and the frontend owns that recovery. #wrapUserStub(stub: DurableObjectStub): DurableObjectStub { - const wrapped: DurableObjectStub = new Proxy(stub, { + return new Proxy(stub, { get: (target, prop) => { - if (prop === RAW_USER_STUB) return target; const value = Reflect.get(target, prop); if (typeof value !== "function") return value; return (...args: unknown[]) => { @@ -139,14 +135,14 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { if (flagged || neverSent) { // Guard against thrashing: concurrent failures on the same dead stub must not // each discard the replacement the first one already resolved. - if (this.userStub === wrapped) this.userStub = undefined; + if (this.rawUserStub === target) this.userStub = this.rawUserStub = undefined; if (neverSent) { - // Retry exactly once, against the raw target of the re-armed stub — going - // through the proxy again would retry unboundedly under repeated resets. - const raw = (this.user as unknown as Record)[RAW_USER_STUB] as - DurableObjectStub; - return Reflect.apply(Reflect.get(raw, prop) as (...a: unknown[]) => unknown, - raw, args); + // Retry exactly once, on the raw target of the re-armed cache — retrying through + // the proxy would retry unboundedly under repeated resets. + void this.user; + const fresh = this.rawUserStub!; + return Reflect.apply(Reflect.get(fresh, prop) as (...a: unknown[]) => unknown, + fresh, args); } } throw err; @@ -154,7 +150,6 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi { }; }, }); - return wrapped; } #isAdmin(): boolean { diff --git a/packages/workshop-frontend/src/BlueprintLandingPage.tsx b/packages/workshop-frontend/src/BlueprintLandingPage.tsx index b44c78eb..e452bc49 100644 --- a/packages/workshop-frontend/src/BlueprintLandingPage.tsx +++ b/packages/workshop-frontend/src/BlueprintLandingPage.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from './rpcErrors' +import { logRpcFailure } from './rpcErrors' import { useState, useEffect, useCallback, useMemo, useRef, type ReactNode } from 'react' import { useNavigate, useParams, useRouter } from '@tanstack/react-router' import { RpcStub, RpcTarget } from 'capnweb' @@ -121,7 +121,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) { // When authenticated, fetch models for binding assignment. useEffect(() => { if (isAuthenticated && authenticatedApi) { - withDoResetRetry(() => authenticatedApi.listModels()) + authenticatedApi.listModels() .then(setModels) .catch(err => logRpcFailure('Failed to load models:', err)) } else { @@ -184,7 +184,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) { ready() {} } - withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber())) + authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber()) .then(stub => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index 4036c284..15b8c2ea 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -1,4 +1,4 @@ -import { isDurableObjectResetError, isTransientRpcError, logRpcFailure, reportDoResetError } from "./rpcErrors"; +import { isTransientRpcError, logRpcFailure } from "./rpcErrors"; import { Fragment, memo, @@ -1842,11 +1842,10 @@ export const ChatInput = ({ const [capsules, setCapsules] = useState([]); const [pendingAttachments, setPendingAttachments] = useState([]); const [isSending, setIsSending] = useState(false); - const [sendHiccup, setSendHiccup] = useState(false); - const chatKeyRef = useRef(chatKey); - chatKeyRef.current = chatKey; - - useEffect(() => setSendHiccup(false), [chatKey]); + // The chat the "may not have been sent" hint belongs to; the render condition scopes it, and + // leaving the chat dismisses it. + const [sendHiccup, setSendHiccup] = useState<{ chatKey?: number | null } | null>(null); + useEffect(() => setSendHiccup(null), [chatKey]); const [isAttachmentDragActive, setIsAttachmentDragActive] = useState(false); const [selectedSlashCommand, setSelectedSlashCommand] = useState(null); // The caret the slash command picker parses at. Deliberately updated only when it moves to a @@ -2285,7 +2284,7 @@ export const ChatInput = ({ const handleSend = async () => { if (sendInFlightRef.current || isSending || isBlocked) return; - setSendHiccup(false); + setSendHiccup(null); const attachmentsSnapshot = pendingAttachments; const readyAttachments = attachmentsSnapshot .filter((attachment) => attachment.uploadState === "ready" && attachment.ref) @@ -2467,9 +2466,7 @@ export const ChatInput = ({ const submittedChatKey = chatKey; void handleSend().catch((err) => { // The onSend handlers already log; the composer only needs the hint state. - if (isTransientRpcError(err) && chatKeyRef.current === submittedChatKey) { - setSendHiccup(true); - } + if (isTransientRpcError(err)) setSendHiccup({ chatKey: submittedChatKey }); }); }; @@ -3064,7 +3061,7 @@ export const ChatInput = ({
)} {draftUpdateBanner} - {sendHiccup && ( + {sendHiccup && sendHiccup.chatKey === chatKey && (
Connection hiccup — your message may not have been sent. Check the thread, then try again.
@@ -5386,8 +5383,7 @@ function ChatInterface({ ); } } catch (err) { - if (isDurableObjectResetError(err)) reportDoResetError("chat.send", err); - if (!logRpcFailure("Failed to send message:", err)) { + if (!logRpcFailure("Failed to send message:", err, { reportSite: "chat.send" })) { toasts.add({ title: "Failed to send message", variant: "error" }); } throw err; @@ -5410,8 +5406,7 @@ function ChatInterface({ message, model, capsules, attachments, formats); onNavigateToChatRef.current(newChatId); } catch (err) { - if (isDurableObjectResetError(err)) reportDoResetError("chat.new", err); - if (!logRpcFailure("Failed to create new chat:", err)) { + if (!logRpcFailure("Failed to create new chat:", err, { reportSite: "chat.new" })) { toasts.add({ title: "Failed to start conversation", variant: "error" }); } throw err; diff --git a/packages/workshop-frontend/src/ConnectAccountModal.tsx b/packages/workshop-frontend/src/ConnectAccountModal.tsx index 7be36f61..7afeb75a 100644 --- a/packages/workshop-frontend/src/ConnectAccountModal.tsx +++ b/packages/workshop-frontend/src/ConnectAccountModal.tsx @@ -4,7 +4,6 @@ import { RpcStub } from 'capnweb' import { AuthenticatedApi, GatekeeperVendorFilter } from '@gadgets/workshop-shared/api' import { VendorDescription } from '@gadgets/workshop-shared/gatekeeper' import VendorCard from './VendorCard' -import { withDoResetRetry } from './rpcErrors' interface ConnectAccountModalProps { visible: boolean @@ -42,7 +41,7 @@ export default function ConnectAccountModal({ const fetchVendors = async () => { setVendorsLoading(true) try { - const vendorList = await withDoResetRetry(() => authenticatedApi.listGatekeeperVendors(filter)) + const vendorList = await authenticatedApi.listGatekeeperVendors(filter) const unavailable = vendorList.filter(v => v.unavailable) if (unavailable.length > 0) { toasts.add({ diff --git a/packages/workshop-frontend/src/GatekeeperModal.tsx b/packages/workshop-frontend/src/GatekeeperModal.tsx index 8e57587b..cd2eb6fe 100644 --- a/packages/workshop-frontend/src/GatekeeperModal.tsx +++ b/packages/workshop-frontend/src/GatekeeperModal.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from './rpcErrors' +import { logRpcFailure } from './rpcErrors' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Dialog, useKumoToastManager, type PortalContainer } from '@cloudflare/kumo' import { @@ -369,7 +369,7 @@ export default function GatekeeperModal({ setSpawnerEnv( (spawnerEnvCandidatesRef.current ?? []).map(entry => ({ ...entry, enabled: true }))) - withDoResetRetry(() => authenticatedApi.listModels()).then(models => { + authenticatedApi.listModels().then(models => { if (cancelled) return setAvailableModels(models) if (models.length > 0) { @@ -388,7 +388,7 @@ export default function GatekeeperModal({ toasts.add({ title: "Couldn't load AI models", variant: 'error' }) }) - withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()).then(vendors => { + authenticatedApi.listGatekeeperVendors().then(vendors => { if (cancelled) return setVendors(vendors) }).catch(err => { @@ -432,7 +432,7 @@ export default function GatekeeperModal({ } const subscriber = new AccountsSubscriber() - withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) + authenticatedApi.subscribeConnectedAccounts(subscriber) .then(stub => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/ObserverConfigModal.tsx b/packages/workshop-frontend/src/ObserverConfigModal.tsx index 32b5c716..4bfe5c37 100644 --- a/packages/workshop-frontend/src/ObserverConfigModal.tsx +++ b/packages/workshop-frontend/src/ObserverConfigModal.tsx @@ -1,4 +1,3 @@ -import { withDoResetRetry } from './rpcErrors' import { useState, useEffect, useRef } from 'react' import { Dialog, Select, Loader, Text, useKumoToastManager } from '@cloudflare/kumo' import { Warning, Plus, ArrowClockwise, CheckCircle } from '@phosphor-icons/react' @@ -144,8 +143,8 @@ export default function ObserverConfigModal({ } } - withDoResetRetry(() => authenticatedApi - .subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true })) + authenticatedApi + .subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true }) .then(stub => { if (cancelled) { stub[Symbol.dispose](); return } subStub = stub @@ -166,10 +165,10 @@ export default function ObserverConfigModal({ // ── load vendor metadata for display and resource-scope resolution ───────────── useEffect(() => { let cancelled = false - withDoResetRetry(() => Promise.all([ + Promise.all([ authenticatedApi.listGatekeeperVendors(), authenticatedApi.listAddableGatekeepers(), - ])) + ]) .then(([vendors, addable]) => { if (cancelled) return const map = new Map() diff --git a/packages/workshop-frontend/src/OnboardingWizard.tsx b/packages/workshop-frontend/src/OnboardingWizard.tsx index 6bc6d41e..9d4e0466 100644 --- a/packages/workshop-frontend/src/OnboardingWizard.tsx +++ b/packages/workshop-frontend/src/OnboardingWizard.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from './rpcErrors' +import { logRpcFailure } from './rpcErrors' import { useState, useEffect, useRef, useCallback } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' import { RpcTarget } from 'capnweb' @@ -120,10 +120,10 @@ export default function OnboardingWizard({ // Load models + AI config const fetchModels = useCallback(async () => { try { - const [modelList, cfg] = await withDoResetRetry(() => Promise.all([ + const [modelList, cfg] = await Promise.all([ authenticatedApi.listModels(), authenticatedApi.getAiConfig(), - ])) + ]) setModels(modelList) setAiConfig(cfg) // Default to the first model in the list @@ -221,7 +221,7 @@ export default function OnboardingWizard({ const subscriber = new AccountsSubscriber() let subscriptionStub: { [Symbol.dispose](): void } | null = null - withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) + authenticatedApi.subscribeConnectedAccounts(subscriber) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/ResourcePicker.tsx b/packages/workshop-frontend/src/ResourcePicker.tsx index 1340bb18..003d6946 100644 --- a/packages/workshop-frontend/src/ResourcePicker.tsx +++ b/packages/workshop-frontend/src/ResourcePicker.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from './rpcErrors' +import { logRpcFailure } from './rpcErrors' import { useState, useEffect, useRef, useMemo, useCallback, type MutableRefObject } from 'react' import { Tooltip, useKumoToastManager } from '@cloudflare/kumo' import { Plus, CaretRight, Warning } from '@phosphor-icons/react' @@ -159,8 +159,7 @@ export default function ResourcePicker({ const subscriber = new AccountsSubscriber() const subscribe = async () => { try { - const stub = await withDoResetRetry( - () => authenticatedApi.subscribeConnectedAccounts(subscriber)) + const stub = await authenticatedApi.subscribeConnectedAccounts(subscriber) if (cancelled) stub[Symbol.dispose]() else subscriptionRef.current = { stub } } catch (error) { diff --git a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx index 2d6ea839..b4df31d1 100644 --- a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx +++ b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from '../../rpcErrors' +import { logRpcFailure } from '../../rpcErrors' import { createContext, useCallback, @@ -90,7 +90,7 @@ export function SidebarWorkspacesProvider({ children }: { children: ReactNode }) useEffect(() => { let cancelled = false setGadgetsLoading(true) - withDoResetRetry(() => authenticatedApi.listGadgets()) + authenticatedApi.listGadgets() .then((list) => { if (cancelled) return setGadgets(list) diff --git a/packages/workshop-frontend/src/components/ConnectionChips.tsx b/packages/workshop-frontend/src/components/ConnectionChips.tsx index 4af6daf8..dec3b835 100644 --- a/packages/workshop-frontend/src/components/ConnectionChips.tsx +++ b/packages/workshop-frontend/src/components/ConnectionChips.tsx @@ -1,4 +1,3 @@ -import { withDoResetRetry } from '../rpcErrors' import { Plus } from '@phosphor-icons/react' import { Link } from '@tanstack/react-router' import { useAuthenticatedApi } from '../AuthContext' @@ -46,7 +45,7 @@ export default function ConnectionChips() { } const subscriber = new ChipsSubscriber() - const subPromise = withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) + const subPromise = authenticatedApi.subscribeConnectedAccounts(subscriber) subPromise.then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/homePromptFlow.test.tsx b/packages/workshop-frontend/src/homePromptFlow.test.tsx index 337d356a..ad401ad6 100644 --- a/packages/workshop-frontend/src/homePromptFlow.test.tsx +++ b/packages/workshop-frontend/src/homePromptFlow.test.tsx @@ -4,6 +4,7 @@ import { act } from "react"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { withReadRetries } from "./rpcErrors"; const testState = vi.hoisted(() => { const listModels = vi.fn<() => Promise>(async () => []); @@ -27,11 +28,18 @@ vi.mock("@cloudflare/kumo", () => ({ useKumoToastManager: () => ({ add: testState.addToast }), })); -vi.mock("./AuthContext", () => ({ - useAuthenticatedApi: () => ({ - authenticatedApi: testState.authenticatedApi, - }), -})); +vi.mock("./AuthContext", () => { + // The read-retry chokepoint wraps the stub where useAuth creates it; mirror that here so this + // suite exercises the same retry policy the app ships. Wrapped once — effects key on the + // stub's identity, so a fresh proxy per render would loop them. + let wrapped: unknown; + return { + useAuthenticatedApi: () => ({ + authenticatedApi: (wrapped ??= withReadRetries( + testState.authenticatedApi as unknown as Parameters[0])), + }), + }; +}); vi.mock("./ChatInterface", () => ({ ChatInput: ({ seedText, seedNonce }: { seedText?: string; seedNonce?: number }) => { diff --git a/packages/workshop-frontend/src/routes/__root.tsx b/packages/workshop-frontend/src/routes/__root.tsx index ba508712..b211cef6 100644 --- a/packages/workshop-frontend/src/routes/__root.tsx +++ b/packages/workshop-frontend/src/routes/__root.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from '../rpcErrors' +import { logRpcFailure } from '../rpcErrors' import { useState, useEffect } from 'react' import { createRootRoute, Outlet, useRouterState } from '@tanstack/react-router' import { TooltipProvider, Toasty } from '@cloudflare/kumo' @@ -152,7 +152,7 @@ function AuthenticatedShell({ useEffect(() => { let cancelled = false - withDoResetRetry(() => authenticatedApi.isOnboardingCompleted()).then((completed) => { + authenticatedApi.isOnboardingCompleted().then((completed) => { if (!cancelled) setOnboardingNeeded(!completed) }).catch((err) => { logRpcFailure('Failed to check onboarding status:', err) diff --git a/packages/workshop-frontend/src/routes/gatekeepers.tsx b/packages/workshop-frontend/src/routes/gatekeepers.tsx index ca323b46..36ec78a3 100644 --- a/packages/workshop-frontend/src/routes/gatekeepers.tsx +++ b/packages/workshop-frontend/src/routes/gatekeepers.tsx @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from '../rpcErrors' +import { logRpcFailure } from '../rpcErrors' import { createFileRoute } from '@tanstack/react-router' import { useEffect, useMemo, useRef, useState } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' @@ -483,7 +483,7 @@ function ConnectorsPage() { setAccountsLoaded(false) setVendorsLoaded(false) - withDoResetRetry(() => authenticatedApi.listAddableGatekeepers()) + authenticatedApi.listAddableGatekeepers() .then((list) => { if (!cancelled) setAddable(list) }) @@ -491,7 +491,7 @@ function ConnectorsPage() { logRpcFailure('Failed to load addable gatekeepers:', err) }) - withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()) + authenticatedApi.listGatekeeperVendors() .then((vendorList) => { if (cancelled) return const unavailable = vendorList.filter((v) => v.unavailable) @@ -551,7 +551,7 @@ function ConnectorsPage() { const subscriber = new AccountsSubscriber() - withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber)) + authenticatedApi.subscribeConnectedAccounts(subscriber) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() diff --git a/packages/workshop-frontend/src/routes/index.tsx b/packages/workshop-frontend/src/routes/index.tsx index a6a37560..36ecbb0a 100644 --- a/packages/workshop-frontend/src/routes/index.tsx +++ b/packages/workshop-frontend/src/routes/index.tsx @@ -1,4 +1,4 @@ -import { isDurableObjectResetError, logRpcFailure, reportDoResetError, withDoResetRetry } from "../rpcErrors"; +import { classifyRpcError, logRpcFailure } from "../rpcErrors"; import { useState, useEffect, useRef, useCallback } from "react"; import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { useKumoToastManager } from "@cloudflare/kumo"; @@ -58,15 +58,17 @@ export function HomePageContent({ prompt }: HomeSearch) { useEffect(() => { let cancelled = false; - withDoResetRetry(() => authenticatedApi.listModels()) + authenticatedApi.listModels() .then((list) => { if (cancelled) return; setModels(list); setSelectedModel(getStoredSelectedModel(list)); }) .catch((err) => { - const transient = logRpcFailure("Failed to fetch models:", err); - if (!transient || isDurableObjectResetError(err)) { + logRpcFailure("Failed to fetch models:", err); + // Toast unless it's a connection error (reconnect refetches); a do-reset here already + // survived the read retry, so the user should hear about it. + if (classifyRpcError(err) !== "connection") { toasts.add({ title: "Couldn't load AI models", variant: "error" }); } }); @@ -119,8 +121,8 @@ export function HomePageContent({ prompt }: HomeSearch) { // Open the conversation we just started. navigate({ to: "/workspace/$id", params: { id }, search: { chat } }); } catch (err) { - if (isDurableObjectResetError(err)) reportDoResetError("workspace.create", err); - const transient = logRpcFailure("Failed to create gadget:", err); + const transient = logRpcFailure("Failed to create gadget:", err, + { reportSite: "workspace.create" }); // A retry reuses the provisional gadget while the draft contains gadget-scoped references. if (!attachments?.length && !capsules?.length) { provisionalOverseerRef.current?.stub[Symbol.dispose](); diff --git a/packages/workshop-frontend/src/routes/providers.tsx b/packages/workshop-frontend/src/routes/providers.tsx index ef6e1c95..16e16e04 100644 --- a/packages/workshop-frontend/src/routes/providers.tsx +++ b/packages/workshop-frontend/src/routes/providers.tsx @@ -18,7 +18,6 @@ import { import AddModelModal from '../AddModelModal' import { useDocumentTitle } from '../useDocumentTitle' import { MENU_CONTENT, MENU_ITEM, MENU_ITEM_DANGER } from '../components/menuStyles' -import { withDoResetRetry } from '../rpcErrors' export const Route = createFileRoute('/providers')({ component: ProvidersPage }) @@ -149,8 +148,8 @@ function ProvidersPage() { setLoadError(false) try { const [modelList, qm, cfg] = await Promise.all([ - withDoResetRetry(() => authenticatedApi.listModels()), - withDoResetRetry(() => authenticatedApi.getQuickModel()), + authenticatedApi.listModels(), + authenticatedApi.getQuickModel(), authenticatedApi.getAiConfig(), ]) setModels(modelList) diff --git a/packages/workshop-frontend/src/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts index f3e45739..d96b3616 100644 --- a/packages/workshop-frontend/src/rpcErrors.test.ts +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -3,7 +3,7 @@ import { readFileSync } from 'node:fs' // @ts-expect-error node builtin without @types/node import { createRequire } from 'node:module' -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { deserialize, serialize } from 'capnweb' import { AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api' @@ -13,12 +13,15 @@ import { reportIssue } from './errorReporting' import { classifyRpcError, getDurableObjectId, isDurableObjectResetError, isOverloadedError, CONNECTION_MESSAGES, isTransientRpcError, logRpcFailure, reportDoResetError, withDoResetRetry, + withReadRetries, } from './rpcErrors' +const rpcError = (message: string, props?: object) => Object.assign(new Error(message), props) + // The reject frame observed in prod for a DO storage-timeout reset. function storageTimeoutReset() { - return Object.assign( - new Error('Durable Object storage operation exceeded timeout which caused object to be reset.'), + return rpcError( + 'Durable Object storage operation exceeded timeout which caused object to be reset.', { remote: true, overloaded: true, durableObjectReset: true, durableObjectId: 'eed0859e' }, ) } @@ -29,8 +32,7 @@ describe('classifyRpcError', () => { }) it('trusts the durableObjectReset flag over an unrecognized message', () => { - const err = Object.assign(new Error('internal error'), { durableObjectReset: true }) - expect(classifyRpcError(err)).toBe('do-reset') + expect(classifyRpcError(rpcError('internal error', { durableObjectReset: true }))).toBe('do-reset') }) it('falls back to known reset messages without flags', () => { @@ -46,12 +48,11 @@ describe('classifyRpcError', () => { }) it('prefers do-reset when both reset and retryable flags are set', () => { - const err = Object.assign(new Error('x'), { durableObjectReset: true, retryable: true }) - expect(classifyRpcError(err)).toBe('do-reset') + expect(classifyRpcError(rpcError('x', { durableObjectReset: true, retryable: true }))).toBe('do-reset') }) it('classifies the retryable flag as connection', () => { - expect(classifyRpcError(Object.assign(new Error('x'), { retryable: true }))).toBe('connection') + expect(classifyRpcError(rpcError('x', { retryable: true }))).toBe('connection') }) it('classifies capnweb transport messages as connection', () => { @@ -66,8 +67,7 @@ describe('classifyRpcError', () => { it('classifies auth failures, which must never be retried or quieted', () => { // Coded errors are authoritative; bare messages are the fallback for older deployments. expect(classifyRpcError(createAuthError(AUTH_ERROR_CODES.invalidSessionToken))).toBe('auth') - expect(classifyRpcError(Object.assign(new Error('nope'), { code: 'INVALID_SESSION_TOKEN' }))) - .toBe('auth') + expect(classifyRpcError(rpcError('nope', { code: 'INVALID_SESSION_TOKEN' }))).toBe('auth') expect(classifyRpcError(new Error('invalid session token'))).toBe('auth') expect(classifyRpcError(new Error('Not authenticated with Access.'))).toBe('auth') }) @@ -114,17 +114,18 @@ describe('reportDoResetError', () => { }) describe('withDoResetRetry', () => { - it('retries once after a reset error', async () => { + afterEach(() => vi.useRealTimers()) + + it.each([ + ['reset', storageTimeoutReset()], + ['retryable-flagged invocation', rpcError('internal error', { remote: true, retryable: true })], + ])('retries once after a %s failure', async (_kind, failure) => { vi.useFakeTimers() - try { - const fn = vi.fn<() => Promise>().mockRejectedValueOnce(storageTimeoutReset()).mockResolvedValueOnce('ok') - const result = withDoResetRetry(fn) - await vi.advanceTimersByTimeAsync(2000) - expect(await result).toBe('ok') - expect(fn).toHaveBeenCalledTimes(2) - } finally { - vi.useRealTimers() - } + const fn = vi.fn<() => Promise>().mockRejectedValueOnce(failure).mockResolvedValueOnce('ok') + const result = withDoResetRetry(fn) + await vi.advanceTimersByTimeAsync(2000) + expect(await result).toBe('ok') + expect(fn).toHaveBeenCalledTimes(2) }) it('does not retry non-reset errors', async () => { @@ -133,21 +134,6 @@ describe('withDoResetRetry', () => { expect(fn).toHaveBeenCalledTimes(1) }) - it('retries once on a retryable-flagged invocation failure', async () => { - vi.useFakeTimers() - try { - const fn = vi.fn<() => Promise>() - .mockRejectedValueOnce(Object.assign(new Error('internal error'), { remote: true, retryable: true })) - .mockResolvedValueOnce('ok') - const result = withDoResetRetry(fn) - await vi.advanceTimersByTimeAsync(2000) - expect(await result).toBe('ok') - expect(fn).toHaveBeenCalledTimes(2) - } finally { - vi.useRealTimers() - } - }) - // Local transport errors carry no flags; their recovery belongs to the connection manager, // so the retry must refuse them even though they classify as transient. it('does not retry flagless transport errors', async () => { @@ -158,16 +144,39 @@ describe('withDoResetRetry', () => { it('gives up after the second failure', async () => { vi.useFakeTimers() - try { - const fn = vi.fn<() => Promise>().mockRejectedValue(storageTimeoutReset()) - const result = withDoResetRetry(fn) - result.catch(() => {}) - await vi.advanceTimersByTimeAsync(2000) - await expect(result).rejects.toThrow('exceeded timeout') - expect(fn).toHaveBeenCalledTimes(2) - } finally { - vi.useRealTimers() - } + const fn = vi.fn<() => Promise>().mockRejectedValue(storageTimeoutReset()) + const result = withDoResetRetry(fn) + result.catch(() => {}) + await vi.advanceTimersByTimeAsync(2000) + await expect(result).rejects.toThrow('exceeded timeout') + expect(fn).toHaveBeenCalledTimes(2) + }) +}) + +// The chokepoint installed by useAuth: listed idempotent reads retry via withDoResetRetry, +// everything else passes through untouched. +describe('withReadRetries', () => { + afterEach(() => vi.useRealTimers()) + + const wrap = (methods: object) => + withReadRetries(methods as unknown as Parameters[0]) + + it('retries a listed read once after a reset', async () => { + vi.useFakeTimers() + const listModels = vi.fn<() => Promise>() + .mockRejectedValueOnce(storageTimeoutReset()).mockResolvedValueOnce([]) + const api = wrap({ listModels }) + const result = api.listModels() + await vi.advanceTimersByTimeAsync(2000) + expect(await result).toEqual([]) + expect(listModels).toHaveBeenCalledTimes(2) + }) + + it('passes writes through with no retry', async () => { + const setQuickModel = vi.fn<() => Promise>().mockRejectedValue(storageTimeoutReset()) + const api = wrap({ setQuickModel }) + await expect(api.setQuickModel('m')).rejects.toThrow('exceeded timeout') + expect(setQuickModel).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/workshop-frontend/src/rpcErrors.ts b/packages/workshop-frontend/src/rpcErrors.ts index 94d65633..1d1112ea 100644 --- a/packages/workshop-frontend/src/rpcErrors.ts +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -1,4 +1,8 @@ -import { AUTH_ERROR_MESSAGES, getAuthErrorCode } from '@gadgets/workshop-shared/api' +import type { RpcStub } from 'capnweb' +import { + AUTH_ERROR_MESSAGES, getAuthErrorCode, WORKERD_DEAD_CAPABILITY_MESSAGE, + type AuthenticatedApi, +} from '@gadgets/workshop-shared/api' import { reportIssue } from './errorReporting' // Classifies errors surfaced through capnweb RPC. The backend runs with @@ -11,15 +15,15 @@ export type RpcErrorClass = 'do-reset' | 'connection' | 'auth' | 'other' // Fallbacks: workerd errors normally arrive with `durableObjectReset` set (capnweb carries // the flags in a dedicated slot); the first four strings only matter when something re-wrapped -// the error. The last is what LATER calls on an already-dead capability reject with — flagless -// (verified in a workerd probe); the flagged error only reaches calls in flight at reset time. -// Over our RPC surface a dead hosting context always means the capability needs reopening. +// the error. The last is what LATER calls on an already-dead capability reject with — flagless; +// the flagged error only reaches calls in flight at reset time. Over our RPC surface a dead +// hosting context always means the capability needs reopening. const DO_RESET_MESSAGES = [ 'Durable Object reset because its code was updated', 'Durable Object storage operation exceeded timeout', "Durable Object's isolate exceeded its memory limit", 'Durable Object exceeded its CPU time limit', - 'The execution context which hosts this callback is no longer running', + WORKERD_DEAD_CAPABILITY_MESSAGE, ] // Transport failures raised locally by capnweb, plus its own-session teardown message. These @@ -77,8 +81,14 @@ export function isTransientRpcError(err: unknown): boolean { // Logs an RPC failure: quietly for transient errors (a retry or reconnect is expected to cure // them), loudly otherwise. Returns true when transient so call sites can skip their toasts. -export function logRpcFailure(message: string, err: unknown): boolean { - const transient = isTransientRpcError(err) +// Pass `reportSite` from action paths (sends, creates) to also report do-reset errors to the +// client-errors endpoint, so resets that cost the user an action stay visible in telemetry. +export function logRpcFailure( + message: string, err: unknown, options?: { reportSite?: string }, +): boolean { + const cls = classifyRpcError(err) + if (cls === 'do-reset' && options?.reportSite) reportDoResetError(options.reportSite, err) + const transient = cls === 'do-reset' || cls === 'connection' if (transient) console.debug(message, err) else console.error(message, err) return transient @@ -108,3 +118,37 @@ export async function withDoResetRetry(fn: () => Promise, delayMs = 1500): export function reportDoResetError(site: string, err: unknown, options?: { gadgetId?: string }) { reportIssue(`do-reset.${site}`, err, { severity: 'warning', handled: true, ...options }) } + +// The AuthenticatedApi methods that withReadRetries retries: idempotent reads whose results UI +// effects re-fetch anyway. Method-level, because idempotency is a property of the method, not +// of any one call site. Writes are deliberately absent — a retry after an ambiguous failure +// could double-apply them. +const RETRYING_READS = new Set([ + 'listModels', + 'getQuickModel', + 'getAiConfig', + 'listGadgets', + 'listGatekeeperVendors', + 'listAddableGatekeepers', + 'isOnboardingCompleted', + 'subscribeConnectedAccounts', +]) + +// Wraps the authenticated API stub so the reads above transparently retry once after a +// backend-side transient failure (see withDoResetRetry). Installed once where the stub is +// created (useAuth), so every consumer gets the policy for free; everything else passes +// through untouched. All function-valued properties are forwarded via a fresh method call on +// the target: capnweb's callable property proxies treat property access (`.bind`, `.apply`) +// as RPC path segments, and its real methods (`then`, dispose) need the stub as receiver. +export function withReadRetries(api: RpcStub): RpcStub { + const target = api as unknown as Record unknown> + return new Proxy(api, { + get(_, prop) { + const value = Reflect.get(api, prop) + if (typeof value !== 'function') return value + if (!RETRYING_READS.has(prop)) return (...args: unknown[]) => target[prop](...args) + return (...args: unknown[]) => + withDoResetRetry(() => target[prop](...args) as Promise) + }, + }) +} diff --git a/packages/workshop-frontend/src/useAuth.ts b/packages/workshop-frontend/src/useAuth.ts index a4a28087..abf4a487 100644 --- a/packages/workshop-frontend/src/useAuth.ts +++ b/packages/workshop-frontend/src/useAuth.ts @@ -1,6 +1,7 @@ import { useState, useEffect, useRef } from 'react' import { RpcStub } from 'capnweb' import { PublicApi, AuthenticatedApi } from '@gadgets/workshop-shared/api' +import { withReadRetries } from './rpcErrors' const CF_ACCESS_MODE = import.meta.env.VITE_CF_ACCESS_MODE === 'true' @@ -55,7 +56,7 @@ export function useAuth(publicApi: RpcStub) { // Use promise pipelining - no need to await. The CF Access JWT is already attached // to the request by the browser (injected by the Access service worker/cookie), so // the server validates it and returns an authenticated stub immediately. - const authenticatedApi = publicApi.authenticateFromCfAccess() + const authenticatedApi = withReadRetries(publicApi.authenticateFromCfAccess()) setAuthState({ token: null, authenticatedApi, @@ -80,7 +81,8 @@ export function useAuth(publicApi: RpcStub) { // Use promise pipelining - we can use the returned promise as a stub immediately // without awaiting. Authentication errors will be handled when the stub is actually used. - const authenticatedApi = publicApi.authenticate(token) + // withReadRetries makes the idempotent reads retry once after a Durable Object reset. + const authenticatedApi = withReadRetries(publicApi.authenticate(token)) setAuthState({ token, authenticatedApi, diff --git a/packages/workshop-frontend/src/useVendorBranding.ts b/packages/workshop-frontend/src/useVendorBranding.ts index bfdc41ed..d054c62f 100644 --- a/packages/workshop-frontend/src/useVendorBranding.ts +++ b/packages/workshop-frontend/src/useVendorBranding.ts @@ -1,4 +1,4 @@ -import { logRpcFailure, withDoResetRetry } from './rpcErrors' +import { logRpcFailure } from './rpcErrors' import { useEffect, useState } from 'react' import { RpcStub } from 'capnweb' import { AuthenticatedApi } from '@gadgets/workshop-shared/api' @@ -32,7 +32,7 @@ export function useVendorBranding( if (!promise) { const api = authenticatedApi promise = (async () => { - const vendors = await withDoResetRetry(() => api.listGatekeeperVendors()) + const vendors = await api.listGatekeeperVendors() const map = new Map() for (const vendor of vendors) { const { logo, color } = vendor.description diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index 14916a63..68877ae5 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -251,6 +251,22 @@ export interface ObserverConfigCallback extends RpcTarget { configure(needs: ObserverBindingNeed[]): Promise; } +/** Builds the create/read helpers for a family of expected errors carrying stable + * machine-readable codes. The per-code messages double as the classification fallback for errors + * from older deployments that lost the code in transit, so changing one is a compatibility break. */ +function codedErrorFamily(messages: Record) { + const codes = new Set(Object.keys(messages)); + return { + create: (code: Code): Error & { code: Code } => + Object.assign(new Error(messages[code]), { code }), + getCode: (error: unknown): Code | undefined => { + const candidate = typeof error === "object" && error !== null && "code" in error + ? error.code : undefined; + return codes.has(candidate) ? candidate as Code : undefined; + }, + }; +} + /** Stable error codes attached to expected failures from `AuthenticatedApi.openGadget()`. */ export const OPEN_GADGET_ERROR_CODES = { workspaceNotFound: "WORKSPACE_NOT_FOUND", @@ -261,29 +277,16 @@ export const OPEN_GADGET_ERROR_CODES = { export type OpenGadgetErrorCode = typeof OPEN_GADGET_ERROR_CODES[keyof typeof OPEN_GADGET_ERROR_CODES]; -const OPEN_GADGET_ERROR_MESSAGES: Record = { +const openGadgetErrors = codedErrorFamily({ [OPEN_GADGET_ERROR_CODES.workspaceNotFound]: "Workspace not found.", [OPEN_GADGET_ERROR_CODES.workspaceAccessDenied]: "You don't have access to this workspace.", -}; +}); /** Creates an expected `openGadget()` error with a machine-readable code. */ -export function createOpenGadgetError( - code: OpenGadgetErrorCode): Error & { code: OpenGadgetErrorCode } { - return Object.assign(new Error(OPEN_GADGET_ERROR_MESSAGES[code]), { code }); -} +export const createOpenGadgetError = openGadgetErrors.create; /** Reads the machine-readable code from an expected `openGadget()` error. */ -export function getOpenGadgetErrorCode(error: unknown): OpenGadgetErrorCode | undefined { - if (typeof error !== "object" || error === null) return undefined; - - const candidate = "code" in error ? error.code : undefined; - return isOpenGadgetErrorCode(candidate) ? candidate : undefined; -} - -function isOpenGadgetErrorCode(value: unknown): value is OpenGadgetErrorCode { - return value === OPEN_GADGET_ERROR_CODES.workspaceNotFound || - value === OPEN_GADGET_ERROR_CODES.workspaceAccessDenied; -} +export const getOpenGadgetErrorCode = openGadgetErrors.getCode; /** Stable error codes attached to authentication failures. */ export const AUTH_ERROR_CODES = { @@ -295,29 +298,26 @@ export const AUTH_ERROR_CODES = { export type AuthErrorCode = typeof AUTH_ERROR_CODES[keyof typeof AUTH_ERROR_CODES]; /** Messages for auth failures thrown without a surviving code; clients match these only as a - * classification fallback, so changing one is a compatibility break with older deployments. */ + * classification fallback. */ export const AUTH_ERROR_MESSAGES: Record = { [AUTH_ERROR_CODES.invalidSessionToken]: "invalid session token", [AUTH_ERROR_CODES.notAuthenticatedWithAccess]: "Not authenticated with Access.", }; +const authErrors = codedErrorFamily(AUTH_ERROR_MESSAGES); + /** Creates an authentication failure with a machine-readable code. */ -export function createAuthError(code: AuthErrorCode): Error & { code: AuthErrorCode } { - return Object.assign(new Error(AUTH_ERROR_MESSAGES[code]), { code }); -} +export const createAuthError = authErrors.create; /** Reads the machine-readable code from an authentication failure. */ -export function getAuthErrorCode(error: unknown): AuthErrorCode | undefined { - if (typeof error !== "object" || error === null) return undefined; - - const candidate = "code" in error ? error.code : undefined; - return isAuthErrorCode(candidate) ? candidate : undefined; -} - -function isAuthErrorCode(value: unknown): value is AuthErrorCode { - return value === AUTH_ERROR_CODES.invalidSessionToken || - value === AUTH_ERROR_CODES.notAuthenticatedWithAccess; -} +export const getAuthErrorCode = authErrors.getCode; + +/** What calls on a capability whose hosting Durable Object already reset reject with. It arrives + * flagless — the flagged `durableObjectReset` error only reaches calls in flight at reset time — + * and so proves the call never reached the object (verified in a workerd probe). Matched by the + * backend's never-sent retry and the frontend classifier; the frontend canary test pins it. */ +export const WORKERD_DEAD_CAPABILITY_MESSAGE = + "The execution context which hosts this callback is no longer running"; // Top-level API exposed to the user after they have authenticated. export interface AuthenticatedApi extends RpcTarget {