Storage streams use 'complete' event for end of stream but built-in streams use 'finish'

[ryanseys]

As was raised in #340, I looked into why there was a discrepancy between what the developer thought and what was the real case. There was a suggestion made to update our docs, but our docs aren't the issue here. A snippet from the tests shows the issue:

Using 'finish' event:

file.createReadStream()
.pipe(fs.createWriteStream(tmpFilePath))
.on('error', done)
.on('finish', function() {
  file.delete(function(err) {
    assert.ifError(err);

    fs.readFile(tmpFilePath, function(err, data) {
      assert.equal(data, fileContent);
      done();
    });
  });
});

Using 'complete' event:

var file = bucket.file(filenames[0]);
fs.createReadStream(files.logo.path)
  .pipe(file.createWriteStream())
  .on('error', done)
  .on('complete', function() {
    file.copy(filenames[1], function(err, copiedFile) {
      assert.ifError(err);
      copiedFile.copy(filenames[2], done);
    });
  });

Seems the only difference is the type of file that is getting piped to. In the first case, it's a regular stream from fs and in the second it's our implementation of the storage file write stream.

So my question is, should we use a consistent finish event everywhere or is this by-design or otherwise okay?

[stephenplusplus]

Our complete event comes from request: https://github.com/request/request/blob/e33a883dd412dc7a1fdd1e1f282e840faa540609/request.js#L1224

Being consistent with fs would be nice, but off hand, I'm not sure we can do that.

[ryanseys]

Are you sure? I thought we intercept pretty much every event and emit our own events.

I'm looking at code here and here for example.

[stephenplusplus]

Ooh, I forgot about our new proxy stream. Yeah, we should be able to emit whatever we choose in these cases.

[stephenplusplus]

This is quite complex.

A native readable stream is done when it emits 'end', which is the same time the stream is .end()-ed. The end listener doesn't receive arguments, and if you pass arguments to stream.end(), they get written to the stream immediately prior to being ended. I wonder if this is why request uses complete, so they can do some post-processing before emitting the response headers/body/status arguments.

I'm okay with keeping complete as the consistent event for readable and writable end signals.

[ryanseys]

Oh yeah, good observation. That looks like exactly what they are doing. I'm happy with this too. It just makes it look weird when you have pipe with different streams, but in either case you have to make sure you're using the right event. complete as a consistent "yo, im done now" event is good for me.