Browse Source

When upload to filestore fails, produce an error

Shane Kilkelly 9 years ago
parent
commit
ec25ee9045

+ 11 - 2
services/web/app/coffee/Features/FileStore/FileStoreHandler.coffee

@@ -25,10 +25,19 @@ module.exports = FileStoreHandler =
 				timeout:fiveMinsInMs
 				timeout:fiveMinsInMs
 			writeStream = request(opts)
 			writeStream = request(opts)
 			readStream.pipe writeStream
 			readStream.pipe writeStream
-			writeStream.on "end", callback
+
+			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)->
 			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"
 				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
 				callback err
+
 			writeStream.on "error", (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"
 				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
 				callback err
@@ -79,4 +88,4 @@ module.exports = FileStoreHandler =
 			callback(err)
 			callback(err)
 
 
 	_buildUrl: (project_id, file_id)->
 	_buildUrl: (project_id, file_id)->
-		return "#{settings.apis.filestore.url}/project/#{project_id}/file/#{file_id}"
+		return "#{settings.apis.filestore.url}/project/#{project_id}/file/#{file_id}"

+ 29 - 2
services/web/test/UnitTests/coffee/FileStore/FileStoreHandlerTests.coffee

@@ -17,8 +17,8 @@ describe "FileStoreHandler", ->
 		@writeStream =
 		@writeStream =
 			my:"writeStream"
 			my:"writeStream"
 			on: (type, cb)-> 
 			on: (type, cb)-> 
-				if type == "end"
-					cb()
+				if type == "response"
+					cb({statusCode: 200})
 		@readStream = {my:"readStream", on: sinon.stub()}
 		@readStream = {my:"readStream", on: sinon.stub()}
 		@request = sinon.stub()
 		@request = sinon.stub()
 		@settings = apis:{filestore:{url:"http//filestore.sharelatex.test"}}
 		@settings = apis:{filestore:{url:"http//filestore.sharelatex.test"}}
@@ -79,6 +79,16 @@ describe "FileStoreHandler", ->
 				@handler._buildUrl.calledWith(@project_id, @file_id).should.equal true
 				@handler._buildUrl.calledWith(@project_id, @file_id).should.equal true
 				done()
 				done()
 
 
+		it 'should callback with null', (done) ->
+			@fs.createReadStream.returns
+				pipe:->
+				on: (type, cb)->
+					if type == "end"
+						cb()
+			@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, (err) =>
+				expect(err).to.not.exist
+				done()
+
 		describe "symlink", ->
 		describe "symlink", ->
 			it "should not read file if it is symlink", (done)->
 			it "should not read file if it is symlink", (done)->
 				@isSafeOnFileSystem = false
 				@isSafeOnFileSystem = false
@@ -86,6 +96,23 @@ describe "FileStoreHandler", ->
 					@fs.createReadStream.called.should.equal false
 					@fs.createReadStream.called.should.equal false
 					done()
 					done()
 
 
+		describe "when upload fails", ->
+			beforeEach ->
+				@writeStream.on = (type, cb) ->
+					if type == "response"
+						cb({statusCode: 500})
+
+			it 'should callback with an error', (done) ->
+				@fs.createReadStream.returns
+					pipe:->
+					on: (type, cb)->
+						if type == "end"
+							cb()
+				@handler.uploadFileFromDisk @project_id, @file_id, @fsPath, (err) =>
+					expect(err).to.exist
+					expect(err).to.be.instanceof Error
+					done()
+
 	describe "deleteFile", ->
 	describe "deleteFile", ->
 
 
 		it "should send a delete request to filestore api", (done)->
 		it "should send a delete request to filestore api", (done)->