feat(durable-iterator): rewrite + enchance#965
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces the durable-event-iterator package with a reorganized durable-iterator implementation: removes the old package and tests; adds new durable-iterator package (client, link/plugin, durable-object handler/object, resume storage, websocket adapters, schemas, contract, consts, errors); updates docs, playgrounds, shared utilities, signing helper, and CI configs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant ORPCClient as ORPC Client
participant DIPlugin as DurableIteratorLinkPlugin
participant WS as ReconnectableWebSocket
participant DO as DurableIteratorObject
participant Handler as Durable Iterator Handler
Client->>ORPCClient: call that yields durable-iterator output
ORPCClient->>DIPlugin: interceptor detects durable-iterator response
DIPlugin->>DIPlugin: getToken() / parse token → create id
DIPlugin->>WS: open ws://.../?id=...&token=...
WS->>DO: upgrade forwarded (upgradeDurableIteratorRequest)
DO->>Handler: subscribe() → returns AsyncIterator of events
Handler-->>WS: stream events (hibernation-encoded)
DIPlugin->>ORPCClient: build durable client link
ORPCClient-->>Client: return proxied AsyncIterator (overlayProxy)
sequenceDiagram
autonumber
participant Procedure as ProcedureClient
participant Handler as RPC Handler
participant It as AsyncIterator
participant Shared as overlayProxy
Procedure->>Handler: call -> handler returns AsyncIterator
Handler-->>Procedure: AsyncIterator
Procedure->>Shared: overlayProxy(AsyncIterator, AsyncIteratorClass)
Shared-->>Procedure: proxied iterator
Procedure-->>Caller: proxied iterator returned
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
dinwwwh
force-pushed
the
feat/durable-event-iterator/rewrite-and-improve
branch
from
044b6e8 to
58d94af
Compare
dinwwwh
force-pushed
the
feat/durable-event-iterator/rewrite-and-improve
branch
from
e8e68d1 to
cdd93bb
Compare
…ields (rpc methods in this case)
dosubot
Bot
added
the
size:XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (66)
packages/server/src/adapters/websocket/rpc-handler.ts (1)
7-7: Consider extending interface capabilities for consistency.The
RPCHandlerOptions<T>interface currently only extendsStandardRPCHandlerOptions<T>without adding any WebSocket-specific options. Looking at the fetch and node adapters, they both add astrictGetMethodPluginEnabledoption. While WebSocket may not need this specific option, consider whether any WebSocket-specific configuration should be exposed here for future extensibility or consistency with the adapter pattern.packages/contract/src/plugins/response-validation.ts (1)
23-27: Clarify the comment (current wording is confusing).Suggest rewording for precision and grammar.
/** - * run before (validate after) retry plugin, because validation failed can't be retried - * run before (validate after) durable iterator plugin, because we expect durable iterator to validation (if user use it) + * Initialize before Retry and Durable Iterator plugins so that—with last-added interceptors being outermost— + * response validation runs after them: + * - Retry: validation errors are not retriable. + * - Durable Iterator: validate the final transformed payload (if enabled). */packages/durable-iterator/tests/shared.ts (3)
22-25: Track accepted sockets so getWebSockets() returns real connectionsRight now getWebSockets() always returns [], which can hide bugs in code that iterates/acts on active sockets. Record accepted sockets and return that list.
Apply this diff within the object:
- waitUntil: vi.fn(), - acceptWebSocket: vi.fn(), - getWebSockets: vi.fn(() => []), + waitUntil: vi.fn(), + acceptWebSocket: vi.fn((ws) => { sockets.push(ws) }), + getWebSockets: vi.fn(() => sockets),Add the sockets store inside createDurableObjectState (outside the selected lines):
// right after `const db = new Database(':memory:')` const sockets: any[] = []
31-37: Let close() transition readyState to CLOSED (3)This makes the mock behave closer to WebSocket semantics and catches misuse after closure.
- readyState: 1, - send: vi.fn(), - close: vi.fn(), + readyState: 1, + send: vi.fn(), + close: vi.fn(function (this: any) { this.readyState = 3 }),
10-11: Make SQL method detection case‑insensitiveSafer for lowercase/mixed‑case queries in tests.
- const method = query.includes('SELECT') || query.includes('RETURNING') ? 'all' : 'run' - const result = db.prepare(query)[method](...bindings) + const q = query.toUpperCase() + const method = q.includes('SELECT') || q.includes('RETURNING') ? 'all' : 'run' + const result = db.prepare(query)[method](...bindings)packages/durable-iterator/src/client/iterator.test-d.ts (1)
7-9: Avoid DOM type dependency in type test (WebSocket).Using the
WebSockettype forceslib: ["dom"]or a custom type. Preferunknown(orany) here since the arg type is irrelevant to the assertion.- sendMessage: (ws: WebSocket) => Client<object, string, string, 'any-error'> + sendMessage: (ws: unknown) => Client<object, string, string, 'any-error'>packages/shared/src/proxy.ts (1)
53-83: Docstring binding semantics are misleading; bind target varies (overlay or target).The code binds functions to whichever object provides the property, not always to the overlay. Update the comment to avoid confusion.
Apply this diff to clarify:
/** * Create a proxy that overlays one object (`overlay`) on top of another (`target`). * * - Properties from `overlay` take precedence. * - Properties not in `overlay` fall back to `target`. - * - Methods from either object are bound to `overlay` so `this` is consistent. + * - Methods are bound to the object they originate from (overlay or target) so `this` remains correct. * * Useful when you want to override or extend behavior without fully copying/merging objects. */packages/client/src/plugins/retry.test.ts (4)
77-87: Reduce flakiness in timing assertions.CI timing variance can exceed 249ms. Loosen the upper bound.
Apply this diff:
- expect(Date.now() - start).toBeLessThanOrEqual(249) + expect(Date.now() - start).toBeLessThanOrEqual(400)
196-208: OK to assert overlay proxy usage (non‑retry path).Count=1 is reasonable with current internals; if internals evolve, consider asserting ≥1 instead of exact.
210-223: Overlay proxy call count could be brittle across refactors.Prefer asserting at least one call and argument shapes, not exact totals.
Apply this diff:
- expect(overlayProxySpy).toHaveBeenCalledTimes(5) // handler 4, plugin 1 + expect(overlayProxySpy.mock.calls.length).toBeGreaterThanOrEqual(5) // handler 4, plugin 1 (min)
368-382: Reduce flakiness in iterator retry timing assertions.Same rationale as above.
Apply this diff:
- expect(Date.now() - start).toBeLessThanOrEqual(249) + expect(Date.now() - start).toBeLessThanOrEqual(400)packages/client/src/plugins/retry.ts (3)
153-196: Prevent resource leaks when switching iterators on retryWhen a retry yields a new iterator, the previous iterator isn’t closed. Proactively calling
return()on the old iterator avoids dangling sockets/streams.Apply this diff:
catch (error) { const meta = getEventMeta(error) lastEventId = meta?.id ?? lastEventId lastEventRetry = meta?.retry ?? lastEventRetry - const maybeEventIterator = await next({ error }) + const maybeEventIterator = await next({ error }) if (!isAsyncIteratorObject(maybeEventIterator)) { throw new ClientRetryPluginInvalidEventIteratorRetryResponse( 'RetryPlugin: Expected an Event Iterator, got a non-Event Iterator', ) } - current = maybeEventIterator + // Close previous iterator to avoid leaking resources. + const prev = current + current = maybeEventIterator + try { + await prev.return?.() + } + catch { + // noop — best-effort cleanup + } /** * If iterator is aborted while retrying, we should cleanup right away */ if (isIteratorAborted) { await current.return?.() throw error } }
117-124: Make retry delay abortableCurrently the wait before retry isn’t canceled by
AbortSignal, causing unnecessary stalls after abort. Make the delay abortable.Apply this diff:
- const retryDelayMs = await value(retryDelay, attemptOptions) - - await new Promise(resolve => setTimeout(resolve, retryDelayMs)) + const retryDelayMs = await value(retryDelay, attemptOptions) + await new Promise<void>((resolve, reject) => { + const timer = setTimeout(resolve, retryDelayMs) + const sig = attemptOptions.signal + if (sig) { + const onAbort = () => { + clearTimeout(timer) + reject((sig as any).reason ?? new Error('Aborted')) + } + if (sig.aborted) onAbort() + else sig.addEventListener('abort', onAbort, { once: true }) + } + })
59-60: Makeorderreadonly and confirm plugin sequencingMark the plugin's
orderas readonly to prevent accidental mutation. 1_800_000 places Retry after durable-iterator (1_500_000) and before dedupe-requests (4_000_000) and batch (5_000_000).Location: packages/client/src/plugins/retry.ts (around lines 59–60)
Apply this minimal refactor:
- order = 1_800_000 + readonly order = 1_800_000 as constpackages/durable-iterator/src/error.ts (1)
1-2: Set error name and pass-through options for better stack/cause metadata.Add an explicit constructor to set
nameand forwardErrorOptions(cause) reliably.-export class DurableIteratorError extends Error { -} +export class DurableIteratorError extends Error { + constructor(message?: string, options?: ErrorOptions) { + super(message, options) + this.name = 'DurableIteratorError' + } +}packages/durable-iterator/src/schemas.test.ts (1)
118-131: Fix potential flakiness: exp === now depends on wall-clock.The implementation checks
exp < now(strict). Usingexp === nowcan intermittently pass. Make it unambiguously expired.- const currentTime = Math.floor(Date.now() / 1000) + const currentTime = Math.floor(Date.now() / 1000) const payload: DurableIteratorTokenPayload = { chn: 'test-channel', iat: currentTime, - exp: currentTime, // expires exactly now + exp: currentTime - 1, // ensure expired }packages/durable-iterator/src/object.ts (1)
12-19: Type utility looks good. Consider a brief doc comment.Optional: add a short JSDoc explaining the intent (pick RPC-capable members; exclude AsyncIterator methods).
packages/durable-iterator/src/client/iterator.ts (2)
49-65: Avoid parsing the token for non-RPC property access.Parse only when needed. This prevents throws on e.g.
Symbol.toStringTagreads and reduces overhead.const proxy = new Proxy(iterator, { get(target, prop) { - const token = options.getToken() - const { rpc: allowMethods } = parseDurableIteratorToken(token) - - if (prop === CLIENT_DURABLE_ITERATOR_TOKEN_SYMBOL) { - return token - } - - if (typeof prop === 'string' && allowMethods?.includes(prop)) { - return createORPCClient(link, { path: [prop] }) - } + if (prop === CLIENT_DURABLE_ITERATOR_TOKEN_SYMBOL) { + return options.getToken() + } + if (typeof prop === 'string') { + const { rpc: allowMethods } = parseDurableIteratorToken(options.getToken()) + if (allowMethods?.includes(prop)) { + return createORPCClient(link, { path: [prop] }) + } + } const v = Reflect.get(target, prop) return typeof v === 'function' ? v.bind(target) // Require .bind itself for calling : v }, })
72-74: Nit: update docstring to match rename (drop “Event”).- * If return a token if the client is a Client Durable Event Iterator. + * Return a token if the client is a Client Durable Iterator.packages/durable-iterator/src/client/plugin.ts (4)
86-92: Fix context merge order to avoid clobbering plugin context
options.contextspread currently overwrites the plugin context symbol if present. Place the plugin context last.- const next = () => options.next({ - ...options, - context: { - [this.CONTEXT_SYMBOL]: pluginContext, - ...options.context, - }, - }) + const next = () => options.next({ + ...options, + context: { + ...options.context, + [this.CONTEXT_SYMBOL]: pluginContext, + }, + })
56-58: Update docs link to new iterator namingThe JSDoc still references “durable-event-iterator”.
- * @see {@link https://orpc.unnoq.com/docs/integrations/durable-event-iterator Durable Event Iterator Integration} + * @see {@link https://orpc.unnoq.com/docs/integrations/durable-iterator Durable Iterator Integration}
136-180: Clamp negative refresh delay to 0 to avoid scheduling anomaliesIf
exp - now - beforeSecondsis negative, the timer should fire immediately but explicitly clamping is clearer and safer across runtimes.- const nowInSeconds = Math.floor(Date.now() / 1000) - - refreshTokenBeforeExpireTimeoutId = setTimeout(async () => { + const nowInSeconds = Math.floor(Date.now() / 1000) + const delayMs = Math.max(0, (tokenAndPayload.payload.exp - nowInSeconds - beforeSeconds) * 1000) + + refreshTokenBeforeExpireTimeoutId = setTimeout(async () => { // retry until success or finished const newTokenAndPayload = await retry({ times: Number.POSITIVE_INFINITY, delay: 2000 }, async (exit) => { try { const output = await next() return this.validateToken(output, options.path) } catch (err) { if (isFinished) { exit(err) } throw err } }) @@ - }, (tokenAndPayload.payload.exp - nowInSeconds - beforeSeconds) * 1000) + }, delayMs)
154-157: Make token “tags” comparison order-agnosticJSON string equality is sensitive to array order; tags are typically sets.
- const canProactivelyUpdateToken - = newTokenAndPayload.payload.chn === tokenAndPayload.payload.chn - && stringifyJSON(newTokenAndPayload.payload.tags) === stringifyJSON(tokenAndPayload.payload.tags) + const canProactivelyUpdateToken = + newTokenAndPayload.payload.chn === tokenAndPayload.payload.chn && + (() => { + const a = new Set(newTokenAndPayload.payload.tags ?? []) + const b = new Set(tokenAndPayload.payload.tags ?? []) + if (a.size !== b.size) return false + for (const t of a) if (!b.has(t)) return false + return true + })()packages/durable-iterator/src/durable-object/resume-storage.ts (2)
52-55: Validate/sanitizeresumeTablePrefixto prevent SQL identifier injection
resumeTablePrefixis interpolated into SQL identifiers. Restrict to a safe charset to avoid accidental injection via quotes.- this.retentionSeconds = fallback(options.resumeRetentionSeconds, Number.NaN) // disabled by default - this.schemaPrefix = fallback(options.resumeTablePrefix, 'orpc:durable-iterator:resume:') + this.retentionSeconds = fallback(options.resumeRetentionSeconds, Number.NaN) // disabled by default + const rawPrefix = fallback(options.resumeTablePrefix, 'orpc:durable-iterator:resume:') + if (!/^[\w:-]+$/.test(rawPrefix)) { + throw new Error('Invalid resumeTablePrefix: only letters, digits, underscore, dash, and colon are allowed') + } + this.schemaPrefix = rawPrefix this.serializer = new StandardRPCJsonSerializer(options)
146-151: Tag filter short‑circuit is correct; consider documenting semanticsEvents with tags only deliver to websockets sharing at least one tag; tagless websockets won’t receive tagged events.
Add a brief comment to clarify intended behavior for future maintainers.
packages/durable-iterator/src/durable-object/resume-storage.test.ts (2)
7-7: Rename suite for clarityUse the class name to make test output clearer.
-describe('eventStreamStorage', () => { +describe('EventResumeStorage', () => {
24-26: Avoidvoid new …in testsInstantiation side‑effects can be expressed without
void, which reads oddly in tests.- void new EventResumeStorage(ctx, { resumeRetentionSeconds: 1 }) + new EventResumeStorage(ctx, { resumeRetentionSeconds: 1 })packages/durable-iterator/src/index.test.ts (1)
1-3: Tighten test: assert DurableIteratorHandlerPlugin is exported from indexpackages/durable-iterator/src/index.test.ts — index.ts re-exports ./plugin; make the test explicit:
-it('export something', async () => { - expect(Object.keys(await import('./index'))).toContain('DurableIteratorHandlerPlugin') -}) +it('exports DurableIteratorHandlerPlugin from index', async () => { + const api = await import('./index') + expect('DurableIteratorHandlerPlugin' in api).toBe(true) +})packages/durable-iterator/src/client/index.test.ts (1)
1-3: Clarify test name and assert export directlyUse a precise name and assert via property presence (also type-check) for a more robust surface test.
Apply this diff:
-it('export something', async () => { - expect(Object.keys(await import('./index'))).toContain('DurableIteratorLinkPlugin') -}) +it('should export DurableIteratorLinkPlugin from client index', async () => { + const mod = await import('./index') + expect(mod).toHaveProperty('DurableIteratorLinkPlugin') + expect(typeof mod.DurableIteratorLinkPlugin).toBe('function') +})playgrounds/cloudflare-worker/src/components/chat-room.tsx (1)
31-34: Prefer a single approach and handle abort errors
- Keep one send path to reduce noise.
- Consider catching AbortError in the effect to avoid unhandled rejections on unmount.
Apply this diff to clean up the send path:
- // await client.message.send({ message }) - // or --- - await iterator?.publishMessageRPC({ message }) + await iterator?.publishMessageRPC({ message })Outside the selected range, wrap the effect body to handle aborts:
useEffect(() => { const controller = new AbortController() ;(async () => { try { const iterator = await client.message.on(undefined, { signal: controller.signal }) setIterator(iterator) for await (const message of iterator) { setMessages(messages => [...messages, message.message]) } } catch (err) { if ((err as any)?.name !== 'AbortError') { console.error(err) } } })() return () => controller.abort() }, [])apps/content/docs/helpers/signing.md (1)
26-27: Add an explicit security warning about getSignedValueClarify that getSignedValue must not be used for auth/security decisions.
Apply this diff:
// Extract value without verification const extractedValue = getSignedValue(signedValue) // 'user123' +// --- + +::: warning +getSignedValue does not verify authenticity. Do not use it for authentication, +authorization, or any security-critical decisions. Use `unsign` to verify. +:::playgrounds/cloudflare-worker/worker/index.ts (1)
86-89: Consider sourcing signingKey from env/secretsHardcoding 'key' is fine for a playground, but prefer env for clarity and future-proofing.
Example:
return upgradeDurableIteratorRequest(request, { signingKey: env.SIGNING_KEY, namespace: env.CHAT_ROOM, })packages/server/src/helpers/signing.ts (1)
91-117: Add a misuse warning in JSDocAdd a brief remark to prevent security misuse.
Apply this diff:
/** * Extracts the value part from a signed string without verification. * * This function simply extracts the original value from a signed string * without performing any signature verification. It's useful when you need * to access the value quickly without the overhead of cryptographic verification. + * + * @remarks Do not use this for authentication/authorization or any + * security-sensitive logic. Use `unsign` to verify integrity. * * @examplepackages/durable-iterator/src/contract.ts (2)
9-12: Use unknown over any for output type.Prefer AsyncIteratorClass for safer typing.
- .output(type<AsyncIteratorClass<any>>()), + .output(type<AsyncIteratorClass<unknown>>()),
15-19: Validate valibot tupleWithRest availability (or switch to tuple + rest).Confirm v.tupleWithRest exists in your valibot version; otherwise use tuple with rest.
If needed, replace with valibot’s tuple + rest equivalent (adjust per your valibot version):
- Example pattern: v.tuple([v.string()], v.rest(v.string()))
packages/durable-iterator/src/durable-object/websocket.test.ts (2)
6-8: Isolate state per test (re-create ws/proxy).Avoid cross-test leakage of attachments/mocks by instantiating per test.
beforeEach(() => { vi.clearAllMocks() + ws = createCloudflareWebsocket() + proxied = toDurableIteratorWebsocket(ws) as any }) describe('toDurableIteratorWebsocket', () => { - const ws = createCloudflareWebsocket() - const proxied = toDurableIteratorWebsocket(ws) as any + let ws: any + let proxied: anyAlso applies to: 11-13
54-69: Use fake timers; assert close code.Eliminates flakiness; also assert the 1008 close code.
- it('proxied and auto close if expired on send', async () => { - const nowInSeconds = Math.floor(Date.now() / 1000) + it('proxied and auto close if expired on send', async () => { + vi.useFakeTimers() + const now = new Date('2020-01-01T00:00:00.000Z') + vi.setSystemTime(now) + const nowInSeconds = Math.floor(now.getTime() / 1000) proxied['~orpc'].serializeTokenPayload({ id: 'some-id', exp: nowInSeconds + 1 }) proxied.send('data') expect(ws.send).toHaveBeenCalledTimes(1) expect(ws.close).toHaveBeenCalledTimes(0) vi.mocked(ws.send as () => any).mockClear() - await sleep(1001) + vi.advanceTimersByTime(1001) proxied.send('data') expect(ws.send).toHaveBeenCalledTimes(1) expect(ws.close).toHaveBeenCalledTimes(1) + expect(ws.close).toHaveBeenCalledWith(1008, 'Token expired') expect(ws.close).toHaveBeenCalledBefore(ws.send) + vi.useRealTimers() })After applying, remove the now-unused sleep import on Line 1.
packages/durable-iterator/src/iterator.test-d.ts (1)
24-24: Nit: rename test title (remove “event”).- it('resolve correct client durable event iterator type', async () => { + it('resolve correct client durable iterator type', async () => {packages/durable-iterator/src/iterator.test.ts (3)
8-8: Avoid secrets scanner false positives.Use an obvious dummy or env-provided value to prevent Gitleaks noise.
- const testSigningKey = 'test-signing-key-32-chars-long-123' + const testSigningKey = process.env.CI_TEST_SIGNING_KEY ?? 'unit-test-secret'If CI still flags, consider allowlisting this exact test file/key in gitleaks config.
24-48: Freeze time to stabilize iat/exp assertions.Prevents second-boundary flakiness.
- it('token & throw when interacting with client iterator', async () => { - const date = new Date() + it('token & throw when interacting with client iterator', async () => { + vi.useFakeTimers() + const now = new Date('2020-01-01T00:00:00.000Z') + vi.setSystemTime(now) const options = { tags: ['tag1', 'tag2'], att: { userId: 'user123' }, rpc: ['getUser', 'sendMessage'] as any, signingKey: testSigningKey, } const iterator = new DurableIterator(testChannel, options) as any const clientIterator = await iterator const token = getClientDurableIteratorToken(clientIterator) expect(token).toBeDefined() const payload = await verifyDurableIteratorToken(testSigningKey, token!) expect(payload?.chn).toBe(testChannel) expect(payload?.tags).toEqual(['tag1', 'tag2']) expect(payload?.att).toEqual({ userId: 'user123' }) expect(payload?.rpc).toEqual(['getUser', 'sendMessage']) - expect(payload?.iat).toEqual(Math.floor(date.getTime() / 1000)) - expect(payload?.exp).toEqual(Math.floor(date.getTime() / 1000) + 60 * 60 * 24) + expect(payload?.iat).toEqual(Math.floor(now.getTime() / 1000)) + expect(payload?.exp).toEqual(Math.floor(now.getTime() / 1000) + 60 * 60 * 24) await expect(clientIterator.next()).rejects.toThrow() await expect(clientIterator.getUser()).rejects.toThrow() + vi.useRealTimers() })
50-63: Freeze time for TTL test too.- it('can change token TTL', async () => { - const date = new Date() + it('can change token TTL', async () => { + vi.useFakeTimers() + const now = new Date('2020-01-01T00:00:00.000Z') + vi.setSystemTime(now) const options = { tokenTTLSeconds: 3600, // 1 hour signingKey: testSigningKey, } const iterator = new DurableIterator(testChannel, options) as any const clientIterator = await iterator const token = getClientDurableIteratorToken(clientIterator) const payload = await verifyDurableIteratorToken(testSigningKey, token!) - expect(payload?.exp).toEqual(Math.floor(date.getTime() / 1000) + 3600) + expect(payload?.exp).toEqual(Math.floor(now.getTime() / 1000) + 3600) + vi.useRealTimers() })packages/durable-iterator/src/durable-object/upgrade.test.ts (1)
97-136: Optional: test last-value-wins for duplicate query params.Since upgrade reads the last token/id via getAll(...).at(-1), add a test covering duplicates to lock behavior.
packages/durable-iterator/src/durable-object/object-state.test.ts (1)
8-10: Isolate proxied state per test.Create ctx/proxied in beforeEach to avoid shared mutable state between tests.
-describe('toDurableIteratorObjectState', () => { - const ctx = createDurableObjectState() - const proxied = toDurableIteratorObjectState(ctx) as any +describe('toDurableIteratorObjectState', () => { + let ctx: any + let proxied: any + beforeEach(() => { + ctx = createDurableObjectState() + proxied = toDurableIteratorObjectState(ctx) as any + })packages/durable-iterator/src/client/iterator.test.ts (2)
21-30: Create fresh iterator per test to avoid shared state.
iteratoris created once for the whole suite. Prefer per‑test setup to avoid cross‑test coupling (e.g., after a.return()).Example:
let iterator: ReturnType<typeof createClientDurableIterator> let next: jest.Mock, cleanup: jest.Mock, call: jest.Mock beforeEach(() => { next = vi.fn(() => Promise.resolve({ value: '__next__', done: false })) cleanup = vi.fn(() => Promise.resolve()) call = vi.fn(() => Promise.resolve('__call__')) iterator = createClientDurableIterator(new AsyncIteratorClass<any>(next, cleanup), { call }, { getToken }) })
55-56: Brittle expectation on mock call count.
Proxy.getcan be triggered by engine/TS helpers; counting exactgetTokencalls risks flakes. Assert minimum instead.Suggested change:
-expect(getToken).toHaveBeenCalledTimes(5) +expect(getToken).toHaveBeenCalledTimes(expect.toBeGreaterThanOrEqual(5))playgrounds/cloudflare-worker/worker/routers/message.ts (1)
7-12: Avoid hardcoding secrets in examples.Use env/config for
signingKeyrather than a literal to prevent accidental leaks in real use.Suggested change:
- return new DurableIterator<ChatRoom>('some-room', { - signingKey: 'key', + return new DurableIterator<ChatRoom>('some-room', { + signingKey: context.env.SIGNING_KEY,packages/durable-iterator/package.json (1)
4-4: Version0.0.0is unconventional for publication.Consider
0.0.1or0.1.0to avoid tooling edge cases.apps/content/docs/client/event-iterator.md (1)
38-43: Example calls both abort and return. Use one.Executing both can confuse readers and is unnecessary.
Apply this diff:
-// Stop the stream after 1 second +// Stop the stream after 1 second setTimeout(async () => { - controller.abort() - // or - await iterator.return() + controller.abort() + // or: + // await iterator.return() }, 1000)playgrounds/cloudflare-worker/worker/dos/chat-room.ts (2)
7-13:DurableObjectStategeneric parameter looks incorrect.
DurableObjectState<TProps>’s type parameter is for props, not env. UseDurableObjectState(or the correct props type).Apply this diff:
- constructor( - ctx: DurableObjectState<Cloudflare.Env>, - env: Env, - ) { + constructor( + ctx: DurableObjectState, + env: Env, + ) {
15-20: Use env/config instead of hardcoded signing key.Avoid literals for secrets in examples.
- signingKey: 'key', + signingKey: env.SIGNING_KEY,apps/content/docs/integrations/durable-iterator.md (1)
152-155: Update docs to new Durable Iterator naming and import paths.Several references still use DurableEventIterator* names or non‑experimental import paths.
Apply these diffs:
-import { withEventMeta } from '@orpc/durable-iterator' +import { withEventMeta } from '@orpc/experimental-durable-iterator/durable-object'-import { DurableEventIterator } from '@orpc/experimental-durable-iterator' +import { DurableIterator } from '@orpc/experimental-durable-iterator' ... - return new DurableEventIterator<ChatRoom>('some-room', { + return new DurableIterator<ChatRoom>('some-room', {-import { DurableEventIteratorHandlerPlugin } from '@orpc/experimental-durable-iterator' +import { DurableIteratorHandlerPlugin } from '@orpc/experimental-durable-iterator' ... - new DurableEventIteratorHandlerPlugin(), + new DurableIteratorHandlerPlugin(),-import { DurableEventIteratorLinkPlugin } from '@orpc/experimental-durable-iterator/client' +import { DurableIteratorLinkPlugin } from '@orpc/experimental-durable-iterator/client' ... - new DurableEventIteratorLinkPlugin({ + new DurableIteratorLinkPlugin({-`DurableEventIteratorLinkPlugin` establishes a WebSocket connection +`DurableIteratorLinkPlugin` establishes a WebSocket connection-import { DurableEventIterator } from '@orpc/experimental-durable-iterator' +import { DurableIterator } from '@orpc/experimental-durable-iterator' ... - return new DurableEventIterator<ChatRoom>('some-room', { + return new DurableIterator<ChatRoom>('some-room', {-import type { ClientDurableEventIterator } from '@orpc/experimental-durable-iterator/client' +import type { ClientDurableIterator } from '@orpc/experimental-durable-iterator/client' ... - onMessage: oc.output(type<ClientDurableEventIterator<ChatRoom, 'publishMessage'>>()), + onMessage: oc.output(type<ClientDurableIterator<ChatRoom, 'publishMessage'>>()),Additionally, the first Durable Object example uses
onErrorbut doesn't import it; add:import { onError } from '@orpc/server'Also applies to: 168-176, 189-199, 206-219, 221-223, 310-323, 371-383
packages/durable-iterator/src/schemas.ts (1)
35-37: Use second‑precision for expiry check to avoid early invalidation.Floor the current time and use
<=to align with common exp semantics.Apply this diff:
- if (payload.exp < (Date.now() / 1000)) { + const nowInSeconds = Math.floor(Date.now() / 1000) + if (payload.exp <= nowInSeconds) { return undefined }packages/durable-iterator/src/durable-object/websocket.ts (2)
60-62: Fix comment referencing outdated name.The comment still references "Durable Event Iterator" instead of "Durable Iterator", which is inconsistent with the rename throughout the codebase.
Apply this diff to fix the naming consistency:
/** - * Durable Event Iterator internal apis + * Durable Iterator internal apis */
65-65: Consider documenting the WebSocket cache behavior.The
WeakMapcache ensures reference equality preservation, which is critical for WebSocket adapter behavior. Consider adding a brief comment explaining why this cache is necessary.+/** + * Cache to ensure the same WebSocket always maps to the same DurableIteratorWebsocket instance. + * This preserves reference equality, which is required for proper WebSocket adapter behavior. + */ const websocketReferencesCache = new WeakMap<WebSocket, DurableIteratorWebsocket>()packages/durable-iterator/src/durable-object/object.ts (2)
42-43: Remove unnecessary return statement from void method.The
publishEventmethod has avoidreturn type but returns the result of the handler call.Apply this diff to fix the return type issue:
publishEvent(payload: T, options: PublishEventOptions = {}): void { - return this['~orpc'].publishEvent(payload, options) + this['~orpc'].publishEvent(payload, options) }
46-54: Consider clarifying the @info comment about non-upgrade requests.The comment mentions intercepting non-upgrade requests, but the method name
fetchand its delegation pattern don't make this immediately clear. Consider expanding the comment to explain how non-upgrade requests would be handled./** * Upgrades websocket connection * - * @info You can safety intercept non-upgrade requests + * @info You can safely intercept non-upgrade requests by overriding this method in a subclass + * and handling them before calling super.fetch() * @warning No verification is done here, you should verify the token payload before calling this method. */packages/durable-iterator/src/durable-object/upgrade.ts (1)
49-51: Verify id validation consistency.The code checks
typeof id !== 'string'but thegetAll().at(-1)pattern can returnundefined. Consider using a more explicit check for consistency with the token validation below.Apply this diff for consistency:
- if (typeof id !== 'string') { + if (!id) { return new Response('ID is required', { status: 401 }) }packages/durable-iterator/src/iterator.ts (2)
14-14: Fix typo in comment.There's an extra space in "The signing key".
/** - * The signing key used to sign the token + * The signing key used to sign the token */
66-101: Consider the implications of usingthenon a class.While the implementation creates a thenable object that integrates with async/await, using
thenon a class can be confusing as it makes the class instance behave like a Promise. This might lead to unexpected behavior in certain contexts.Consider these alternatives:
Option 1: Rename to a more explicit method
- then<TResult1 = ClientDurableIterator<T, RPC>, TResult2 = never>( + async build(): Promise<ClientDurableIterator<T, RPC>> { - onfulfilled?: ((value: ClientDurableIterator<T, RPC>) => TResult1 | PromiseLike<TResult1>) | null | undefined, - onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null | undefined, - ): PromiseLike<TResult1 | TResult2> { - return (async () => { const tokenTTLSeconds = this.options.tokenTTLSeconds ?? 60 * 60 * 24 // 24 hours const nowInSeconds = Math.floor(Date.now() / 1000) const token = await signDurableIteratorToken(this.options.signingKey, { chn: this.chn, tags: this.options.tags, att: this.options.att, rpc: this.options.rpc, iat: nowInSeconds, exp: nowInSeconds + tokenTTLSeconds, }) const iterator = new AsyncIteratorClass<any>( () => Promise.reject(new DurableIteratorError('Cannot be iterated directly.')), () => Promise.reject(new DurableIteratorError('Cannot be cleaned up directly.')), ) const link: ClientLink<object> = { call() { throw new DurableIteratorError('Cannot call methods directly.') }, } const durableIterator = createClientDurableIterator(iterator, link, { getToken: () => token, }) return durableIterator as ClientDurableIterator<T, RPC> - })().then(onfulfilled, onrejected) }Option 2: Keep the thenable pattern but add clear documentation
+ /** + * Implements PromiseLike to allow await syntax. + * This method signs a token and creates a ClientDurableIterator. + * + * @example + * const iterator = await new DurableIterator(channelName, options) + */ then<TResult1 = ClientDurableIterator<T, RPC>, TResult2 = never>(packages/durable-iterator/src/client/plugin.test.ts (2)
31-31: Remove unnecessaryasyncfrom describe block
describeshould not beasync. Drop it to avoid unintended promise handling during test definition.-describe('durableIteratorLinkPlugin', async () => { +describe('durableIteratorLinkPlugin', () => {
307-326: Avoid registeringafterEachinside anit; clean up the listener within the testRegistering
afterEachinside a test is brittle and leaks listeners across tests. Clean up withprocess.offat the end (or use try/finally).- const unhandledRejectionHandler = vi.fn() - process.on('unhandledRejection', unhandledRejectionHandler) - afterEach(() => { - process.off('unhandledRejection', unhandledRejectionHandler) - }) + const unhandledRejectionHandler = vi.fn() + process.on('unhandledRejection', unhandledRejectionHandler) @@ await output.return() // cleanup @@ expect(unhandledRejectionHandler.mock.calls[0]![0]).toEqual( new DurableIteratorError(`Expected valid token for procedure durableIterator`), ) + process.off('unhandledRejection', unhandledRejectionHandler)packages/durable-iterator/src/plugin.ts (1)
11-16: Update docs link to new Durable Iterator pageThe reference still points to durable-event-iterator. Update to durable-iterator.
-/** - * @see {@link https://orpc.unnoq.com/docs/integrations/durable-event-iterator Durable Event Iterator Integration} - */ +/** + * @see {@link https://orpc.unnoq.com/docs/integrations/durable-iterator Durable Iterator Integration} + */playgrounds/cloudflare-worker/worker-configuration.d.ts (4)
435-436: Empty interface is intentional; suppress Biome for this generated file
interface DurableObjectClass<...> {}is used for declaration merging; do not convert to a type alias. Suppress Biome’s noEmptyInterface for this generated d.ts.Apply suppression inline if you must appease Biome:
+/* biome-ignore lint/suspicious/noEmptyInterface: declaration merging in generated types */ interface DurableObjectClass<_T extends Rpc.DurableObjectBranded | undefined = undefined> { }Alternatively, exclude this file or the rule in your Biome config for generated d.ts files.
1380-1384: Fix generic parameter in Request constructor
RequestInfois parameterized as<CfHostMetadata, Cf>, but here it’s passed<CfProperties>, which is incorrect and degrades typing.Apply:
declare var Request: { prototype: Request; - new <CfHostMetadata = unknown, Cf = CfProperties<CfHostMetadata>>(input: RequestInfo<CfProperties> | URL, init?: RequestInit<Cf>): Request<CfHostMetadata, Cf>; + new <CfHostMetadata = unknown, Cf = CfProperties<CfHostMetadata>>(input: RequestInfo<CfHostMetadata, Cf> | URL, init?: RequestInit<Cf>): Request<CfHostMetadata, Cf>; };
5425-5430: Use Omit instead of Exclude to drop object properties
Exclude<GatewayOptions, 'id'>doesn’t remove theidkey from an object type. UseOmitto makeidoptional correctly.Apply:
-type UniversalGatewayOptions = Exclude<GatewayOptions, 'id'> & { +type UniversalGatewayOptions = Omit<GatewayOptions, 'id'> & { /** ** @deprecated */ id?: string; };
5510-5511: Empty interfaces are by design for declaration merging—suppress Biome
AutoRAGNameNotSetError,Cloudflare.Env, andCloudflare.GlobalPropsare intentionally empty for interface merging. Don’t convert to type aliases; suppress Biome on these or exclude generated d.ts from linting.Options:
- Add inline suppressions:
/* biome-ignore lint/suspicious/noEmptyInterface: enables declaration merging */- Or ignore this file/glob in Biome config (preferred for generated artifacts).
Also applies to: 6982-6990, 7010-7011
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (21)
packages/durable-iterator/README.md (2)
64-64: Add install snippet and brief experimental/migration note under the new section.Improves DX and reduces confusion coming from the rename.
## `@orpc/experimental-durable-iterator` +[!TIP] +> Experimental: API surface may change. This is the rewrite/rename of `@orpc/durable-event-iterator`. +> See migration notes in the docs (Durable Iterator integration guide). + +### Install + +```bash +pnpm add @orpc/experimental-durable-iterator +# or +npm i @orpc/experimental-durable-iterator +# or +yarn add @orpc/experimental-durable-iterator +```
5-5: Fix empty H1.An empty top-level heading hurts readability and accessibility. Set a meaningful title.
-<h1></h1> +<h1>oRPC</h1>.github/workflows/release.yaml (1)
48-52: Add cache-dependency-path for reliable PNPM cache hits.Being explicit avoids cache misses in mono-repos or non-standard lockfile paths.
Apply this diff:
- - uses: actions/setup-node@v5 - with: - node-version: 22 - cache: pnpm + - uses: actions/setup-node@v5 + with: + node-version: 22 + cache: pnpm + cache-dependency-path: pnpm-lock.yamlOptionally pin PNPM for reproducibility (outside this hunk):
- uses: pnpm/action-setup@v4 with: version: 9.12.3 # pick your org’s pinned version.github/workflows/release-next.yaml (3)
18-22: Move Node setup before PNPM setup.Set up Node first to ensure the toolchain is consistent when pnpm is installed/configured.
Proposed reordering (outside the selected range):
- - uses: pnpm/action-setup@v4 - - - uses: actions/setup-node@v5 + - uses: actions/setup-node@v5 with: node-version: 22 cache: pnpm + cache-dependency-path: '**/pnpm-lock.yaml' + + - uses: pnpm/action-setup@v4
18-22: Improve pnpm cache hit rate in monorepos.Explicitly set cache-dependency-path to lockfiles to avoid cache misses when lockfiles aren’t at repo root.
Apply within this step:
- uses: actions/setup-node@v5 with: node-version: 22 cache: pnpm + cache-dependency-path: '**/pnpm-lock.yaml'
18-22: Optional: prevent duplicate publishes on rapid pushes.Add a concurrency group to cancel in-progress runs of this job.
Example (outside the selected range):
concurrency: group: release-next-${{ github.ref }} cancel-in-progress: true.github/workflows/ci.yaml (3)
35-39: Consider a Node matrix for tests.Run tests across LTS/current (e.g., 18.x, 20.x, 22.x) to catch version-specific issues, while keeping lint/publish on a single Node version.
Example (outside this hunk):
jobs: test: runs-on: ubuntu-latest strategy: fail-fast: false matrix: node: [18.x, 20.x, 22.x] steps: - uses: actions/checkout@v4 - uses: actions/setup-node@v5 with: node-version: ${{ matrix.node }} check-latest: true cache: pnpm cache-dependency-path: pnpm-lock.yaml - uses: pnpm/action-setup@v4 - run: pnpm i - run: pnpm run test:coverage
55-59: Mirror the caching/versioning hardening here.Apply the same tweaks as in the lint job:
- - uses: actions/setup-node@v5 - with: - node-version: 22 - cache: pnpm + - uses: actions/setup-node@v5 + with: + node-version: 22.x + check-latest: true + cache: pnpm + cache-dependency-path: pnpm-lock.yaml
17-21: Harden setup-node: pin to 22.x, enable check-latest, and cache pnpm via lockfilepackageManager is pnpm@10.16.1 and pnpm-lock.yaml exists — apply the diff below and mirror in other jobs.
- - uses: actions/setup-node@v5 - with: - node-version: 22 - cache: pnpm + - uses: actions/setup-node@v5 + with: + node-version: 22.x + check-latest: true + cache: pnpm + cache-dependency-path: pnpm-lock.yamlRecommended step order for each job:
- uses: actions/checkout@v4 - uses: actions/setup-node@v5 with: node-version: 22.x check-latest: true cache: pnpm cache-dependency-path: pnpm-lock.yaml - uses: pnpm/action-setup@v4 - run: pnpm ipackages/durable-iterator/src/client/plugin.ts (8)
232-245: Don’t throw when plugin context is absent; fail open.Throwing here can break unrelated calls if another interceptor drops context. Degrade gracefully by proceeding without marking the response.
- const pluginContext = options.context[this.CONTEXT_SYMBOL] as DurableIteratorLinkPluginContext | undefined - - if (!pluginContext) { - throw new DurableIteratorError('Plugin context has been corrupted or modified by another plugin or interceptor') - } - - const response = await options.next() - - pluginContext.isDurableIteratorResponse = response.headers[DURABLE_ITERATOR_PLUGIN_HEADER_KEY] === DURABLE_ITERATOR_PLUGIN_HEADER_VALUE - - return response + const pluginContext = options.context[this.CONTEXT_SYMBOL] as DurableIteratorLinkPluginContext | undefined + const response = await options.next() + if (pluginContext) { + pluginContext.isDurableIteratorResponse = + response.headers[DURABLE_ITERATOR_PLUGIN_HEADER_KEY] === DURABLE_ITERATOR_PLUGIN_HEADER_VALUE + } + return response
183-187: Guard clearTimeout against undefined.Prevents a TS type error and is safer across environments.
const closeConnection = () => { isFinished = true - clearTimeout(refreshTokenBeforeExpireTimeoutId) + if (refreshTokenBeforeExpireTimeoutId !== undefined) { + clearTimeout(refreshTokenBeforeExpireTimeoutId) + } websocket.close() }
136-180: Clamp refresh delay and bail out early if finished.
- Clamp the setTimeout delay to [0, 2_147_483_647] ms to avoid negative/overflow values.
- Exit early after fetching a new token if the iterator was finished during refresh.
- const nowInSeconds = Math.floor(Date.now() / 1000) - - refreshTokenBeforeExpireTimeoutId = setTimeout(async () => { + const nowInSeconds = Math.floor(Date.now() / 1000) + const delayMs = Math.max( + 0, + Math.min(2_147_483_647, Math.round((tokenAndPayload.payload.exp - nowInSeconds - beforeSeconds) * 1000)), + ) + + refreshTokenBeforeExpireTimeoutId = setTimeout(async () => { // retry until success or finished const newTokenAndPayload = await retry({ times: Number.POSITIVE_INFINITY, delay: 2000 }, async (exit) => { try { const output = await next() return this.validateToken(output, options.path) } catch (err) { if (isFinished) { exit(err) } throw err } }) + if (isFinished) { + return + } + const canProactivelyUpdateToken = newTokenAndPayload.payload.chn === tokenAndPayload.payload.chn && stringifyJSON(newTokenAndPayload.payload.tags) === stringifyJSON(tokenAndPayload.payload.tags) tokenAndPayload = newTokenAndPayload await refreshTokenBeforeExpire() // recursively call @@ - }, (tokenAndPayload.payload.exp - nowInSeconds - beforeSeconds) * 1000) + }, delayMs)
110-115: Support relative WS URLs (or explicitly document absolute requirement).new URL(...) throws on relative URLs without a base. Either clamp to absolute in docs or support relative using location.origin.
- const url = new URL(await value(this.url, tokenAndPayload.payload, options)) + const raw = await value(this.url, tokenAndPayload.payload, options) + const base = (typeof globalThis !== 'undefined' && (globalThis as any).location?.origin) || undefined + const url = typeof raw === 'string' ? new URL(raw, base) : new URL(raw.toString())
121-125: Avoid duplicate ClientRetryPlugin instances.If callers already pass a ClientRetryPlugin, this adds a second instance. Dedup to prevent double handling.
- plugins: [ - ...toArray(this.linkOptions.plugins), - new ClientRetryPlugin(), - ], + plugins: [ + ...toArray(this.linkOptions.plugins).filter(p => !(p instanceof ClientRetryPlugin)), + new ClientRetryPlugin(), + ],
72-77: Provide a robust fallback for createId when crypto.randomUUID is unavailable.Older runtimes may lack crypto.randomUUID. Use getRandomValues when available; otherwise, fallback.
Please confirm your target runtimes (Node/browser versions). If Node < 16.17 or older browsers are in scope, apply:
constructor({ url, refreshTokenBeforeExpireInSeconds, ...options }: DurableIteratorLinkPluginOptions<T>) { this.url = url - this.createId = fallback(options.createId, () => crypto.randomUUID()) + this.createId = fallback(options.createId, () => { + const c: any = (globalThis as any).crypto + if (c?.randomUUID) return c.randomUUID() + const bytes: Uint8Array = + c?.getRandomValues?.(new Uint8Array(16)) ?? Uint8Array.from({ length: 16 }, () => Math.floor(Math.random() * 256)) + bytes[6] = (bytes[6] & 0x0f) | 0x40 + bytes[8] = (bytes[8] & 0x3f) | 0x80 + const hex: string[] = [] + for (const b of bytes) hex.push(b.toString(16).padStart(2, '0')) + return `${hex.slice(0, 4).join('')}-${hex.slice(4, 6).join('')}-${hex.slice(6, 8).join('')}-${hex.slice(8, 10).join('')}-${hex.slice(10).join('')}` + }) this.refreshTokenBeforeExpireInSeconds = fallback(refreshTokenBeforeExpireInSeconds, Number.NaN) this.linkOptions = options }
49-51: Doc nit: “an infinite value”.Grammar fix in the option JSDoc.
- * - Use a infinite value to disable refreshing + * - Use an infinite value to disable refreshing
140-152: Replace fixed-delay infinite retry with exponential backoff + jitter (cap)
packages/durable-iterator/src/client/plugin.ts (~line 140): currently uses retry({ times: Number.POSITIVE_INFINITY, delay: 2000 }, ...). radash.retry (re-exported via packages/shared) supports a backoff function — use exponential backoff with jitter and a max delay (and pass options.signal if available) to avoid thundering-herd (e.g. backoff: i => Math.min(30_000, Math.floor(Math.random() * (2000 * 2 ** (i - 1))))).packages/durable-iterator/src/plugin.ts (4)
31-34: Prevent pluginContext from being overwritten by an existing context entrySpread options.context first, then set your symbol to ensure your fresh pluginContext takes precedence.
Apply:
- context: { - [this.CONTEXT_SYMBOL]: pluginContext, - ...options.context, - }, + context: { + ...options.context, + [this.CONTEXT_SYMBOL]: pluginContext, + },
45-50: Avoid injecting undefined header valuesBuild headers conditionally to skip the key entirely when not applicable.
Apply:
headers: { ...result.response.headers, - [DURABLE_ITERATOR_PLUGIN_HEADER_KEY]: pluginContext.isClientDurableIteratorOutput - ? DURABLE_ITERATOR_PLUGIN_HEADER_VALUE - : undefined, + ...(pluginContext.isClientDurableIteratorOutput + ? { [DURABLE_ITERATOR_PLUGIN_HEADER_KEY]: DURABLE_ITERATOR_PLUGIN_HEADER_VALUE } + : {}), },
56-61: Degrade gracefully if plugin context is absentThrowing here can break legitimate flows where the client interceptors run outside the request pipeline. Pass-through instead.
Apply:
- if (!pluginContext) { - throw new DurableIteratorError('Plugin context has been corrupted or modified by another plugin or interceptor') - } - - const output = await options.next() + const output = await options.next() + if (!pluginContext) { + // Outside request context or plugin not active — pass through + return output + }
64-69: LGTM — token extraction and response shapingHeader key/value are defined in packages/durable-iterator/src/consts.ts (DURABLE_ITERATOR_PLUGIN_HEADER_KEY = 'x-orpc-dei', DURABLE_ITERATOR_PLUGIN_HEADER_VALUE = '1'); if retained for backward compatibility, add a short doc comment or changelog entry.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/ci.yaml(3 hunks).github/workflows/release-next.yaml(1 hunks).github/workflows/release.yaml(1 hunks)packages/durable-iterator/README.md(2 hunks)packages/durable-iterator/src/client/iterator.test.ts(1 hunks)packages/durable-iterator/src/client/iterator.ts(1 hunks)packages/durable-iterator/src/client/plugin.test.ts(1 hunks)packages/durable-iterator/src/client/plugin.ts(1 hunks)packages/durable-iterator/src/durable-object/upgrade.test.ts(1 hunks)packages/durable-iterator/src/durable-object/websocket.ts(1 hunks)packages/durable-iterator/src/iterator.test-d.ts(1 hunks)packages/durable-iterator/src/plugin.test.ts(3 hunks)packages/durable-iterator/src/plugin.ts(2 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/durable-iterator/src/client/plugin.test.ts
- packages/durable-iterator/src/client/iterator.ts
- packages/durable-iterator/src/iterator.test-d.ts
- packages/durable-iterator/src/client/iterator.test.ts
- packages/durable-iterator/src/plugin.test.ts
- packages/durable-iterator/src/durable-object/websocket.ts
- packages/durable-iterator/src/durable-object/upgrade.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/durable-iterator/src/client/plugin.ts (9)
packages/shared/src/value.ts (3)
Value(1-1)fallback(17-19)value(3-12)packages/shared/src/index.ts (2)
Promisable(23-23)retry(22-22)packages/durable-iterator/src/schemas.ts (2)
DurableIteratorTokenPayload(6-6)parseDurableIteratorToken(52-60)packages/durable-iterator/src/consts.ts (4)
DURABLE_ITERATOR_ID_PARAM(2-2)DURABLE_ITERATOR_TOKEN_PARAM(1-1)DURABLE_ITERATOR_PLUGIN_HEADER_KEY(3-3)DURABLE_ITERATOR_PLUGIN_HEADER_VALUE(4-4)packages/client/src/client.ts (1)
createORPCClient(17-43)packages/shared/src/array.ts (1)
toArray(1-3)packages/shared/src/json.ts (1)
stringifyJSON(9-12)packages/client/src/types.ts (1)
ClientLink(36-38)packages/durable-iterator/src/client/iterator.ts (1)
createClientDurableIterator(40-69)
packages/durable-iterator/src/plugin.ts (6)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/adapters/standard/plugin.ts (1)
StandardHandlerPlugin(5-8)packages/server/src/adapters/standard/handler.ts (1)
StandardHandlerOptions(27-48)packages/durable-iterator/src/consts.ts (2)
DURABLE_ITERATOR_PLUGIN_HEADER_KEY(3-3)DURABLE_ITERATOR_PLUGIN_HEADER_VALUE(4-4)packages/durable-iterator/src/error.ts (1)
DurableIteratorError(1-2)packages/durable-iterator/src/client/iterator.ts (1)
getClientDurableIteratorToken(74-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: publish-commit
🔇 Additional comments (7)
packages/durable-iterator/README.md (1)
11-12: NPM link/badge update looks correct; verify publish timing.The URL encoding and package name match the new experimental package. Please ensure the package is (or will be) published under this name so the badge resolves correctly post-merge.
.github/workflows/release.yaml (1)
48-52: LGTM: Node 22 + pnpm cache step added; verify provenance on publishpackage.json (root) contains:
pnpm run build:packages && pnpm --filter='./packages/*' publish -r --access=public
No explicit --provenance flag found; add --provenance to the publish command or confirm your pnpm/npm version/config will emit provenance during publish..github/workflows/release-next.yaml (1)
18-22: Verify Node 22 compatibility & pin pnpm in CIAutomated scan found no "engines" or "packageManager" fields in package.json files and no workflows referencing pnpm/action-setup — confirm all packages work on Node 22 and pin pnpm (via packageManager or workflow action) to avoid CI drift.
packages/durable-iterator/src/client/plugin.ts (1)
62-66: Plugin ordering looks correct.High order to run before retry prevents iterator breakage. Good.
packages/durable-iterator/src/plugin.ts (3)
3-5: LGTM: imports updated to new APINew token getter, consts, and error import look correct.
7-9: LGTM: plugin context shapeMinimal context with a single boolean flag is appropriate.
14-16: LGTM: class rename and context symbolRenamed plugin and per-instance symbol usage are consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/durable-iterator/src/durable-object/upgrade.test.ts (2)
18-41: Align signing key in “missing client id” test to avoid order‑dependencyThe test currently signs with 'signing-key' but passes options.signingKey = 'test-sign'. It still passes due to earlier ID check, but aligning keys removes reliance on validation order.
Minimal change:
- const token = await signDurableIteratorToken('signing-key', { + const token = await signDurableIteratorToken('test-sign', {
105-111: Optional: reduce time‑based flakinessUsing Date.now() is fine here, but you can lock time with vi.setSystemTime for fully deterministic tokens.
Also applies to: 117-127
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/durable-iterator/src/durable-object/upgrade.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/durable-iterator/src/durable-object/upgrade.test.ts (2)
packages/durable-iterator/src/schemas.ts (1)
signDurableIteratorToken(20-22)packages/durable-iterator/src/consts.ts (2)
DURABLE_ITERATOR_TOKEN_PARAM(1-1)DURABLE_ITERATOR_ID_PARAM(2-2)
🪛 GitHub Check: test
packages/durable-iterator/src/durable-object/upgrade.test.ts
[failure] 132-132: packages/durable-iterator/src/durable-object/upgrade.test.ts > upgradeDurableIteratorRequest > upgrades valid request
AssertionError: expected "spy" to be called with arguments: [ …(2) ]
Received:
1st spy call:
@@ -12,8 +12,8 @@
110,
101,
108,
],
{
-
"test": "test",
-
},
"locationHint": "something",
]
Number of calls: 1
❯ packages/durable-iterator/src/durable-object/upgrade.test.ts:132:27
🪛 GitHub Actions: CI
packages/durable-iterator/src/durable-object/upgrade.test.ts
[error] 132-132: AssertionError: expected "spy" to be called with arguments: [ …(2) ]; Received: payload mismatch where expected { "test": "test" } but got { "locationHint": "something" } in the first call. This indicates the upgradeDurableIteratorRequest test case failed.
🔇 Additional comments (1)
packages/durable-iterator/src/durable-object/upgrade.test.ts (1)
5-16: Good coverage of rejection pathsSolid negative tests for non‑websocket, missing id, missing token, invalid token/payload. Assertions match the upgrade logic and status codes.
Also applies to: 43-58, 60-76, 78-95
1 participant
targetsandexcludeas callback for filterCloses: https://github.com/unnoq/orpc/issues/993
Summary by CodeRabbit
New Features
Documentation
Chores