Skip to content

Conversation

horochx
Copy link
Contributor

@horochx horochx commented Aug 28, 2025

Description

Add a try...catch block around writer.close() within handleStreamingMode to prevent failures.

Motivation

In streaming scenarios, if the user aborts the request, the process enters the first catch block due to a streaming write failure (Response object has been garbage collected). The finally block is then executed, which attempts to call writer.close(). This leads to a second error (WritableStream is closed) that is currently unhandled. On Node.js versions 15 and above, this unhandled rejection will cause the process to exit unexpectedly.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

Copy link
Contributor

Code Quality bug fix Reliability

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

Added try...catch blocks around writer.close() calls within the finally blocks of handleStreamingMode in src/handlers/streamHandler.ts for both streaming modes.

🔍 Impact of the Change

This change ensures that potential errors occurring during the stream writer's closure are gracefully handled and logged, preventing unhandled promise rejections and improving the overall robustness and stability of the streaming process.

📁 Total Files Changed

  • src/handlers/streamHandler.ts: Implemented error handling for stream writer closure.

🧪 Test Added

N/A

🔒Security Vulnerabilities

N/A

Motivation

To gracefully handle potential failures when closing a stream writer, preventing application crashes and improving error observability.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

.

Tip

Quality Recommendations

  1. Replace console.error with a structured logging mechanism (e.g., a dedicated logger instance) for better observability, centralized error management, and easier integration with monitoring systems.

Tanka Poem ♫

Stream's end, a soft sigh,
Yet sometimes, a harsh error.
Now, a gentle catch,
No more unhandled sorrow,
Code flows, robust and serene. ✨

Sequence Diagram

sequenceDiagram
    participant H as StreamHandler
    participant W as Writer
    participant E as Encoder

    H->>H: handleStreamingMode(response, writer, encoder, data)
    loop For each chunk in data
        H->>E: encode(chunk)
        E-->>H: encodedChunk
        H->>W: write(encodedChunk)
        alt Error during write
            W--xH: writeError
            H->>H: console.error('Error during stream processing:', error)
        end
    end
    H->>H: finally block
    activate H
    H->>+W: close()
    alt Close successful
        W-->>-H: closeSuccess
    else Close failed
        W--x-H: closeError
        H->>H: console.error('Failed to close the writer:', closeError)
    end
    deactivate H
Loading

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

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

Good error handling improvements for stream writer cleanup, but console.error should be replaced with proper logging for production code.

Copy link
Collaborator

@narengogi narengogi left a comment

Choose a reason for hiding this comment

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

lgtm

@VisargD VisargD merged commit faf88d8 into Portkey-AI:main Sep 3, 2025
1 check passed
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.

3 participants