diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index 8d40113200..ede27b20be 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -9,6 +9,7 @@ fs = require("fs") knox = require("knox") path = require("path") LocalFileWriter = require("./LocalFileWriter") +Errors = require("./Errors") _ = require("underscore") thirtySeconds = 30 * 1000 @@ -73,6 +74,12 @@ module.exports = s3Stream = s3Client.get(key, headers) s3Stream.end() s3Stream.on 'response', (res) -> + if res.statusCode == 404 + logger.log bucketName:bucketName, key:key, "file not found in s3" + return callback new Errors.NotFoundError("File not found in S3: #{bucketName}:#{key}"), null + if res.stausCode != 200 + logger.log bucketName:bucketName, key:key, "error getting file from s3" + return callback new Error("Got non-200 response from S3: #{res.statusCode}"), null callback null, res s3Stream.on 'error', (err) -> logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3" @@ -130,4 +137,3 @@ module.exports = exists = res.statusCode == 200 logger.log bucketName:bucketName, key:key, exists:exists, "checked if file exsists in s3" callback(err, exists) - diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee index ca324ae212..78fec6cea5 100644 --- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee +++ b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee @@ -35,6 +35,8 @@ describe "S3PersistorManagerTests", -> "logger-sharelatex": log:-> err:-> + "./Errors": @Errors = + NotFoundError: sinon.stub() @key = "my/key" @bucketName = "my-bucket" @error = "my errror" @@ -67,8 +69,51 @@ describe "S3PersistorManagerTests", -> @stubbedKnoxClient.get.calledWith(@key, {'Range': 'bytes=0-8'}).should.equal true done() + describe "error conditions", -> + beforeEach -> + @fakeResponse = + statusCode: 500 + @stubbedKnoxClient.get.returns( + on: (key, callback) => + if key == 'response' + callback(@fakeResponse) + end: -> + ) + describe "when the file doesn't exist", -> + + beforeEach -> + @fakeResponse = + statusCode: 404 + + it "should produce a NotFoundError", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + expect(stream).to.equal null + expect(err).to.not.equal null + expect(err instanceof @Errors.NotFoundError).to.equal true + done() + + it "should have bucket and key in the Error message", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + error_message = @Errors.NotFoundError.lastCall.args[0] + expect(error_message).to.not.equal null + error_message.should.match(new RegExp(".*#{@bucketName}.*")) + error_message.should.match(new RegExp(".*#{@key}.*")) + done() + + describe "when the S3 service produces an error", -> + beforeEach -> + @fakeResponse = + statusCode: 500 + + it "should produce an error", (done) -> + @S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback + expect(stream).to.equal null + expect(err).to.not.equal null + expect(err instanceof Error).to.equal true + @Errors.NotFoundError.called.should.equal false + done() describe "sendFile", ->