piped request end and close events seem not to be fired

[abissens]

Hi,

Piping request stream to busboy seems to prevent request end and close event from firing
Could this indicate some leak issues ?

Test code

test('request end/close events are fired', done => {
      let [end, close] = [false, false];
      http.createServer((req, res) => {
          req.on('end', () => end = true)
              .on('close', () => close = true)
              .pipe(busboy({headers: req.headers}))
              .on('close', () => res.writeHead(204, {'Connection': 'close'}).end());
      }).listen(8080);
  
      const req = http.request({
          host: 'localhost',
          port: 8080,
          method: 'POST',
          headers: {
              'Content-Type': 'multipart/form-data; boundary=--------------------------719223781919663014682861'
          }
      }, (res) => {
          expect(res.statusCode).toEqual(204);
          expect([close, end]).toEqual([true, true]);
          done();
      });
      Readable.from(body).pipe(req);
  })
  
  const body = ['----------------------------719223781919663014682861',
      'Content-Disposition: form-data; name="file"; filename="test_file"',
      'Content-Type: application/octet-stream',
      '',
      'File content',
      '----------------------------719223781919663014682861--'].join('\r\n')

I also noticed that when the busboy stream forced end is commented out the test passes

System
Node v16.14.0
Busboy 1.5.0

PS: this also keeps from using the promisified stream.pipeline with busboy since the resulting promise wouldn't resolve

[mscdex]

You're not reading the file data from the busboy instance. If you don't do that then the busboy stream will never end/close.

At the very minimum you need something like:

const bb = busboy({headers: req.headers});
bb.on('file', (fieldName, file) => {
  file.resume(); // discards the file data
});
// ...
req.pipe(bb);

[abissens]

I think that it resumes by default. But still even when explicitly resuming the test don't pass
May be the request reader stream should be explicitly resumed when forcing busboy end
I've tried this : abissens@322c4cf

[mscdex]

AFAIK there is no leak happening here. Once you respond to the request from the busboy 'close' handler, the request goes away.

[mscdex]

I'll remove it anyway, it just means that useless (or potentially useful depending on your use case) data can possibly pass through busboy. If that ever becomes an issue for someone, we can revisit it I guess.

[slewsys]

Perhaps this.end() should be conditioned on something (maybe type of input)?

I'm using async-busboy patched for busboy version 1.5 (see, e.g., @revoinc/async-busboy). npm run test completes successfully, but times out with busboy version 1.6.

[mscdex]

@slewsys Does it work if you add stream.end() in your tests? If so, I don't think that is asking too much 🤷‍♂️ . It's hard to please everyone.

[slewsys]

Good exercise :) I'll try to provide a solution. In the mean time, I'm able to successfully use @revoinc/async-busboy with busboy v1.6 in another package, the Koa Joi router. So it would appear that it is indeed the async-busboy testsuite that is at fault.

[slewsys]

Yes, adding stream.end() works, thank you!

In particular, the async-busboy test suite defines a request as:

import Stream from "stream"
function request () {
  const stream = new Stream.PassThrough()
  stream.headers = {
    'content-type': 'multipart/form-data; boundary=---------------------------paZqsnEHRufoShdX6fh0lUhXBP4k',
  }
  stream.write([
    '-----------------------------paZqsnEHRufoShdX6fh0lUhXBP4k',
    'Content-Disposition: form-data; name="file_name_0"',
    '',
    'super alpha file',
    '-----------------------------paZqsnEHRufoShdX6fh0lUhXBP4k',
    ...
    ].join('\r\n'))
  return stream
}
 

Inserting stream.end() after stream.write() and before return stream allows the tests to complete successfully with busboy v1.6.