Skip to content

Fix: Add error handler to streaming onFinish promise chain#1539

Open
kernel-systems-bot wants to merge 1 commit intobrowserbase:mainfrom
kernel-systems-bot:kernel-systems-bot/streaming-hang
Open

Fix: Add error handler to streaming onFinish promise chain#1539
kernel-systems-bot wants to merge 1 commit intobrowserbase:mainfrom
kernel-systems-bot:kernel-systems-bot/streaming-hang

Conversation

@kernel-systems-bot
Copy link
Contributor

@kernel-systems-bot kernel-systems-bot commented Jan 14, 2026

Fix: Streaming onFinish Promise Chain Missing Error Handler

Summary

Adds missing .catch() handler to the ensureClosed().then() promise chain in streaming mode's onFinish callback. Without this handler, if ensureClosed() fails, the streamResult.result promise hangs forever instead of rejecting with an error.

Problem

In v3AgentHandler.ts, the streaming mode onFinish callback calls ensureClosed() and chains .then() to resolve the result promise, but has no .catch() handler:

// BEFORE (bug):
this.ensureClosed(...).then((closeResult) => {
  resolveResult(result);
}); // <- NO .catch() - if ensureClosed throws, promise hangs forever

Solution

Add a .catch() handler that properly rejects the result promise:

// AFTER (fix):
this.ensureClosed(...)
  .then((closeResult) => {
    resolveResult(result);
  })
  .catch((err) => {
    rejectResult(err);
  });

Impact

  • Before fix: If ensureClosed() fails during streaming, streamResult.result hangs indefinitely, causing memory leaks and unrecoverable state
  • After fix: Errors are properly propagated, allowing callers to handle failures gracefully

Test Plan

  • Added regression test streaming-onfinish-error-handling.test.ts
  • Run npm test to verify all tests pass
  • Verify streaming agent execution still works correctly in happy path

Files Changed

  • packages/core/lib/v3/handlers/v3AgentHandler.ts - Added .catch() handler
  • packages/core/tests/streaming-onfinish-error-handling.test.ts - New regression test

Feedback? Email p0@kernel.dev


Summary by cubic

Fixes a hang in streaming results by adding an error handler to the onFinish ensureClosed() promise chain. Errors now reject streamResult.result instead of hanging, so callers can handle failures.

  • Bug Fixes
    • Added .catch((err) => rejectResult(err)) to the ensureClosed() chain in v3AgentHandler onFinish.
    • Added a regression test to confirm rejection on failure and resolution on success.

Written for commit e20e06e. Summary will update on new commits.

@changeset-bot
Copy link

changeset-bot bot commented Jan 14, 2026

⚠️ No Changeset found

Latest commit: e20e06e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 14, 2026

Greptile Summary

This PR fixes a bug where the streaming agent's result promise would hang indefinitely if ensureClosed() failed. The fix adds a .catch() handler to properly reject the promise with the error.

  • Added .catch((err) => rejectResult(err)) to the ensureClosed().then() promise chain in v3AgentHandler.ts
  • Added comprehensive regression tests covering both error and success paths
  • Aligns with existing error handling patterns already present in onError and onAbort callbacks

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it's a straightforward bug fix with proper test coverage.
  • The change is minimal and focused, adding only a .catch() handler to an existing promise chain. The fix follows the same pattern already used in onError and onAbort handlers. The regression test properly validates both error and success paths.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/core/lib/v3/handlers/v3AgentHandler.ts Added .catch() handler to the ensureClosed().then() promise chain in the streaming onFinish callback to properly propagate errors instead of leaving the result promise hanging indefinitely.
packages/core/tests/streaming-onfinish-error-handling.test.ts Added regression test that verifies the streamResult.result promise properly rejects when ensureClosed() fails, and resolves correctly in the success path.

Sequence Diagram

sequenceDiagram
    participant Client
    participant V3AgentHandler
    participant LLMClient
    participant ensureClosed

    Client->>V3AgentHandler: stream(options)
    V3AgentHandler->>LLMClient: streamText(options)
    LLMClient-->>V3AgentHandler: fullStream
    V3AgentHandler-->>Client: AgentStreamResult (with result promise)
    
    Note over LLMClient: Streaming completes
    LLMClient->>V3AgentHandler: onFinish callback
    V3AgentHandler->>ensureClosed: ensureClosed()
    
    alt ensureClosed succeeds
        ensureClosed-->>V3AgentHandler: closeResult
        V3AgentHandler->>V3AgentHandler: resolveResult(result)
        V3AgentHandler-->>Client: result promise resolves
    else ensureClosed fails (NEW FIX)
        ensureClosed-->>V3AgentHandler: Error
        V3AgentHandler->>V3AgentHandler: rejectResult(err)
        V3AgentHandler-->>Client: result promise rejects
    end
Loading

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant