refactor: improve error handling in consumeAsyncIterator function#1639
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
More templates
@orpc/arktype
@orpc/bun
@orpc/client
@orpc/cloudflare
@orpc/contract
@orpc/experimental-effect
@orpc/evlog
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/next
@orpc/openapi
@orpc/opentelemetry
@orpc/pino
@orpc/publisher
@orpc/ratelimit
@orpc/server
@orpc/shared
@orpc/tanstack-query
@orpc/valibot
@orpc/zod
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This refactor changes the public consumeAsyncIterator contract so iterator errors are silently swallowed when no handler is provided, and leaves the related JSDoc warning stale.
Reviewed changes — removes the rethrow branch in consumeAsyncIterator's catch block so iterator errors are no longer surfaced as unhandled rejections.
- Remove rethrow in
consumeAsyncIterator— errors now pass only tooptions.onError?.(...); withoutonError, they are silently swallowed. - Delete unhandled-rejection test — the test covering the removed behavior is removed.
⚠️ Iterator errors now swallowed silently when no handler is provided
The replaced catch block deliberately threw the error from a fire-and-forget async IIFE so it landed in the unhandled-rejection channel. That behavior was documented by the @warning JSDoc and verified by the deleted test. The new catch only invokes options.onError?.(...); if the caller supplies neither onError nor onFinish, the error vanishes.
This changes the public API contract of an export used by @orpc/client and makes misconfigured callers harder to debug.
Technical details
# Iterator errors now swallowed silently when no handler is provided
## Affected sites
- `packages/shared/src/iterator.ts:220` — `catch (error)` now calls only `options.onError?.(...)`; no rethrow or fallback surfacing.
- `packages/shared/src/iterator.ts:187` — JSDoc `@warning` still claims errors are thrown into the unhandled-rejection channel, which is no longer true after this change.
- `packages/shared/src/iterator.test.ts:468-496` — the test that verified the unhandled-rejection path is removed without a replacement.
## Required outcome
- The documented behavior must match the actual behavior. If errors are intentionally swallowed, update or remove the stale `@warning` JSDoc.
- The new contract must be covered by a test so future refactors don't reintroduce or alter it accidentally.
## Suggested approach
- If silent swallowing is intentional: update the JSDoc to say errors are ignored when no `onError`/`onFinish` is provided, and add a test asserting that no unhandled rejection fires in that case.
- If silent swallowing is unintentional: keep the error surfaced. A cleaner alternative to throwing inside the void async IIFE is attaching a rejection handler to the IIFE promise and forwarding to the callbacks, or requiring `onError`/`onFinish` at the type level.
## Open questions for the human
- Is the new silent-swallowing behavior an intentional contract change, or should errors still be surfaced to callers who omit handlers?Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
| @@ -219,15 +219,6 @@ export function consumeAsyncIterator<T, TReturn, TError = ThrowableError>( | |||
| } | |||
| catch (error) { | |||
There was a problem hiding this comment.
This catch block now silently swallows iterator errors when neither onError nor onFinish is provided. The previous behavior threw the error into the unhandled-rejection channel so misconfigured callers were at least noisy. If silent swallowing is intentional, add a test that locks in the new contract and update the stale @warning JSDoc at iterator.ts:187.
| }) | ||
| }) | ||
|
|
||
| it('on error without onError and onFinish', async ({ onTestFinished }) => { |
There was a problem hiding this comment.
This test is removed without a replacement that documents the new error-handling contract. Add a test that verifies the expected behavior when no onError/onFinish is provided — either that no unhandled rejection is produced, or that the error is still surfaced somehow.
1 participant
No description provided.