🎨 Improved error handling for images on file storage which don't exist (#9282)

refs https://github.com/TryGhost/Team/issues/41

- differentiate error codes
- return 404 if image was not found
- else return a 500
- use i18n keys
- use errors.utils.isIgnitionError (!)
This commit is contained in:
Katharina Irrgang 2017-11-28 15:27:18 +01:00 committed by Kevin Ansfield
parent be9ce107bd
commit 9e388aee4d
5 changed files with 15 additions and 4 deletions

View File

@ -134,9 +134,16 @@ class LocalFileStore extends StorageBase {
return new Promise(function (resolve, reject) { return new Promise(function (resolve, reject) {
fs.readFile(targetPath, function (err, bytes) { fs.readFile(targetPath, function (err, bytes) {
if (err) { if (err) {
if (err.code === 'ENOENT') {
return reject(new errors.NotFoundError({
err: err,
message: i18n.t('errors.errors.imageNotFoundWithRef', {img: options.path})
}));
}
return reject(new errors.GhostError({ return reject(new errors.GhostError({
err: err, err: err,
message: 'Could not read image: ' + targetPath message: i18n.t('errors.errors.cannotReadImage', {img: options.path})
})); }));
} }

View File

@ -486,6 +486,8 @@
"renderingErrorPage": "Rendering Error Page", "renderingErrorPage": "Rendering Error Page",
"caughtProcessingError": "Ghost caught a processing error in the middleware layer.", "caughtProcessingError": "Ghost caught a processing error in the middleware layer.",
"imageNotFound": "Image not found", "imageNotFound": "Image not found",
"imageNotFoundWithRef": "Image not found: {img}",
"cannotReadImage": "Could not read image: {img}",
"pageNotFound": "Page not found", "pageNotFound": "Page not found",
"resourceNotFound": "Resource not found" "resourceNotFound": "Resource not found"
} }

View File

@ -202,9 +202,10 @@ getImageSizeFromFilePath = function getImageSizeFromFilePath(imagePath) {
} }
})); }));
}).catch(function (err) { }).catch(function (err) {
if (err instanceof errors.GhostError) { if (errors.utils.isIgnitionError(err)) {
return Promise.reject(err); return Promise.reject(err);
} }
return Promise.reject(new errors.InternalServerError({ return Promise.reject(new errors.InternalServerError({
message: err.message, message: err.message,
code: 'IMAGE_SIZE_STORAGE', code: 'IMAGE_SIZE_STORAGE',

View File

@ -175,7 +175,7 @@ describe('Local File System Storage', function () {
done(new Error('image should not exist')); done(new Error('image should not exist'));
}) })
.catch(function (err) { .catch(function (err) {
(err instanceof errors.GhostError).should.eql(true); (err instanceof errors.NotFoundError).should.eql(true);
err.code.should.eql('ENOENT'); err.code.should.eql('ENOENT');
done(); done();
}); });

View File

@ -4,6 +4,7 @@ var should = require('should'),
nock = require('nock'), nock = require('nock'),
configUtils = require('../../utils/configUtils'), configUtils = require('../../utils/configUtils'),
utils = require('../../../server/utils'), utils = require('../../../server/utils'),
errors = require('../../../server/errors'),
storage = require('../../../server/adapters/storage'), storage = require('../../../server/adapters/storage'),
path = require('path'), path = require('path'),
@ -463,7 +464,7 @@ describe('Image Size', function () {
result = imageSize.getImageSizeFromFilePath(url) result = imageSize.getImageSizeFromFilePath(url)
.catch(function (err) { .catch(function (err) {
should.exist(err); should.exist(err);
err.message.should.match(/Could not read image:/); (err instanceof errors.NotFoundError).should.eql(true);
done(); done();
}); });
}); });