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

Author: ndisidore

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<AuthenticatedApi> {
     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<AuthenticatedApi> {
     if (!this.accessPayload) {
-      throw new Error("Not authenticated with Access.");
+      throw createAuthError(AUTH_ERROR_CODES.notAuthenticatedWithAccess);
     }
 
     let email = this.accessPayload.email as string;

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<Cloudflare.Env> {
   }
 
   async authenticate(token: string): Promise<void> {
-    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);
     }
   }
 

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<string>>().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<string>>().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<string>>()
+        .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<string>>().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<string>>().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<string, unknown>
+    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')
+  })
+})

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<T>(fn: () => Promise<T>, delayMs = 1500): Promise<T> {
   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()
   }

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<AuthErrorCode, string> = {
+  [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.