feat(client): consumeEventIterator utility - #976
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
the
size:L
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new exported utility Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as App Code
participant CEI as consumeEventIterator
participant AI as AsyncIterator
participant CB as Callbacks
App->>CEI: consumeEventIterator(iteratorOrPromise, { onEvent, onError, onSuccess, onFinish })
CEI->>AI: await iteratorOrPromise
loop iterate until done or error
CEI->>AI: next()
alt yields value (done=false)
AI-->>CEI: { value, done:false }
CEI-->>CB: onEvent(value)
else completes (done=true)
AI-->>CEI: { value, done:true }
CEI-->>CB: onSuccess(value?)
CEI-->>CB: onFinish([null, value?, true])
end
end
rect rgba(240,80,80,0.08)
note right of CEI: Error path
AI--xCEI: throw / rejected promise
CEI-->>CB: onError?(error)
CEI-->>CB: onFinish([error, undefined, false])
end
%% Unsubscribe flow
App-->>CEI: const unsubscribe = (...)
App->>CEI: await unsubscribe()
CEI->>AI: return() // graceful termination
CEI-->>CB: onFinish([null, undefined, true])
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new utility function, consumeEventIterator, to the client library. Its purpose is to streamline the consumption of asynchronous event streams by providing a structured way to handle events, errors, and completion states through lifecycle callbacks, simplifying the management of streaming data.
Highlights
- New Utility Function: Introduces
consumeEventIteratorin@orpc/clientfor managing event streams. - Lifecycle Callbacks: Provides
onEvent,onError,onSuccess, andonFinishcallbacks for comprehensive stream handling. - Unsubscribe Mechanism: The utility returns a function to gracefully stop the event stream.
- Documentation: Adds detailed documentation and usage examples for the new utility.
- Robust Testing: Includes extensive unit and type tests to ensure correctness and proper type inference.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a new consumeEventIterator utility, which is a great addition for simplifying event stream consumption. The implementation is solid, and it's well-documented and tested. I've identified a high-severity type safety issue in the error handling logic and a medium-severity issue in one of the tests concerning test isolation. My feedback includes suggestions to address these points to improve the robustness and correctness of the new utility and its tests.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
packages/client/src/utils.ts (3)
85-86: Make unsubscribe wait for onFinish to avoid a race on teardown.Currently,
cancel()resolves afteriterator.return()completes but beforeonFinishis guaranteed to run. Tie the returned promise to the internal runner so callers whoawait cancel()know finalizers have fired.- void (async () => { + const runner = (async () => { @@ - })() + })() @@ - return async () => { - await (await iterator)?.return?.() - } + return async () => { + const it = await iterator + try { + await it?.return?.() + } finally { + if (options.onFinish) { + // Ensure onFinish has fired before resolving cancel() + await runner.catch(() => undefined) + } + } + }Also applies to: 123-125
81-84: Improve type inference with overloads for plain vs. promised iterators.Overloads make
TErrorinfer asunknownfor plain iterators and as the provided error type for promised ones (matching your dts tests).export function consumeEventIterator<T, TReturn, TError>( - iterator: AsyncIteratorObject<T, TReturn> | ClientPromiseResult<AsyncIteratorObject<T, TReturn>, TError>, - options: ConsumeEventIteratorOptions<T, TReturn, TError>, -): () => Promise<void> { + iterator: ClientPromiseResult<AsyncIteratorObject<T, TReturn>, TError>, + options: ConsumeEventIteratorOptions<T, TReturn, TError>, +): () => Promise<void> +export function consumeEventIterator<T, TReturn>( + iterator: AsyncIteratorObject<T, TReturn>, + options: ConsumeEventIteratorOptions<T, TReturn, unknown>, +): () => Promise<void> +export function consumeEventIterator<T, TReturn, TError>( + iterator: AsyncIteratorObject<T, TReturn> | ClientPromiseResult<AsyncIteratorObject<T, TReturn>, TError>, + options: ConsumeEventIteratorOptions<T, TReturn, TError>, +): () => Promise<void> {
102-120: Callback exceptions will surface as unhandled rejections.
onEvent/onError/onSuccess/onFinishexceptions will reject the internal task (even if handlers are present). If that’s intentional, document it; otherwise, wrap callbacks in try/catch and report via a user-provided logger hook.apps/content/docs/client/event-iterator.md (1)
72-98: Add a short note about required error handling and cancel semantics.Make it explicit that at least one of
onErrororonFinishshould be provided and thatawait cancel()waits for graceful shutdown.## Using `consumeEventIterator` oRPC provides a utility function `consumeEventIterator` to consume an event iterator with lifecycle callbacks. +::: warning +Provide at least one of `onError` or `onFinish`. If neither is supplied, rejections bubble as unhandled promise rejections. +::: + ```ts import { consumeEventIterator } from '@orpc/client' @@ setTimeout(async () => { // Stop the stream after 1 second - await cancel() + // Awaiting ensures the iterator has been gracefully closed before proceeding. + await cancel() }, 1000)</blockquote></details> <details> <summary>packages/client/src/utils.test.ts (3)</summary><blockquote> `131-170`: **Stabilize “unsubscribe” timing to avoid flakiness.** The 1ms delay races the generator’s first yield (25ms). Wait for the first event, then unsubscribe. Apply: ```diff - await new Promise(resolve => setTimeout(resolve, 1)) - await unsubscribe() + await vi.waitFor(() => expect(onEvent).toHaveBeenCalledTimes(1)) + await unsubscribe()
44-49: Avoid unused variable in tests.unsubscribe isn’t used in this case; drop the binding.
Apply:
- const unsubscribe = consumeEventIterator(iterator, { + void consumeEventIterator(iterator, {
78-83: Same nit: drop unused unsubscribe binding.Apply:
- const unsubscribe = consumeEventIterator(iterator, { + void consumeEventIterator(iterator, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/content/docs/client/event-iterator.md(1 hunks)packages/client/src/utils.test-d.ts(2 hunks)packages/client/src/utils.test.ts(2 hunks)packages/client/src/utils.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/client/src/utils.test-d.ts (3)
packages/client/src/utils.ts (1)
consumeEventIterator(81-126)packages/client/src/types.ts (1)
ClientPromiseResult(24-24)packages/shared/src/interceptor.ts (1)
OnFinishState(64-66)
packages/client/src/utils.ts (2)
packages/shared/src/interceptor.ts (1)
OnFinishState(64-66)packages/client/src/types.ts (1)
ClientPromiseResult(24-24)
packages/client/src/utils.test.ts (2)
packages/shared/src/interceptor.ts (3)
onError(46-62)onSuccess(33-41)onFinish(71-97)packages/client/src/utils.ts (1)
consumeEventIterator(81-126)
🪛 Biome (2.1.2)
packages/client/src/utils.test.ts
[error] 185-185: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
⏰ 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). (2)
- GitHub Check: publish-commit
- GitHub Check: lint
🔇 Additional comments (4)
packages/client/src/utils.test-d.ts (1)
70-104: Type inference checks look solid.Covers both promised and plain iterator inputs;
TErrorinference matches expectations.packages/client/src/utils.test.ts (3)
31-63: Great success-path coverage for consumeEventIterator.Assertions for per-yield onEvent, onSuccess, and onFinish state look correct and match the implementation.
65-98: Error-path behavior verified well.Good checks for onError firing, onFinish state, and absence of onSuccess.
216-243: Iterator-promise rejection test looks solid.Covers the branch where iterator is a rejected promise; correct expectations for onError/onFinish and zero events.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/client/src/utils.test-d.ts (2)
70-86: Also assert the unsubscribe function’s type.
consumeEventIteratorreturns() => Promise<void>. Asserting it here hardens the contract and guards regressions.- void consumeEventIterator({} as ClientPromiseResult<AsyncGenerator<'message-value', 'done-value'>, 'error-value'>, { + const cancel = consumeEventIterator({} as ClientPromiseResult<AsyncGenerator<'message-value', 'done-value'>, 'error-value'>, { onEvent: (message) => { expectTypeOf(message).toEqualTypeOf<'message-value'>() }, onError: (error) => { expectTypeOf(error).toEqualTypeOf<'error-value'>() }, onSuccess: (value) => { expectTypeOf(value).toEqualTypeOf<'done-value' | undefined>() }, onFinish: (state) => { expectTypeOf(state).toEqualTypeOf<OnFinishState<'done-value' | undefined, 'error-value'>>() }, }) + expectTypeOf(cancel).toEqualTypeOf<() => Promise<void>>()
88-103: Mirror the return-type assertion for the plain AsyncIterator case.Keeps both branches equally covered.
- void consumeEventIterator({} as AsyncIterator<'message-value', 'done-value'>, { + const cancel = consumeEventIterator({} as AsyncIterator<'message-value', 'done-value'>, { onEvent: (message) => { expectTypeOf(message).toEqualTypeOf<'message-value'>() }, onError: (error) => { expectTypeOf(error).toEqualTypeOf<unknown>() }, onSuccess: (value) => { expectTypeOf(value).toEqualTypeOf<'done-value' | undefined>() }, onFinish: (state) => { expectTypeOf(state).toEqualTypeOf<OnFinishState<'done-value' | undefined, unknown>>() }, }) + expectTypeOf(cancel).toEqualTypeOf<() => Promise<void>>()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/client/src/utils.test-d.ts(2 hunks)packages/client/src/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/client/src/utils.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/client/src/utils.test-d.ts (3)
packages/client/src/utils.ts (1)
consumeEventIterator(81-126)packages/client/src/types.ts (1)
ClientPromiseResult(24-24)packages/shared/src/interceptor.ts (1)
OnFinishState(64-66)
⏰ 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). (2)
- GitHub Check: publish-commit
- GitHub Check: lint
🔇 Additional comments (1)
packages/client/src/utils.test-d.ts (1)
1-1: Imports LGTM and correctly marked as type-only.The new public types and util are imported cleanly. No issues.
Also applies to: 3-3, 5-5
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests