return a 404 error (instead of a 500) when copying a missing file

This commit is contained in:
Brian Gough 2019-01-09 10:31:59 +00:00
parent 04176d479b
commit 9d93eee3e8
4 changed files with 29 additions and 4 deletions

View file

@ -60,8 +60,11 @@ module.exports = FileController =
logger.log key:key, bucket:bucket, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "reciving request to copy file" logger.log key:key, bucket:bucket, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "reciving request to copy file"
PersistorManager.copyFile bucket, "#{oldProject_id}/#{oldFile_id}", key, (err)-> PersistorManager.copyFile bucket, "#{oldProject_id}/#{oldFile_id}", key, (err)->
if err? if err?
logger.log err:err, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "something went wrong copying file" if err instanceof Errors.NotFoundError
res.send 500 res.send 404
else
logger.log err:err, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "something went wrong copying file"
res.send 500
else else
res.send 200 res.send 200

View file

@ -102,8 +102,14 @@ module.exports =
# use the AWS SDK instead of knox due to problems with error handling (https://github.com/Automattic/knox/issues/114) # use the AWS SDK instead of knox due to problems with error handling (https://github.com/Automattic/knox/issues/114)
s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) -> s3.copyObject {Bucket: bucketName, Key: destKey, CopySource: source}, (err) ->
if err? if err?
logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in aws" if err.code is 'NoSuchKey'
callback(err) logger.err bucketName:bucketName, sourceKey:sourceKey, "original file not found in s3 when copying"
callback(new Errors.NotFoundError("original file not found in S3 when copying"))
else
logger.err err:err, bucketName:bucketName, sourceKey:sourceKey, destKey:destKey, "something went wrong copying file in aws"
callback(err)
else
callback()
deleteFile: (bucketName, key, callback)-> deleteFile: (bucketName, key, callback)->
logger.log bucketName:bucketName, key:key, "delete file in s3" logger.log bucketName:bucketName, key:key, "delete file in s3"

View file

@ -28,6 +28,8 @@ describe "FileController", ->
"./LocalFileWriter":@LocalFileWriter "./LocalFileWriter":@LocalFileWriter
"./FileHandler": @FileHandler "./FileHandler": @FileHandler
"./PersistorManager":@PersistorManager "./PersistorManager":@PersistorManager
"./Errors": @Errors =
NotFoundError: sinon.stub()
"settings-sharelatex": @settings "settings-sharelatex": @settings
"logger-sharelatex": "logger-sharelatex":
log:-> log:->
@ -111,6 +113,13 @@ describe "FileController", ->
done() done()
@controller.copyFile @req, @res @controller.copyFile @req, @res
it "should send a 404 if the original file was not found", (done) ->
@PersistorManager.copyFile.callsArgWith(3, new @Errors.NotFoundError())
@res.send = (code)=>
code.should.equal 404
done()
@controller.copyFile @req, @res
it "should send a 500 if there was an error", (done)-> it "should send a 500 if there was an error", (done)->
@PersistorManager.copyFile.callsArgWith(3, "error") @PersistorManager.copyFile.callsArgWith(3, "error")
@res.send = (code)=> @res.send = (code)=>

View file

@ -218,6 +218,13 @@ describe "S3PersistorManagerTests", ->
@stubbedS3Client.copyObject.calledWith({Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key}).should.equal true @stubbedS3Client.copyObject.calledWith({Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key}).should.equal true
done() done()
it "should return a NotFoundError object if the original file does not exist", (done)->
NoSuchKeyError = {code: "NoSuchKey"}
@stubbedS3Client.copyObject.callsArgWith(1, NoSuchKeyError)
@S3PersistorManager.copyFile @bucketName, @sourceKey, @destKey, (err)=>
expect(err instanceof @Errors.NotFoundError).to.equal true
done()
describe "deleteDirectory", -> describe "deleteDirectory", ->
beforeEach -> beforeEach ->