handlerFactory calls res.send() breaking controllers wanting to stream to the response body

[remojansen]

Moved here from https://github.com/inversify/inversify-express-utils/issues/155

---

i've got this endpoint:

  @httpGet('/')
  async getTheThing(@response() res: Response) {
    try {
      const fileStream = await this.storage.getTheThing();
      res.setHeader('content-type', 'application/tar');
      fileStream.pipe(res);
    } catch (error) {
      return res.status(404).send(error);
    }

This fails with:

Error: write after end
    at write_ (_http_outgoing.js:631:15)
    at ServerResponse.write (_http_outgoing.js:626:10)
    at IncomingMessage.ondata (_stream_readable.js:646:20)
    at IncomingMessage.emit (events.js:160:13)
    at IncomingMessage.Readable.read (_stream_readable.js:482:10)
    at flow (_stream_readable.js:853:34)
    at resume_ (_stream_readable.js:835:3)
    at process._tickCallback (internal/process/next_tick.js:152:19)

Because the handlerFactory at this line calls res.send() which closes the request while the stream is still being written.

I think if a controller uses @response() to get access to the response object then the handler factory shouldn't touch the response object at all.

---

Here's a workaround incase others find this post:

  @httpGet('/')
  async getTheThing(@response() res: Response) {
    try {
      const fileStream = await this.storage.getTheThing();
      res.setHeader('content-type', 'application/tar');
      fileStream.pipe(res);

      # we want to wait for the stream to be consumed so that
      # that when inversify-express-utils calls `res.send()` it doesn't
      # crash a still-in-progress stream pipe.
      await streamFinished(fileStream);
    } catch (error) {
      return res.status(404).send(error);
    }
function streamFinished(stream: NodeJS.ReadableStream) {
  return new Promise((resolve, reject) => {
    stream.on('end', () => resolve());
    stream.on('error', () => reject());
  });
}

[korziee]

Has anyone since fixed this or implemented a better solution?

[RemyJeancolas]

@korziee according to this line, another workaround could be to return a function that would be in charge of piping the stream to the response, something like:

@httpGet('/')
async getTheThing(@response() res: Response) {
  try {
    const fileStream = await this.storage.getTheThing();
    res.setHeader('content-type', 'application/tar');

    // Return a function to avoid the unnecessary call to `res.send` from inversify
    return () => fileStream.pipe(res);
  } catch (error) {
    return res.status(404).send(error);
  }
}

Tested on my end and it works fine.

[korziee]

Hi @RemyJeancolas, thanks for that, I do think that's a much cleaner solution than what I had done.

For reference, I noticed in the handlerFactory function that it checks to see if any headers have been sent, so my thought was to just write something to the return stream and let express set the headers. So I just added a res.write("") before I piped my readable stream to the express response object.

But I much prefer your solution, thanks for the response!

[bhupesh-sf]

I can see the PR is merged for streaming but its still not published on npm, any reason why?