Question: Is pump compatible with the request library?

[JAndritsch]

Greetings!

I'm trying to learn how to better manage stream-related errors in a web application. I'm handling file uploads to a web server using the request library (https://www.npmjs.com/package/request).

Example:

const readStream = fs.createReadStream('path/to/file.jpg');
const writeStream = request.post('http://localhost/uploads');
readStream.pipe(writeStream);

For the most part, this works. However, error handling is a little tricky and I suspect I'm not handling errors and closing all streams properly. What I want to do is make sure that any issues with either stream result in both being closed and cleaned up properly, to prevent leaking file descriptors.

I'm trying to use pump to help with that, as it seems like it should take care of that scenario for me. However, I observe something unexpected when executing the following code:

const readStream = fs.createReadStream('path/to/file.jpg');
const writeStream = request.post('http://localhost/uploads');
writeStream.on('end', () => console.log('the request stream has ended'));
pump(readStream, writeStream, (err) => console.log('PUMP has finished'));

When I run this example, I see "PUMP has finished" appear long before "the request stream has ended". This would indicate that pump thinks the stream is done before we get back an http response.

This poses a problem for me, since I would be relying on pump to tell me when everything is finished before knowing when this asynchronous task is completed.

Is this expected? Am I doing something wrong?

Versions:

pump: 3.0.0
request: 2.81.0
node: 8.9.3

[mafintosh]

Pump finishes when the writable part of the last stream in the pipeline finishes. That means when your request stream above emits “finish”, not “end” as end indicates the ending of the readable part.

[JAndritsch]

Hey @mafintosh, thanks for the quick response!

The thing is, I'm not actually seeing the request stream emit finish. This Github issue may point to the reason why: request/request#2905

[mafintosh]

Yea, thats a request bug then

[JAndritsch]

I agree. This begs another question, however: if finish is never emitted, how does pump know when the write stream is done and should execute the final callback? Shouldn't the situation be that my callback never fires?

[mafintosh]

It has some request only code that hooks up to stream.req.on(finish) due to request being weird

[JAndritsch]

Ok. I see that in the end-of-stream library. Could it be the onlegacyfinish that's doing it?

var onlegacyfinish = function() {
  if (!stream.writable) onfinish();
};

Because if the request library isn't properly emitting the "finish" event, then this would have to be the only way to tell when the stream is no longer writable.