You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix crash on Azure stream chunks without delta - #29
TypeError: Cannot destructure property 'content' of 'part.choices[0].delta' as it is undefined.
at /dist/openai/openai.service.js:294:37
With AI_PROVIDER=openai-azure, Azure sometimes sends stream chunks where choices[0] has no delta, typically content-filter chunks like { choices: [{ index: 0, finish_reason: null, content_filter_results: {} }] }. The throw happened inside a new Promise(async ...) executor. That made it an unhandled rejection, which killed the Node process and every in-flight chat on the pod.
Changes
Read the content with part.choices?.[0]?.delta?.content and skip the chunk when it's null/undefined. Before, content: null would have added the text "null" to the answer.
Replace the new Promise(async ...) with a private forwardCompletionStream method that is started without being awaited and never rejects:
If the stream fails, it logs the error and errors the observable with a generic message. The @Sse('/answer_stream') endpoint is the only consumer, and NestJS already turns that into an SSE error event for that one client. A partial answer is not saved.
Token counting and completeCb are now inside a try/catch, and completeCb is awaited, so errors there are logged. Before, it was fire-and-forget, which was a second unhandled-rejection path (e.g. when saving the session message fails).
Add optional chaining to res.choices[0].message.content in the non-streaming completion. An empty (e.g. filtered) response now defaults to '', the same as the streaming path.
Add openai.service.spec.ts with a mocked Azure client. It covers the content-filter chunk, a stream that fails mid-answer, and a rejecting completion callback. On the old code, the first test fails with the same TypeError as production.
Why no global unhandledRejection handler
It wouldn't have prevented this crash. Sentry v7 runs requests inside a Node domain, and unhandled rejections inside a domain go to domain.emit('error') instead of process.on('unhandledRejection'). That's the "Emitted 'error' event on Domain instance" in the trace. Only an uncaughtException handler would catch it, and keeping the process alive after those is unsafe.
Azure OpenAI sends stream chunks where choices[0] has no delta (e.g.
content filter results). Destructuring delta threw inside an async
Promise executor, which became an unhandled rejection and killed the
process along with every in-flight chat.
Read content with optional chaining and skip empty chunks. Move the
stream loop into a method that never rejects: a stream failure errors
the observable for that one answer, and completion callback failures
are logged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move getTokenCount inside the guarded block so a tokenizer failure
cannot become an unhandled rejection, and default an empty
non-streaming response to '' to match the streaming path.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
APIError.isPrototypeOf(error) is always false for instances, so API
errors were never logged. Use instanceof and log the parsed error body,
since APIError has no data field.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Add regression coverage for non-streaming filtered responses returning an empty result.
Review effort: Lite Findings: None
Previously missed (1)
In code that hasn't changed since last review
Add regression test for empty choices fallback
src/openai/openai.service.ts:347
This fallback is untested: a non-streaming filtered response with choices: [] is the case this optional chaining is intended to handle. Please add a regression test for getChatGptCompletion that asserts it returns response: ''; otherwise this separate crash fix can be accidentally removed without a test failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Production
webcontainers are crash-looping with:With
AI_PROVIDER=openai-azure, Azure sometimes sends stream chunks wherechoices[0]has nodelta, typically content-filter chunks like{ choices: [{ index: 0, finish_reason: null, content_filter_results: {} }] }. The throw happened inside anew Promise(async ...)executor. That made it an unhandled rejection, which killed the Node process and every in-flight chat on the pod.Changes
part.choices?.[0]?.delta?.contentand skip the chunk when it'snull/undefined. Before,content: nullwould have added the text"null"to the answer.new Promise(async ...)with a privateforwardCompletionStreammethod that is started without being awaited and never rejects:@Sse('/answer_stream')endpoint is the only consumer, and NestJS already turns that into an SSEerrorevent for that one client. A partial answer is not saved.completeCbare now inside a try/catch, andcompleteCbis awaited, so errors there are logged. Before, it was fire-and-forget, which was a second unhandled-rejection path (e.g. when saving the session message fails).res.choices[0].message.contentin the non-streaming completion. An empty (e.g. filtered) response now defaults to'', the same as the streaming path.openai.service.spec.tswith a mocked Azure client. It covers the content-filter chunk, a stream that fails mid-answer, and a rejecting completion callback. On the old code, the first test fails with the sameTypeErroras production.Why no global
unhandledRejectionhandlerIt wouldn't have prevented this crash. Sentry v7 runs requests inside a Node
domain, and unhandled rejections inside a domain go todomain.emit('error')instead ofprocess.on('unhandledRejection'). That's the "Emitted 'error' event on Domain instance" in the trace. Only anuncaughtExceptionhandler would catch it, and keeping the process alive after those is unsafe.Verification
yarn buildpassesyarn test: 7 suites / 16 tests pass🤖 Generated with Claude Code