s3-upload-stream
s3-upload-stream copied to clipboard
nathanpeck
Sending a callback to .end() stops upload
If you have a upload object like in the documentation example, you can write some data like this:
upload.write("Hello World")
upload.end()
After that, you will receive a series of events, like uploaded to signal your upload is complete.
It seems though that if you set a callback in the .end() method (as you would do with other streams to be signalled when the data has been flushed), 1. you get signalled immediately and 2. no further events are triggered and no data is uploaded. For example:
upload.write("Hello World")
upload.end(function() {})
This is a harshly confusing behavior, if node streams are not confusing enough by themselves.
Good catch. This is because I'm using the .end() method as a hook for finalizing the MPU on S3 and closing everything out. I'll have to make some modifications to allow this use of .end() as a callback to detect the flush.
Adding extra note for my own reference when I fix this:
http://elegantcode.com/2013/10/14/detecting-the-end-of-a-rainbow-inside-a-writable-stream/
https://github.com/joyent/node/issues/7348#issuecomment-68846489
The recommended method would be to bind a function to the finish method of the inbound stream instead of overriding the writable stream's .end() method as I'm currently doing.
I had the same problem in my project using writable streams. Luckily @TomFrost has made a Writable subclass adding a _flush method that is triggered after the .end() call but before the consumer gets the finish event. Furthermore, _flush receives a callback, so you can delay the finish event (and consumer's callback being invoked) until you call it.
https://www.npmjs.com/package/flushwritable
Awesome... Thanks for the suggestion. I should have a fix using flushwritable within a couple days at most and a new version published on NPM.
Okay I have branch 1.1.0 now with a fix for this issue. You are welcome to try it out in advance. I need to test a bit more before publishing on NPM.
Hey Nathan, nice update.
One of the benefits of FlushWritable is that it delays the WriteStream's finish event until the _flush call has completed its work (by firing it when _flush's callback is called). With your implementation, the callback is called immediately when _flush is executed -- so unless I'm reading this wrong, there's currently no way to detect when all the data has completed uploading. It might be worth it to put that call inside of a callback that indicates completion.
Nice critique. That makes a lot of sense, and I'll definitely make that change before releasing this. Right now you have to listen for the "uploaded" event to know when the upload has been committed on S3.