fix(evlog): do not log ErrorEvent from event stream response bodies - #1918
Conversation
The routing interceptor's response body wrapper logged every error as an internal failure, but the codec encodes business errors (e.g. ORPCError) thrown from event iterators as ErrorEvent for the wire, so each streamed business error was logged twice: once correctly by the client interceptor and again at error level as a bogus internal failure.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/bun
@orpc/client
@orpc/cloudflare
@orpc/contract
@orpc/experimental-effect
@orpc/evlog
@orpc/hibernation
@orpc/json-schema
@orpc/experimental-msw
@orpc/nest
@orpc/next
@orpc/node
@orpc/openapi
@orpc/opentelemetry
@orpc/pinia-colada
@orpc/pino
@orpc/publisher
@orpc/ratelimit
@orpc/server
@orpc/shared
@orpc/swr
@orpc/tanstack-query
@orpc/trpc
@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.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
packages/evlog/src/handler-plugin.ts— guard the routing interceptor's streamonErrorso anErrorEvent(the codec's wire-encoding of a thrown iterator error) no longer produces a spurious internal-failure error log.packages/evlog/src/handler-plugin.test.ts— new test throwsORPCErrorfrom an event iterator, serializes via the realRPCSerializer, and asserts nothing is logged at error level while the stream still finishes with status 200.
The core fix is sound: RPCSerializer.serialize (packages/client/src/rpc-serializer.ts:59) converts every source-iterator error — business or internal — into an ErrorEvent via toORPCError, and the plugin's clientInterceptor already logs all of those at error/warn/info. So the routing wrapper's onError was a duplicate for the standard procedure path, and skipping ErrorEvent there is correct.
One inline note on the comment/scope and one nitpick on the test's coverage.
ℹ️ Nitpicks
- The new comment describes
ErrorEventas strictly "a business error ... already logged by the client interceptor." BecauseRPCSerializerwraps any thrown iterator error into anErrorEvent, the guard is broader than that rationale implies — worth tightening the wording so future readers don't rely on the narrow claim.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| * Errors here are internal (interceptor/framework) failures, | ||
| * except `ErrorEvent`: a business error the protocol delivers | ||
| * inside the event stream, already logged by the client interceptor. | ||
| */ | ||
| logger.error(toErrorOrString(error)) | ||
| if (!(error instanceof ErrorEvent)) { | ||
| logger.error(toErrorOrString(error)) |
There was a problem hiding this comment.
Minor scope note: an ErrorEvent is not strictly a business error. RPCSerializer.serialize's mapError (packages/client/src/rpc-serializer.ts:59) wraps any error thrown from the source iterator via toORPCError into an ErrorEvent — internal failures included. Suppression here is correct in practice only because the plugin's clientInterceptor (logBusinessLogicError, lines 258-272) already logs every iterator error from the procedure path first (error for non-ORPCError, warn for ORPCError, info for aborts). The residual gap is narrow: an error thrown while succeeding a yielded stream value during serialization (a codec transform failure) is wrapped into ErrorEvent above the client-interceptor wrap, so it is no longer error-logged here. Consider rewording the comment to say the guard suppresses the protocol's serialized error envelope rather than "a business error".
1 participant
Business errors thrown from event iterator procedures were logged twice by
EvlogHandlerPlugin: once correctly by the client interceptor, and again at error level by the routing interceptor's response body wrapper, because the codec encodes them asErrorEventfor the wire and the wrapper treated every body error as an internal failure.Fixes
ORPCError, info for aborts) is now the only record.ReadableStreambody errors, and all pre-codec error paths are logged exactly as before.Testing
ORPCErrorfrom an event iterator, encodes it through the realRPCSerializer, and asserts nothing is logged while the wide event still finishes with the response status.