From 69b164092cdccb469052464ea1f3c3697a06d54d Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 9 Nov 2018 14:04:26 +0000 Subject: [PATCH 1/2] suppress unnecessary error logging for NotFound --- services/filestore/app/coffee/FileController.coffee | 2 +- services/filestore/app/coffee/FileHandler.coffee | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/services/filestore/app/coffee/FileController.coffee b/services/filestore/app/coffee/FileController.coffee index 27ac078379..24fd5229de 100644 --- a/services/filestore/app/coffee/FileController.coffee +++ b/services/filestore/app/coffee/FileController.coffee @@ -29,10 +29,10 @@ module.exports = FileController = logger.log start: range.start, end: range.end, "getting range of bytes from file" FileHandler.getFile bucket, key, options, (err, fileStream)-> if err? - logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file" if err instanceof Errors.NotFoundError return res.send 404 else + logger.err err:err, key:key, bucket:bucket, format:format, style:style, "problem getting file" return res.send 500 else if req.query.cacheWarm logger.log key:key, bucket:bucket, format:format, style:style, "request is only for cache warm so not sending stream" diff --git a/services/filestore/app/coffee/FileHandler.coffee b/services/filestore/app/coffee/FileHandler.coffee index 2eeb09bc74..8fc7cd037a 100644 --- a/services/filestore/app/coffee/FileHandler.coffee +++ b/services/filestore/app/coffee/FileHandler.coffee @@ -6,6 +6,7 @@ FileConverter = require("./FileConverter") KeyBuilder = require("./KeyBuilder") async = require("async") ImageOptimiser = require("./ImageOptimiser") +Errors = require('./Errors') module.exports = FileHandler = @@ -32,7 +33,7 @@ module.exports = FileHandler = _getStandardFile: (bucket, key, opts, callback)-> PersistorManager.getFileStream bucket, key, opts, (err, fileStream)-> - if err? + if err? and !(err instanceof Errors.NotFoundError) logger.err bucket:bucket, key:key, opts:FileHandler._scrubSecrets(opts), "error getting fileStream" callback err, fileStream From 23a6d6e81d3cfacc8cb1707b00527368bdc4dbe7 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 9 Nov 2018 14:05:07 +0000 Subject: [PATCH 2/2] consider 403 and 404 as NotFound errors --- services/filestore/app/coffee/S3PersistorManager.coffee | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index 2bd6eb0e9b..0bb713deb0 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/services/filestore/app/coffee/S3PersistorManager.coffee @@ -74,7 +74,9 @@ module.exports = s3Stream = s3Client.get(key, headers) s3Stream.end() s3Stream.on 'response', (res) -> - if res.statusCode == 404 + if res.statusCode in [403, 404] + # S3 returns a 403 instead of a 404 when the user doesn't have + # permission to list the bucket contents. 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.statusCode not in [200, 206]