fix: Chat requests resolve when onFinish throws - #18755
Conversation
Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>
Co-authored-by: anubra266 <30869823+anubra266@users.noreply.github.com> Co-authored-by: Gregor Martynus <39992+gr2m@users.noreply.github.com>
Bugfix reviewOutcome: approved Reproduction replayStatus: no-longer-reproduces The exact original reproduction completed successfully and the original bug signal did not appear. Fixes issueStatus: fully-addresses The callback exception now escapes the request promise, while nested finally cleanup still clears the active response and resume-request state. Side effectsRisk: low Requests now reject and automatic follow-up sending is skipped when a user-provided onFinish callback throws; this is the intended correction and only affects that exceptional path. PerformanceRisk: none The change only restructures exception handling and introduces no additional allocation, iteration, retained state, or asynchronous work. Backwards compatibilityRisk: none The changed path does not read, write, migrate, or alter any stored messages, configuration, or persisted data format. Breaking changesRisk: none Public APIs, types, exports, accepted inputs, output shapes, defaults, and configuration remain unchanged; the Promise request methods now correctly expose an exception originating from the caller's own callback. ArchitectureRisk: none The fix remains localized within AbstractChat's existing request lifecycle and introduces no dependencies, cross-package source imports, shared abstractions, or package-boundary violations. Change scopeStatus: minimal The production exception-handling change, focused regression test, and required patch changeset are all directly necessary to fix and release the reported behavior. SecurityRisk: none The change adds no input processing, network access, credential handling, URL handling, serialization, or privilege boundary. TestingStatus: appropriate The regression test verifies rejection with the exact callback error and active-response cleanup; the shared request implementation covers send, regenerate, and resume paths, and the original public-API reproduction also passes. VerificationInspected the complete three-file merge-base diff and surrounding request, cleanup, public type, framework-wrapper, and concurrency code. The focused Chat test suite passed in both Node and Edge configurations with 35 tests each, package type checking passed, formatting and lint checks passed, and the diff passed whitespace validation. |
|
🚀 Published in:
|
2 participants
Background
Public AbstractChat requests resolved successfully when onFinish threw, preventing callers from detecting callback failures.
Root Cause
AbstractChat wrapped onFinish in a try/catch that logged callback exceptions instead of propagating them; the reproduction confirmed sendMessage resolved after the exact callback error was thrown.
Summary
Allowed onFinish errors to propagate while preserving active-response and resume-request cleanup, added a patch changeset, and removed reproduction-only artifacts.
Testing
Added a regression test verifying sendMessage rejects with the exact onFinish error and still clears the active response.
End-to-end Validation
pnpm -C examples/ai-functions exec tsx src/reproduction/issue-12175-on-finish-error-swallowed.tsexited successfully and confirmed sendMessage rejected with the exact callback error without logging it.Related Issues
Fixes #12175
Closes #18750