streaming requests interact badly with `pipeline(..)`

[djmitche]

Here's a minimal reproduction, tested with node 14 and 15.

// test.js
const request = require('superagent');
const fs = require('fs');
const util = require('util');
const pipeline = util.promisify(require('stream').pipeline);

const main = async () => {
  const stream = fs.createReadStream('test.js');
  const req = request.put('http://httpstat.us/200');
  console.log('here');
  await pipeline(stream, req);
  console.log('there');
};

main().catch(err => console.log('err', err)).then(() => console.log('done'));

The PUT appears (from some tcpdumping and watching logging with DEBUG=*) to complete successfully, but there and done are never printed -- execution of main appears to just cease and node exits normally, suggesting that something, somewhere, isn't calling a callback. Maybe

Replacing await pipeline(stream, req) with

    await new Promise((res, rej) => stream.pipe(req).on('end', res).on('error', rej));

seems to work, but it doesn't catch an error when the /200 is changed to /400 a few lines higher (probably end is emitted first?).

I think the issue is that, as a writable stream, req should be emitting finish and not end, which is for readable streams.

• https://nodejs.org/api/stream.html#stream_event_finish
• https://nodejs.org/api/stream.html#stream_event_end

[djmitche]

I can confirm that changing https://github.com/visionmedia/superagent/blob/1277a880c32191e300b229e352e0633e421046c8/src/node/index.js#L1128-L1131 to emit finish instead fixes this issue.

[djmitche]

..half-fixes the issue. Errors (such as a 500 response) are not propagated, even with that change.