Merge pull request #74 from devframes/spicy-experts-wear

Author: antfu

packages/devframe/src/rpc/handler.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from 'vitest'
+import { defineRpcFunction } from './define'
+import { getRpcResolvedSetupResult } from './handler'
+
+describe('getRpcResolvedSetupResult', () => {
+  it('retries setup after a rejection instead of caching it (object context)', async () => {
+    let calls = 0
+    const def = defineRpcFunction({
+      name: 't:retry',
+      type: 'query',
+      setup: () => {
+        calls++
+        if (calls === 1)
+          throw new Error('transient')
+        return { handler: () => 'ok' }
+      },
+    })
+    const ctx = {}
+    await expect(getRpcResolvedSetupResult(def as any, ctx)).rejects.toThrow('transient')
+    const result = await getRpcResolvedSetupResult(def as any, ctx)
+    expect(result.handler?.()).toBe('ok')
+    expect(calls).toBe(2)
+  })
+
+  it('caches a successful setup result per context (no re-run)', async () => {
+    let calls = 0
+    const def = defineRpcFunction({
+      name: 't:cache',
+      type: 'query',
+      setup: () => {
+        calls++
+        return { handler: () => calls }
+      },
+    })
+    const ctx = {}
+    await getRpcResolvedSetupResult(def as any, ctx)
+    await getRpcResolvedSetupResult(def as any, ctx)
+    expect(calls).toBe(1)
+  })
+})

packages/devframe/src/rpc/handler.ts

@@ -21,16 +21,30 @@ export async function getRpcResolvedSetupResult<
   // closed over a prior context's state.
   if (typeof context === 'object' && context !== null) {
     definition.__cache ??= new WeakMap()
-    let promise = definition.__cache.get(context as object)
+    const cache = definition.__cache
+    let promise = cache.get(context as object)
     if (!promise) {
       promise = Promise.resolve(definition.setup(context))
-      definition.__cache.set(context as object, promise)
+      // If setup rejects, evict so a later call can retry instead of
+      // re-awaiting the cached rejection permanently.
+      promise.catch(() => {
+        if (cache.get(context as object) === promise)
+          cache.delete(context as object)
+      })
+      cache.set(context as object, promise)
     }
     return await promise
   }
 
   // Primitive / undefined context — fall back to a single-slot cache.
-  definition.__promise ??= Promise.resolve(definition.setup(context))
+  if (!definition.__promise) {
+    const promise = Promise.resolve(definition.setup(context))
+    promise.catch(() => {
+      if (definition.__promise === promise)
+        definition.__promise = undefined
+    })
+    definition.__promise = promise
+  }
   return await definition.__promise
 }
 

plans/005-rpc-setup-cache-rejection.md

@@ -17,7 +17,7 @@ changes are allowed as long as they're marked).
 | 002 | Clear the critical `shell-quote` advisory in the bundle | security/deps | P1 | S | — | DONE |
 | 003 | Reject cross-origin WebSocket upgrades ⚠️ | security | P1 | S | — | DONE |
 | 004 | Stop git argument injection via `ref`/`hash` | security/bug | P1 | S | — | TODO |
-| 005 | Don't cache a rejected RPC `setup()` promise | bug | P1 | S | — | TODO |
+| 005 | Don't cache a rejected RPC `setup()` promise | bug | P1 | S | — | DONE |
 | 006 | Bound `SharedState.syncIds` (memory leak) | bug | P1 | S | — | DONE |
 | 007 | Behavioral tests for the auth/OTP trust boundary | tests | P1 | S | — | DONE |
 | 008 | Cap hub terminal buffer + reject restart-after-terminate | bug | P2 | S | — | TODO |