Propagate 'response' event through file.createReadStream()

[robertdimarco]

_Background_:
We run a proxy service that serves a large number of files of varying size to downstream clients. In cases where we encounter an error serving the requested file (such as 404s) we intercept the error to the downstream client and pass through a custom error and template. See #522 for an oversimplified version.

_Request_:
The previous request to remove the required call to file.getMetadata() before invoking file.createReadStream() has been a big help in reducing mean latency since it removed a roundtrip for each download request. Our next challenge has been intercepting errors (namely those 404s) in a way that continues to let us simply pipe the result to the downstream client (and not buffer the download file until completion), and send the appropriate status code + headers.

Currently, in the case of a 404, the stream returned from file.createReadStream() will first emit a data event (an XML response containing an error) and then emits an error event (the formatted error object). Ideally, we could inspect the response headers before deciding whether or not to stream the result from GCS down to the client, and currently we have to buffer some number of data event payloads until we're sure we haven't encountered a GCS error.

_Details_:
The Node http.ClientRequest class specifies a response event that fires once and exposes (amongst other things) the HTTP response headers. If we could subscribe to that event, it would allow us to inspect the response from GCS, send the appropriate status code and headers to our downstream client, prior to piping any data.

diff --git a/lib/storage/file.js b/lib/storage/file.js
index d64aa09..137baba 100644
--- a/lib/storage/file.js
+++ b/lib/storage/file.js
@@ -464,6 +464,8 @@ File.prototype.createReadStream = function(options) {
         request(authorizedReqOpts)
           .on('error', done)

+          .on('response', throughStream.emit.bind(throughStream, 'response'))
+
           .on('data', function(chunk) {
             if (crc32c) {
               localCrcHash = crc.calculate(chunk, localCrcHash);

Don't hesitate to let me know if you have any thoughts or questions, and thanks for all of your hard work on this library!

[ryanseys]

Interesting use case. I don't see why we couldn't support this. I'm all for exposing this functionality if it helps our users :) @stephenplusplus what do you say?

[ryanseys]

@robertdimarco I'm going to make an executive decision here and say "why not!". Could you create a PR for this change with a unit test or two and assign me to review when you're ready? Thanks!

[robertdimarco]

@ryanseys Sure thing, I'll follow-up with that shortly.

[stephenplusplus]

👍

[stephenplusplus]

Thanks, @robertdimarco!