Skip to content

Commit 7998195

Browse files
committed
Simplification pass over the DO-reset resilience work
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.
1 parent b2e88a9 commit 7998195

20 files changed

Lines changed: 224 additions & 174 deletions

packages/workshop-backend/src/server.ts

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { RpcStub, RpcTarget, newWorkersRpcResponse } from "capnweb";
22
import { validateRpc } from "capnweb-validate";
33
import type { JWTPayload } from "jose";
4-
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';
4+
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';
55
import type { UiFeatureFlags } from "@gadgets/workshop-shared/feature-flags";
66
import { getServerConfig } from "./deployment-config.js";
77
import { isPasswordAuthEnabled, getAuthGatekeeperAllowlist } from "./auth/config.js";
@@ -71,10 +71,6 @@ type Env = Cloudflare.Env & {
7171

7272
// =======================================================================================
7373

74-
// Escape hatch on the wrapped user-DO stub (see #wrapUserStub) exposing its raw target, so the
75-
// never-sent retry path can re-issue a call without re-entering the retry wrapper.
76-
const RAW_USER_STUB = Symbol("rawUserStub");
77-
7874
@validateRpc()
7975
class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi {
8076
constructor(private ctx: ExecutionContext, private env: Env,
@@ -91,25 +87,26 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi {
9187
private adminSettings: DurableObjectNamespace<AdminSettings>;
9288
private users: DurableObjectNamespace<UserDurableObject>;
9389

90+
// One stub per session, because e-order (in-order delivery to the DO) is guaranteed per stub.
91+
// A stub is permanently broken once its incarnation of the object resets, so the wrapper below
92+
// drops both fields on a stub-breaking rejection and the next call re-resolves fresh ones.
93+
// `rawUserStub` is the Proxy's target, kept so the never-sent retry can re-issue a call
94+
// without re-entering the wrapper.
9495
private userStub?: DurableObjectStub<UserDurableObject>;
96+
private rawUserStub?: DurableObjectStub<UserDurableObject>;
9597

96-
// E-order (in-order delivery to the DO) is guaranteed per stub, so the session shares one stub
97-
// while it's healthy. But a stub is bound to one incarnation of the object and is permanently
98-
// broken once that incarnation resets, so the wrapper below drops the cached stub the moment a
99-
// call through it fails with a stub-breaking error, and the next call re-resolves a fresh one.
10098
private get user(): DurableObjectStub<UserDurableObject> {
101-
return this.userStub ??= this.#wrapUserStub(this.users.get(this.userId));
99+
return this.userStub ??=
100+
this.#wrapUserStub(this.rawUserStub = this.users.get(this.userId));
102101
}
103102

104103
// Calls on a stub whose DO already reset while idle reject flagless with the brokenness
105104
// reason; the flagged (`durableObjectReset`) error only reaches calls in flight at reset time.
106-
// Verified in a workerd probe — see DO_RESET_MESSAGES in workshop-frontend/src/rpcErrors.ts,
107-
// which pins the first string. The distinction matters: a flagless rejection matching one of
108-
// these proves the call never reached the DO, so re-issuing it on a fresh stub cannot
109-
// double-execute — safe even for writes.
105+
// The distinction matters: a flagless rejection matching one of these proves the call never
106+
// reached the DO, so re-issuing it on a fresh stub cannot double-execute — safe even for writes.
110107
static readonly #DEAD_CAPABILITY_MESSAGES = [
111-
// What production resets leave as the brokenness reason.
112-
"The execution context which hosts this callback is no longer running",
108+
// What production resets leave as the brokenness reason; the frontend canary test pins it.
109+
WORKERD_DEAD_CAPABILITY_MESSAGE,
113110
// What vitest-pool-workers' abortAllDurableObjects() leaves — never occurs in production;
114111
// listed so integration tests exercise the real never-sent retry path.
115112
"Application called abortAllDurableObjects().",
@@ -120,9 +117,8 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi {
120117
// (see #DEAD_CAPABILITY_MESSAGES), re-issues it once on the fresh stub. Flagged errors are
121118
// rethrown untouched — the call may have executed, and the frontend owns that recovery.
122119
#wrapUserStub(stub: DurableObjectStub<UserDurableObject>): DurableObjectStub<UserDurableObject> {
123-
const wrapped: DurableObjectStub<UserDurableObject> = new Proxy(stub, {
120+
return new Proxy(stub, {
124121
get: (target, prop) => {
125-
if (prop === RAW_USER_STUB) return target;
126122
const value = Reflect.get(target, prop);
127123
if (typeof value !== "function") return value;
128124
return (...args: unknown[]) => {
@@ -139,22 +135,21 @@ class AuthenticatedApiImpl extends RpcTarget implements AuthenticatedApi {
139135
if (flagged || neverSent) {
140136
// Guard against thrashing: concurrent failures on the same dead stub must not
141137
// each discard the replacement the first one already resolved.
142-
if (this.userStub === wrapped) this.userStub = undefined;
138+
if (this.rawUserStub === target) this.userStub = this.rawUserStub = undefined;
143139
if (neverSent) {
144-
// Retry exactly once, against the raw target of the re-armed stubgoing
145-
// through the proxy again would retry unboundedly under repeated resets.
146-
const raw = (this.user as unknown as Record<symbol, unknown>)[RAW_USER_STUB] as
147-
DurableObjectStub<UserDurableObject>;
148-
return Reflect.apply(Reflect.get(raw, prop) as (...a: unknown[]) => unknown,
149-
raw, args);
140+
// Retry exactly once, on the raw target of the re-armed cacheretrying through
141+
// the proxy would retry unboundedly under repeated resets.
142+
void this.user;
143+
const fresh = this.rawUserStub!;
144+
return Reflect.apply(Reflect.get(fresh, prop) as (...a: unknown[]) => unknown,
145+
fresh, args);
150146
}
151147
}
152148
throw err;
153149
});
154150
};
155151
},
156152
});
157-
return wrapped;
158153
}
159154

160155
#isAdmin(): boolean {

packages/workshop-frontend/src/BlueprintLandingPage.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logRpcFailure, withDoResetRetry } from './rpcErrors'
1+
import { logRpcFailure } from './rpcErrors'
22
import { useState, useEffect, useCallback, useMemo, useRef, type ReactNode } from 'react'
33
import { useNavigate, useParams, useRouter } from '@tanstack/react-router'
44
import { RpcStub, RpcTarget } from 'capnweb'
@@ -121,7 +121,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) {
121121
// When authenticated, fetch models for binding assignment.
122122
useEffect(() => {
123123
if (isAuthenticated && authenticatedApi) {
124-
withDoResetRetry(() => authenticatedApi.listModels())
124+
authenticatedApi.listModels()
125125
.then(setModels)
126126
.catch(err => logRpcFailure('Failed to load models:', err))
127127
} else {
@@ -184,7 +184,7 @@ export default function BlueprintLandingPage({ rpcStub }: Props) {
184184
ready() {}
185185
}
186186

187-
withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber()))
187+
authenticatedApi.subscribeConnectedAccounts(new AccountsSubscriber())
188188
.then(stub => {
189189
if (cancelled) {
190190
stub[Symbol.dispose]()

packages/workshop-frontend/src/ChatInterface.tsx

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { isDurableObjectResetError, isTransientRpcError, logRpcFailure, reportDoResetError } from "./rpcErrors";
1+
import { isTransientRpcError, logRpcFailure } from "./rpcErrors";
22
import {
33
Fragment,
44
memo,
@@ -1842,11 +1842,10 @@ export const ChatInput = ({
18421842
const [capsules, setCapsules] = useState<InputCapsule[]>([]);
18431843
const [pendingAttachments, setPendingAttachments] = useState<PendingAttachment[]>([]);
18441844
const [isSending, setIsSending] = useState(false);
1845-
const [sendHiccup, setSendHiccup] = useState(false);
1846-
const chatKeyRef = useRef(chatKey);
1847-
chatKeyRef.current = chatKey;
1848-
1849-
useEffect(() => setSendHiccup(false), [chatKey]);
1845+
// The chat the "may not have been sent" hint belongs to; the render condition scopes it, and
1846+
// leaving the chat dismisses it.
1847+
const [sendHiccup, setSendHiccup] = useState<{ chatKey?: number | null } | null>(null);
1848+
useEffect(() => setSendHiccup(null), [chatKey]);
18501849
const [isAttachmentDragActive, setIsAttachmentDragActive] = useState(false);
18511850
const [selectedSlashCommand, setSelectedSlashCommand] = useState<SelectedSlashCommand | null>(null);
18521851
// The caret the slash command picker parses at. Deliberately updated only when it moves to a
@@ -2285,7 +2284,7 @@ export const ChatInput = ({
22852284

22862285
const handleSend = async () => {
22872286
if (sendInFlightRef.current || isSending || isBlocked) return;
2288-
setSendHiccup(false);
2287+
setSendHiccup(null);
22892288
const attachmentsSnapshot = pendingAttachments;
22902289
const readyAttachments = attachmentsSnapshot
22912290
.filter((attachment) => attachment.uploadState === "ready" && attachment.ref)
@@ -2467,9 +2466,7 @@ export const ChatInput = ({
24672466
const submittedChatKey = chatKey;
24682467
void handleSend().catch((err) => {
24692468
// The onSend handlers already log; the composer only needs the hint state.
2470-
if (isTransientRpcError(err) && chatKeyRef.current === submittedChatKey) {
2471-
setSendHiccup(true);
2472-
}
2469+
if (isTransientRpcError(err)) setSendHiccup({ chatKey: submittedChatKey });
24732470
});
24742471
};
24752472

@@ -3064,7 +3061,7 @@ export const ChatInput = ({
30643061
</div>
30653062
)}
30663063
{draftUpdateBanner}
3067-
{sendHiccup && (
3064+
{sendHiccup && sendHiccup.chatKey === chatKey && (
30683065
<div className="px-4 pt-2 text-xs text-kumo-warning">
30693066
Connection hiccup — your message may not have been sent. Check the thread, then try again.
30703067
</div>
@@ -5386,8 +5383,7 @@ function ChatInterface({
53865383
);
53875384
}
53885385
} catch (err) {
5389-
if (isDurableObjectResetError(err)) reportDoResetError("chat.send", err);
5390-
if (!logRpcFailure("Failed to send message:", err)) {
5386+
if (!logRpcFailure("Failed to send message:", err, { reportSite: "chat.send" })) {
53915387
toasts.add({ title: "Failed to send message", variant: "error" });
53925388
}
53935389
throw err;
@@ -5410,8 +5406,7 @@ function ChatInterface({
54105406
message, model, capsules, attachments, formats);
54115407
onNavigateToChatRef.current(newChatId);
54125408
} catch (err) {
5413-
if (isDurableObjectResetError(err)) reportDoResetError("chat.new", err);
5414-
if (!logRpcFailure("Failed to create new chat:", err)) {
5409+
if (!logRpcFailure("Failed to create new chat:", err, { reportSite: "chat.new" })) {
54155410
toasts.add({ title: "Failed to start conversation", variant: "error" });
54165411
}
54175412
throw err;

packages/workshop-frontend/src/ConnectAccountModal.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { RpcStub } from 'capnweb'
44
import { AuthenticatedApi, GatekeeperVendorFilter } from '@gadgets/workshop-shared/api'
55
import { VendorDescription } from '@gadgets/workshop-shared/gatekeeper'
66
import VendorCard from './VendorCard'
7-
import { withDoResetRetry } from './rpcErrors'
87

98
interface ConnectAccountModalProps {
109
visible: boolean
@@ -42,7 +41,7 @@ export default function ConnectAccountModal({
4241
const fetchVendors = async () => {
4342
setVendorsLoading(true)
4443
try {
45-
const vendorList = await withDoResetRetry(() => authenticatedApi.listGatekeeperVendors(filter))
44+
const vendorList = await authenticatedApi.listGatekeeperVendors(filter)
4645
const unavailable = vendorList.filter(v => v.unavailable)
4746
if (unavailable.length > 0) {
4847
toasts.add({

packages/workshop-frontend/src/GatekeeperModal.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logRpcFailure, withDoResetRetry } from './rpcErrors'
1+
import { logRpcFailure } from './rpcErrors'
22
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
33
import { Dialog, useKumoToastManager, type PortalContainer } from '@cloudflare/kumo'
44
import {
@@ -369,7 +369,7 @@ export default function GatekeeperModal({
369369
setSpawnerEnv(
370370
(spawnerEnvCandidatesRef.current ?? []).map(entry => ({ ...entry, enabled: true })))
371371

372-
withDoResetRetry(() => authenticatedApi.listModels()).then(models => {
372+
authenticatedApi.listModels().then(models => {
373373
if (cancelled) return
374374
setAvailableModels(models)
375375
if (models.length > 0) {
@@ -388,7 +388,7 @@ export default function GatekeeperModal({
388388
toasts.add({ title: "Couldn't load AI models", variant: 'error' })
389389
})
390390

391-
withDoResetRetry(() => authenticatedApi.listGatekeeperVendors()).then(vendors => {
391+
authenticatedApi.listGatekeeperVendors().then(vendors => {
392392
if (cancelled) return
393393
setVendors(vendors)
394394
}).catch(err => {
@@ -432,7 +432,7 @@ export default function GatekeeperModal({
432432
}
433433

434434
const subscriber = new AccountsSubscriber()
435-
withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber))
435+
authenticatedApi.subscribeConnectedAccounts(subscriber)
436436
.then(stub => {
437437
if (cancelled) {
438438
stub[Symbol.dispose]()

packages/workshop-frontend/src/ObserverConfigModal.tsx

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { withDoResetRetry } from './rpcErrors'
21
import { useState, useEffect, useRef } from 'react'
32
import { Dialog, Select, Loader, Text, useKumoToastManager } from '@cloudflare/kumo'
43
import { Warning, Plus, ArrowClockwise, CheckCircle } from '@phosphor-icons/react'
@@ -144,8 +143,8 @@ export default function ObserverConfigModal({
144143
}
145144
}
146145

147-
withDoResetRetry(() => authenticatedApi
148-
.subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true }))
146+
authenticatedApi
147+
.subscribeConnectedAccounts(new Subscriber(), { includeForcedAutoProvisionedAccounts: true })
149148
.then(stub => {
150149
if (cancelled) { stub[Symbol.dispose](); return }
151150
subStub = stub
@@ -166,10 +165,10 @@ export default function ObserverConfigModal({
166165
// ── load vendor metadata for display and resource-scope resolution ─────────────
167166
useEffect(() => {
168167
let cancelled = false
169-
withDoResetRetry(() => Promise.all([
168+
Promise.all([
170169
authenticatedApi.listGatekeeperVendors(),
171170
authenticatedApi.listAddableGatekeepers(),
172-
]))
171+
])
173172
.then(([vendors, addable]) => {
174173
if (cancelled) return
175174
const map = new Map<string, GatekeeperVendorInfo>()

packages/workshop-frontend/src/OnboardingWizard.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logRpcFailure, withDoResetRetry } from './rpcErrors'
1+
import { logRpcFailure } from './rpcErrors'
22
import { useState, useEffect, useRef, useCallback } from 'react'
33
import { useKumoToastManager } from '@cloudflare/kumo'
44
import { RpcTarget } from 'capnweb'
@@ -120,10 +120,10 @@ export default function OnboardingWizard({
120120
// Load models + AI config
121121
const fetchModels = useCallback(async () => {
122122
try {
123-
const [modelList, cfg] = await withDoResetRetry(() => Promise.all([
123+
const [modelList, cfg] = await Promise.all([
124124
authenticatedApi.listModels(),
125125
authenticatedApi.getAiConfig(),
126-
]))
126+
])
127127
setModels(modelList)
128128
setAiConfig(cfg)
129129
// Default to the first model in the list
@@ -221,7 +221,7 @@ export default function OnboardingWizard({
221221
const subscriber = new AccountsSubscriber()
222222
let subscriptionStub: { [Symbol.dispose](): void } | null = null
223223

224-
withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber))
224+
authenticatedApi.subscribeConnectedAccounts(subscriber)
225225
.then((stub) => {
226226
if (cancelled) {
227227
stub[Symbol.dispose]()

packages/workshop-frontend/src/ResourcePicker.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logRpcFailure, withDoResetRetry } from './rpcErrors'
1+
import { logRpcFailure } from './rpcErrors'
22
import { useState, useEffect, useRef, useMemo, useCallback, type MutableRefObject } from 'react'
33
import { Tooltip, useKumoToastManager } from '@cloudflare/kumo'
44
import { Plus, CaretRight, Warning } from '@phosphor-icons/react'
@@ -159,8 +159,7 @@ export default function ResourcePicker({
159159
const subscriber = new AccountsSubscriber()
160160
const subscribe = async () => {
161161
try {
162-
const stub = await withDoResetRetry(
163-
() => authenticatedApi.subscribeConnectedAccounts(subscriber))
162+
const stub = await authenticatedApi.subscribeConnectedAccounts(subscriber)
164163
if (cancelled) stub[Symbol.dispose]()
165164
else subscriptionRef.current = { stub }
166165
} catch (error) {

packages/workshop-frontend/src/components/AppShell/SidebarWorkspaces.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { logRpcFailure, withDoResetRetry } from '../../rpcErrors'
1+
import { logRpcFailure } from '../../rpcErrors'
22
import {
33
createContext,
44
useCallback,
@@ -90,7 +90,7 @@ export function SidebarWorkspacesProvider({ children }: { children: ReactNode })
9090
useEffect(() => {
9191
let cancelled = false
9292
setGadgetsLoading(true)
93-
withDoResetRetry(() => authenticatedApi.listGadgets())
93+
authenticatedApi.listGadgets()
9494
.then((list) => {
9595
if (cancelled) return
9696
setGadgets(list)

packages/workshop-frontend/src/components/ConnectionChips.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { withDoResetRetry } from '../rpcErrors'
21
import { Plus } from '@phosphor-icons/react'
32
import { Link } from '@tanstack/react-router'
43
import { useAuthenticatedApi } from '../AuthContext'
@@ -46,7 +45,7 @@ export default function ConnectionChips() {
4645
}
4746

4847
const subscriber = new ChipsSubscriber()
49-
const subPromise = withDoResetRetry(() => authenticatedApi.subscribeConnectedAccounts(subscriber))
48+
const subPromise = authenticatedApi.subscribeConnectedAccounts(subscriber)
5049
subPromise.then((stub) => {
5150
if (cancelled) {
5251
stub[Symbol.dispose]()

0 commit comments

Comments
 (0)