diff --git a/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts b/packages/workshop-backend/__integration__/open-gadget-rpc.test.ts index 5df6aa53..df771951 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,46 @@ 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. 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 () => { + 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. 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 6ffb384c..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 } 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"; @@ -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,71 @@ 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; + + private get user(): DurableObjectStub { + 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. + // 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 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().", + ]; + + // 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 { + return new Proxy(stub, { + get: (target, prop) => { + 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.rawUserStub === target) this.userStub = this.rawUserStub = undefined; + if (neverSent) { + // 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; + }); + }; + }, + }); + } + #isAdmin(): boolean { let name = this.user.id.name; let admins = this.env.ADMINS; @@ -664,7 +729,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]); @@ -675,12 +740,12 @@ 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 { if (!this.accessPayload) { - throw new Error("Not authenticated with Access."); + throw createAuthError(AUTH_ERROR_CODES.notAuthenticatedWithAccess); } let email = this.accessPayload.email as string; @@ -700,7 +765,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-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/BlueprintLandingPage.tsx b/packages/workshop-frontend/src/BlueprintLandingPage.tsx index 16226ae2..e452bc49 100644 --- a/packages/workshop-frontend/src/BlueprintLandingPage.tsx +++ b/packages/workshop-frontend/src/BlueprintLandingPage.tsx @@ -1,3 +1,4 @@ +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' @@ -120,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) + authenticatedApi.listModels() + .then(setModels) + .catch(err => logRpcFailure('Failed to load models:', err)) } else { setModels([]) } @@ -190,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..15b8c2ea 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -1,3 +1,4 @@ +import { isTransientRpcError, logRpcFailure } 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,10 @@ export const ChatInput = ({ const [capsules, setCapsules] = useState([]); const [pendingAttachments, setPendingAttachments] = useState([]); const [isSending, setIsSending] = useState(false); + // 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 @@ -2276,6 +2284,7 @@ export const ChatInput = ({ const handleSend = async () => { if (sendInFlightRef.current || isSending || isBlocked) return; + setSendHiccup(null); const attachmentsSnapshot = pendingAttachments; const readyAttachments = attachmentsSnapshot .filter((attachment) => attachment.uploadState === "ready" && attachment.ref) @@ -2454,8 +2463,10 @@ 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)) setSendHiccup({ chatKey: submittedChatKey }); }); }; @@ -3050,6 +3061,11 @@ export const ChatInput = ({ )} {draftUpdateBanner} + {sendHiccup && sendHiccup.chatKey === chatKey && ( +
+ Connection hiccup — your message may not have been sent. Check the thread, then try again. +
+ )} {/* Textarea */}
{slashCommandPicker.popup} @@ -5213,9 +5229,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 +5383,9 @@ function ChatInterface({ ); } } catch (err) { - console.error("Failed to send message:", err); - toasts.add({ title: "Failed to send message", variant: "error" }); + if (!logRpcFailure("Failed to send message:", err, { reportSite: "chat.send" })) { + toasts.add({ title: "Failed to send message", variant: "error" }); + } throw err; } }; @@ -5388,8 +5406,9 @@ 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 (!logRpcFailure("Failed to create new chat:", err, { reportSite: "chat.new" })) { + toasts.add({ title: "Failed to start conversation", variant: "error" }); + } throw err; } }; @@ -7576,6 +7595,7 @@ function ChatInterface({
overseer.newGatekeeper(accountId, url) } 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 0706d33e..cd2eb6fe 100644 --- a/packages/workshop-frontend/src/GatekeeperModal.tsx +++ b/packages/workshop-frontend/src/GatekeeperModal.tsx @@ -1,3 +1,4 @@ +import { logRpcFailure } from './rpcErrors' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Dialog, useKumoToastManager, type PortalContainer } from '@cloudflare/kumo' import { @@ -440,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 22f5755f..4bfe5c37 100644 --- a/packages/workshop-frontend/src/ObserverConfigModal.tsx +++ b/packages/workshop-frontend/src/ObserverConfigModal.tsx @@ -150,6 +150,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 6e305c21..9d4e0466 100644 --- a/packages/workshop-frontend/src/OnboardingWizard.tsx +++ b/packages/workshop-frontend/src/OnboardingWizard.tsx @@ -1,3 +1,4 @@ +import { logRpcFailure } from './rpcErrors' import { useState, useEffect, useRef, useCallback } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' import { RpcTarget } from 'capnweb' @@ -220,8 +221,7 @@ export default function OnboardingWizard({ const subscriber = new AccountsSubscriber() let subscriptionStub: { [Symbol.dispose](): void } | null = null - authenticatedApi - .subscribeConnectedAccounts(subscriber) + authenticatedApi.subscribeConnectedAccounts(subscriber) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() @@ -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.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..003d6946 100644 --- a/packages/workshop-frontend/src/ResourcePicker.tsx +++ b/packages/workshop-frontend/src/ResourcePicker.tsx @@ -1,3 +1,4 @@ +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' @@ -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 { @@ -158,16 +160,18 @@ export default function ResourcePicker({ const subscribe = async () => { try { const stub = await authenticatedApi.subscribeConnectedAccounts(subscriber) - subscriptionRef.current = { stub } + 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. - 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..b4df31d1 100644 --- a/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx +++ b/packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx @@ -1,3 +1,4 @@ +import { logRpcFailure } from '../../rpcErrors' import { createContext, useCallback, @@ -89,15 +90,14 @@ export function SidebarWorkspacesProvider({ children }: { children: ReactNode }) useEffect(() => { let cancelled = false setGadgetsLoading(true) - authenticatedApi - .listGadgets() + authenticatedApi.listGadgets() .then((list) => { if (cancelled) return setGadgets(list) 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..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 }) => { @@ -57,7 +65,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 +83,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 97eade90..b211cef6 100644 --- a/packages/workshop-frontend/src/routes/__root.tsx +++ b/packages/workshop-frontend/src/routes/__root.tsx @@ -1,3 +1,4 @@ +import { logRpcFailure } from '../rpcErrors' import { useState, useEffect } from 'react' import { createRootRoute, Outlet, useRouterState } from '@tanstack/react-router' import { TooltipProvider, Toasty } from '@cloudflare/kumo' @@ -154,7 +155,7 @@ function AuthenticatedShell({ 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 85a22090..36ec78a3 100644 --- a/packages/workshop-frontend/src/routes/gatekeepers.tsx +++ b/packages/workshop-frontend/src/routes/gatekeepers.tsx @@ -1,3 +1,4 @@ +import { logRpcFailure } from '../rpcErrors' import { createFileRoute } from '@tanstack/react-router' import { useEffect, useMemo, useRef, useState } from 'react' import { useKumoToastManager } from '@cloudflare/kumo' @@ -482,17 +483,15 @@ function ConnectorsPage() { setAccountsLoaded(false) setVendorsLoaded(false) - authenticatedApi - .listAddableGatekeepers() + authenticatedApi.listAddableGatekeepers() .then((list) => { if (!cancelled) setAddable(list) }) .catch((err) => { - console.error('Failed to load addable gatekeepers:', err) + logRpcFailure('Failed to load addable gatekeepers:', err) }) - authenticatedApi - .listGatekeeperVendors() + authenticatedApi.listGatekeeperVendors() .then((vendorList) => { if (cancelled) return const unavailable = vendorList.filter((v) => v.unavailable) @@ -514,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) }) @@ -552,8 +551,7 @@ function ConnectorsPage() { const subscriber = new AccountsSubscriber() - authenticatedApi - .subscribeConnectedAccounts(subscriber) + authenticatedApi.subscribeConnectedAccounts(subscriber) .then((stub) => { if (cancelled) { stub[Symbol.dispose]() @@ -562,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 9506b75b..36ecbb0a 100644 --- a/packages/workshop-frontend/src/routes/index.tsx +++ b/packages/workshop-frontend/src/routes/index.tsx @@ -1,3 +1,4 @@ +import { classifyRpcError, logRpcFailure } from "../rpcErrors"; import { useState, useEffect, useRef, useCallback } from "react"; import { createFileRoute, useNavigate } from "@tanstack/react-router"; import { useKumoToastManager } from "@cloudflare/kumo"; @@ -57,16 +58,19 @@ export function HomePageContent({ prompt }: HomeSearch) { useEffect(() => { let cancelled = false; - authenticatedApi - .listModels() + authenticatedApi.listModels() .then((list) => { if (cancelled) return; setModels(list); setSelectedModel(getStoredSelectedModel(list)); }) .catch((err) => { - console.error("Failed to fetch models:", err); - toasts.add({ title: "Couldn't load AI models", variant: "error" }); + 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" }); + } }); return () => { cancelled = true; @@ -117,13 +121,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); + 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](); 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/rpcErrors.test.ts b/packages/workshop-frontend/src/rpcErrors.test.ts new file mode 100644 index 00000000..d96b3616 --- /dev/null +++ b/packages/workshop-frontend/src/rpcErrors.test.ts @@ -0,0 +1,228 @@ +// 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 { afterEach, 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<(site: string, err: unknown, options?: object) => void>() })) + +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 rpcError( + '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', () => { + expect(classifyRpcError(rpcError('internal error', { durableObjectReset: true }))).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.', + // 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') + } + }) + + it('prefers do-reset when both reset and retryable flags are set', () => { + expect(classifyRpcError(rpcError('x', { durableObjectReset: true, retryable: true }))).toBe('do-reset') + }) + + it('classifies the retryable flag as connection', () => { + expect(classifyRpcError(rpcError('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') + 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(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') + }) + + 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' }) + }) +}) + +describe('withDoResetRetry', () => { + 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() + 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 () => { + const fn = vi.fn<() => Promise>().mockRejectedValue(new Error('Workspace not found.')) + await expect(withDoResetRetry(fn)).rejects.toThrow('Workspace not found.') + expect(fn).toHaveBeenCalledTimes(1) + }) + + // 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() + 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) + }) +}) + +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() + } + }) +}) + +// 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 new file mode 100644 index 00000000..1d1112ea --- /dev/null +++ b/packages/workshop-frontend/src/rpcErrors.ts @@ -0,0 +1,154 @@ +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 +// `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' + +// 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; +// 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', + WORKERD_DEAD_CAPABILITY_MESSAGE, +] + +// 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', +] + +// 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)) + +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' + } + // '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' +} + +// 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' +} + +// 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. +// 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 +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + +// 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) && !flag(err, 'retryable')) 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 }) +} + +// 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 d9c39e7b..d054c62f 100644 --- a/packages/workshop-frontend/src/useVendorBranding.ts +++ b/packages/workshop-frontend/src/useVendorBranding.ts @@ -1,3 +1,4 @@ +import { logRpcFailure } from './rpcErrors' import { useEffect, useState } from 'react' import { RpcStub } from 'capnweb' import { AuthenticatedApi } from '@gadgets/workshop-shared/api' @@ -50,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 } diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index 7cee2e78..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,47 @@ 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; +export const getOpenGadgetErrorCode = openGadgetErrors.getCode; - const candidate = "code" in error ? error.code : undefined; - return isOpenGadgetErrorCode(candidate) ? candidate : undefined; -} +/** Stable error codes attached to authentication failures. */ +export const AUTH_ERROR_CODES = { + invalidSessionToken: "INVALID_SESSION_TOKEN", + notAuthenticatedWithAccess: "NOT_AUTHENTICATED_WITH_ACCESS", +} as const; -function isOpenGadgetErrorCode(value: unknown): value is OpenGadgetErrorCode { - return value === OPEN_GADGET_ERROR_CODES.workspaceNotFound || - value === OPEN_GADGET_ERROR_CODES.workspaceAccessDenied; -} +/** 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. */ +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 const createAuthError = authErrors.create; + +/** Reads the machine-readable code from an authentication failure. */ +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 {