feat(ai): add nitro stream logger#363
Conversation
|
@hussainarslan is attempting to deploy a commit to the HRCD Projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions! 🙏 |
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds createNitroAIStreamLogger() and supporting types/exports, routes late AI stream metadata to a correlated child wide event via Nitro enrich/drain hooks, wraps streaming Responses to emit on completion/error, adds tests, updates package exports/tsdown, and revises docs and an example endpoint. ChangesNitro AI Stream Logger for Evlog
Sequence DiagramsequenceDiagram
participant Handler as Request handler
participant NitroLogger as createNitroAIStreamLogger
participant Stream as ReadableStream
participant Enrich as evlog:enrich
participant Drain as evlog:drain
Handler->>NitroLogger: createNitroAIStreamLogger(event, options)
Handler->>Stream: return wrapped Response body (wrapResponse)
Stream->>NitroLogger: on close / error -> emit(childEvent)
NitroLogger->>Enrich: call evlog:enrich(childEvent)
Enrich->>Drain: pass enriched wide event
NitroLogger->>Drain: call evlog:drain(childEvent) (await or waitUntil)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evlog/src/ai/nitro.ts`:
- Around line 158-189: The function createObservedBody should defensively handle
the case where the incoming ReadableStream is already locked: before calling
body.getReader() check body.locked; if locked, call the provided onError with a
clear TypeError (e.g. "stream is already locked") and return the original body
unchanged (or rethrow if the caller expects an exception), otherwise proceed to
getReader as implemented; update any callers such as wrapResponse to expect this
early-return/error path so we don't attempt to lock an already-locked stream.
In `@packages/evlog/test/ai/nitro.test.ts`:
- Around line 107-124: The test currently mocks getHeaders to return an
authorization header which will be stripped by filterSafeHeaders; either remove
the authorization header from the mock or (preferred) add an explicit assertion
that the authorization header is absent in the data passed to
nitroRuntime.app.hooks.callHook (i.e. after invoking createNitroAIStreamLogger
in the first test assert that the hook call payload / event.headers does not
contain "authorization" or that filterSafeHeaders removed it), referencing the
existing getHeaders mock and the nitroRuntime.app.hooks.callHook mock used in
the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 231965f3-3349-4fc4-8a79-80fe56940b1e
⛔ Files ignored due to path filters (1)
packages/evlog/test/toolkit/__snapshots__/api-surface.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
.changeset/nitro-ai-streams.mdapps/docs/content/2.learn/2.wide-events.mdapps/docs/content/3.integrate/frameworks/04.nitro.mdapps/docs/content/4.use-cases/2.ai-sdk/01.overview.mdapps/docs/content/4.use-cases/2.ai-sdk/02.usage.mdapps/docs/content/4.use-cases/2.ai-sdk/04.metadata.mdapps/nuxthub-playground/server/api/chat.post.tspackages/evlog/README.mdpackages/evlog/package.jsonpackages/evlog/src/ai/nitro.tspackages/evlog/src/logger.tspackages/evlog/test/ai/nitro.test.tspackages/evlog/tsdown.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evlog/src/ai/nitro.ts`:
- Around line 163-166: The locked-stream branch calls onError(new
TypeError('stream is already locked')) with no rejection handling; change that
call to attach a rejection handler so promise rejections can't become unhandled
(e.g., replace the current void onError(...) with onError(new TypeError('stream
is already locked')).catch(() => {/* optionally log */}) or await it inside a
try/catch in the enclosing async context). Update the call in the locked-stream
branch (where body.locked is checked) to handle .catch so any emit/plugin
failures are absorbed or logged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 08e42136-bbc1-453a-acf6-2711d6accb33
📒 Files selected for processing (2)
packages/evlog/src/ai/nitro.tspackages/evlog/test/ai/nitro.test.ts
hussainarslan
force-pushed
the
fix/nitro-ai-stream-helper
branch
from
24df918 to
ef420fb
Compare
|
Thanks @hussainarslan for the PR, it really helped me dig into this. I'm not a huge fan of a Nitro-specific API here, I preferred fixing it lower down, at the streaming response body level (defer emit + late ai merge), while keeping createAILogger(log) everywhere. That approach is in #367, feel free to try it with the |
That's great. Glad to be of help |
2 participants
What
Adds createNitroAIStreamLogger() at evlog/ai/nitro for Nuxt/Nitro AI SDK streaming responses. The helper creates a correlated child request event, wraps streaming responses without buffering, records stream errors, and routes the child event through Nitro enrich/drain hooks including waitUntil.
Why
Issue #321 happens because AI SDK stream metadata can be written after Nitro has already emitted and sealed the parent request event. Keeping that lifecycle warning is useful, but streaming work needs its own event that can finish when the body closes.
How
The helper reads the parent logger with useLogger(event), creates a child request logger with a fresh requestId plus _parentRequestId, exposes createAILogger(childLog), and emits once when wrapResponse() observes stream close/error. Docs, README, playground usage, package exports, tsdown config, changeset, and the API snapshot were updated.
Validation
pnpm run dev:prepare
pnpm run lint
pnpm run typecheck
pnpm run test
pnpm test:coverage
pnpm api:snapshot
Fixes #321
Summary by CodeRabbit
New Features
Documentation
Tests
Chores