Skip to content

StreamableFile pipe can leak resources if response is closed/errored prematurely #9759

Closed
@alexd6631

Description

@alexd6631

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

return body.getStream().pipe(response);

When returning a StreamableFile from a controller, it seems the express adapter will .pipe() it to the express response stream.
As you may know pipe method is somewhat deprecated because it does not properly closes streams in case of abnormal termination, newer code should prefer use pipeline which automatically closes source stream on destination stream error.

So if a client cancels a request while it is streaming a StreamableFile, the stream wrapped by the StreamableFile will be kept forever opened, leading to a potential ressource leak.

Minimum reproduction code

WIP (see below)

Steps to reproduce

I am noticing the issue on a production app I am currently working on and I can't publish the code source without some adaptation effort to create the minimum reproduction code.

In my setup I have an endpoint that streams images store on S3 using the official AWS S3 client and StreamableFile on the controller side. The S3 client use keep alive and a pool of sockets, so if a GetObject stream is not fully consumed it will leak the socket and prevent it to return it to the pool.

I have a front that displays these images, and If I refresh the page several times quickly, it will cancel some in-flights requests on the server, but due to the .pipe behavior, some S3 Stream will not be destroyed. When the whole client pool is "poisoned", all S3 call will now hang forever, which is quite serious condition, only solved by restarting the nest server.

I could work on a minimum reproduction code, but the above setup is already quite involved.
Let me know if there is enough information, or how "minimal" the reproduction code should be in my case.

Expected behavior

StreamableFile should destroy the underlying stream not only on full consumption but also in case of error / early abortion of the consumer stream. This should be achievable easily by replacing .pipe() method by the pipeline function

Package

  • I don't know. Or some 3rd-party package
  • @nestjs/common
  • @nestjs/core
  • @nestjs/microservices
  • @nestjs/platform-express
  • @nestjs/platform-fastify
  • @nestjs/platform-socket.io
  • @nestjs/platform-ws
  • @nestjs/testing
  • @nestjs/websockets
  • Other (see below)

Other package

No response

NestJS version

8.4.5

Packages versions

Node.js version

16.15.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs triageThis issue has not been looked into

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions