Emit errors from upstream on file upload

[ryanseys]

I (think I) have a fix here for #422 but writing the test for it has been troublesome for me, most likely due to my lack of understanding with streams. Hopefully @stephenplusplus can help!

The test is passing (all asserts pass and done is called) and then immediately failing due to stream.end being called on a stream that (I guess) no longer exists?

I'm getting the following error when running the test included in this PR and don't know how to fix:

> mocha test/*

  File
    createWriteStream
      ✓ should re-emit errors
      1) should re-emit errors

  1 passing (23ms)
  1 failing

  1) File createWriteStream should re-emit errors:
     Uncaught TypeError: Cannot read property 'ending' of undefined
      at Duplexify.end (/Users/ryanseys/gcloud-node/node_modules/duplexify/index.js:218:27)
      at handleError (/Users/ryanseys/gcloud-node/lib/storage/file.js:1064:12)
      at DestroyableTransform.<anonymous> (/Users/ryanseys/gcloud-node/lib/storage/file.js:992:7)
      at DestroyableTransform.emit (events.js:129:20)
      at Immediate._onImmediate (/Users/ryanseys/gcloud-node/test/storage/file.js:553:18)
      at processImmediate [as _immediateCallback] (timers.js:358:17)

[stephenplusplus]

I believe handleError doesn't need to end the stream. Errors are emitted back to the stream that was passed into startResumableUpload_, which ends the stream there.

... streams of tears.

[ryanseys]

PTAL 😄

[stephenplusplus]

👍

[teddybearz]

Is it possible that handleError() being called twice? i.e. one error event and one complete event with error come back to back?

[ryanseys]

It was that the stream was being closed too many times. I believe we have addressed this issue fully. If you want to test against master, that would be wonderful!

[teddybearz]

I wonder in real network scenario (not just the test mock environment), it is possible that there will be one error event and one complete event+error being emitted back to back with error code all equals to 404 or 499-600. If it happens, the handleError() will be called twice and startUpload() resumeUpload() will be called twice.

[stephenplusplus]

The write stream will either emit an error or complete, it can't do both.

Regardless, somewhat related, we should aim to "once-ify" our user provided callbacks.

[stephenplusplus]

@teddybearz I can't help but feel I'm missing your point. If I am, thanks for your patience.

[teddybearz]

The write stream will either emit an error or complete, it can't do both.
In theory yes, but for event driven code, it takes some efforts to enforce and maintain that. Maybe better not assume too much and prepare for the worst.

BTW, is there a mock environment I can use to test my code that uses this library? So I can simulate bad-upload/bad-download/network-connection-reset those kinds of error cases programmatically? I am tired of pulling cable now 😄

[ryanseys]

@teddybearz You might have some luck trying out https://github.com/tylertreat/Comcast 😄