Merge pull request #489 from sharelatex/bg-improve-upload-robustness

improve stream error handling on upload

services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee

@@ -20,30 +20,33 @@ module.exports = FileStoreHandler =
 				logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "tried to upload symlink, not contining"
 				return callback(new Error("can not upload symlink"))
 
+			_cb = callback
+			callback = (err) ->
+				callback = ->	# avoid double callbacks
+				_cb(err)
+
 			logger.log project_id:project_id, file_id:file_id, fsPath:fsPath, "uploading file from disk"
 			readStream = fs.createReadStream(fsPath)
-			opts =
-				method: "post"
-				uri: FileStoreHandler._buildUrl(project_id, file_id)
-				timeout:fiveMinsInMs
-			writeStream = request(opts)
-			readStream.pipe writeStream
-
-			writeStream.on 'response', (response) ->
-				if response.statusCode not in [200, 201]
-					err = new Error("non-ok response from filestore for upload: #{response.statusCode}")
-					logger.err {err, statusCode: response.statusCode}, "error uploading to filestore"
-					callback(err)
-				else
-					callback(null)
-
 			readStream.on "error", (err)->
 				logger.err err:err, project_id:project_id, file_id:file_id, fsPath:fsPath, "something went wrong on the read stream of uploadFileFromDisk"
 				callback err
-
-			writeStream.on "error", (err)->
-				logger.err err:err, project_id:project_id, file_id:file_id, fsPath:fsPath, "something went wrong on the write stream of uploadFileFromDisk"
-				callback err
+			readStream.on "open", () ->
+				opts =
+					method: "post"
+					uri: FileStoreHandler._buildUrl(project_id, file_id)
+					timeout:fiveMinsInMs
+				writeStream = request(opts)
+				writeStream.on "error", (err)->
+					logger.err err:err, project_id:project_id, file_id:file_id, fsPath:fsPath, "something went wrong on the write stream of uploadFileFromDisk"
+					callback err
+				writeStream.on 'response', (response) ->
+					if response.statusCode not in [200, 201]
+						err = new Error("non-ok response from filestore for upload: #{response.statusCode}")
+						logger.err {err, statusCode: response.statusCode}, "error uploading to filestore"
+						callback(err)
+					else
+						callback(null)
+				readStream.pipe writeStream
 
 	getFileStream: (project_id, file_id, query, callback)->
 		logger.log project_id:project_id, file_id:file_id, query:query, "getting file stream from file store"

services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee

@@ -40,8 +40,8 @@ describe "FileStoreHandler", ->
 		it "should create read stream", (done)->
 			@fs.createReadStream.returns 
 				pipe:->
-				on: (type, cb)-> 
-					if type == "end"
+				on: (type, cb)->
+					if type == "open"
 						cb()
 			@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, =>
 				@fs.createReadStream.calledWith(@fsPath).should.equal true
@@ -51,7 +51,7 @@ describe "FileStoreHandler", ->
 			@request.returns(@writeStream)
 			@fs.createReadStream.returns 
 				on: (type, cb)-> 
-					if type == "end"
+					if type == "open"
 						cb()
 				pipe:(o)=>
 					@writeStream.should.equal o
@@ -62,7 +62,7 @@ describe "FileStoreHandler", ->
 			@fs.createReadStream.returns 
 				pipe:->
 				on: (type, cb)-> 
-					if type == "end"
+					if type == "open"
 						cb()
 			@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, =>
 				@request.args[0][0].method.should.equal "post"
@@ -73,7 +73,7 @@ describe "FileStoreHandler", ->
 			@fs.createReadStream.returns 
 				pipe:->
 				on: (type, cb)-> 
-					if type == "end"
+					if type == "open"
 						cb()
 			@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, =>
 				@handler._buildUrl.calledWith(@project_id, @file_id).should.equal true
@@ -83,7 +83,7 @@ describe "FileStoreHandler", ->
 			@fs.createReadStream.returns
 				pipe:->
 				on: (type, cb)->
-					if type == "end"
+					if type == "open"
 						cb()
 			@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, (err) =>
 				expect(err).to.not.exist
@@ -113,7 +113,7 @@ describe "FileStoreHandler", ->
 				@fs.createReadStream.returns
 					pipe:->
 					on: (type, cb)->
-						if type == "end"
+						if type == "open"
 							cb()
 				@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, (err) =>
 					expect(err).to.exist