Skip to content

fix: handle streamed response failure - #1861

Open
bigujun wants to merge 1 commit into
lukeautry:masterfrom
bigujun:fix/stream-pipeline
Open

fix: handle streamed response failure#1861
bigujun wants to merge 1 commit into
lukeautry:masterfrom
bigujun:fix/stream-pipeline

Conversation

@bigujun

@bigujun bigujun commented May 27, 2026

Copy link
Copy Markdown

Node process crashes with Uncaught Error when a readable stream is destroyed with an error when using express.

Reproduce steps:

@Get('/stream-with-error')
async getStream() {
  return new Readable({
    read() {
      this.destroy(new Error());
    }
  });
}

One workaround is to watch for the error event and manually close the underlying connection.

@Get('/stream-with-error')
async getStream(@Request() req: express.Request) {
  const readable = new Readable({
    read() {
      this.destroy(new Error());
    }
  });
  // listening to error prevents `Uncaught Error`;
  readable.on('error', () => {  
    // close the connection, so the response will not hang indefinitely on the client side;
    req.destroy();
  });
  return readable;
}

The problem is that readable.pipe(...) does not properly handle errors, so replacing it with pipeline solves the problem, since pipeline will properly clean up.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you written unit tests?
  • Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
  • This PR is associated with an existing issue?

Closing issues

closes #1664

Test plan

Included an express integration test that fails when the patch is not applied.
Test case simulates a readable that will be destroyed with an error and expects the connection to be aborted.
Without the fix applied, the process will throw an Uncaught Error and test will fail.
With the fix applied, the connection will be properly aborted and test will pass.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hello there bigujun 👋

Thank you and congrats 🎉 for opening your first PR on this project.✨

We will review the following PR soon! 👀

iffa added a commit to iffa/tsoa that referenced this pull request Aug 24, 2026
readable.pipe(response) leaves the stream's error event unhandled, so a
Readable destroyed with an error took the whole node process down.
pipeline handles the error and destroys the response, which aborts the
request. The error also goes to express's next() so an application can
log it; a client that hangs up mid-stream is not reported.

A 500 is not reachable here - pipeline destroys the response before its
callback runs - so the request aborts instead.

Upstream lukeautry#1861 discards the error.
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.

Returning a stream of non-existent file shuts down Node

1 participant