From 9e388aee4dc97b02df535223295b2945be38ca88 Mon Sep 17 00:00:00 2001
From: Katharina Irrgang <katharina.irrgang@googlemail.com>
Date: Tue, 28 Nov 2017 15:27:18 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20=20Improved=20error=20handling?=
 =?UTF-8?q?=20for=20images=20on=20file=20storage=20which=20don't=20exist?=
 =?UTF-8?q?=20(#9282)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 (!)
---
 core/server/adapters/storage/LocalFileStorage.js         | 9 ++++++++-
 core/server/translations/en.json                         | 2 ++
 core/server/utils/image-size.js                          | 3 ++-
 core/test/unit/adapters/storage/LocalFileStorage_spec.js | 2 +-
 core/test/unit/utils/image-size_spec.js                  | 3 ++-
 5 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/core/server/adapters/storage/LocalFileStorage.js b/core/server/adapters/storage/LocalFileStorage.js
index 44c1774717..20012f4aba 100644
--- a/core/server/adapters/storage/LocalFileStorage.js
+++ b/core/server/adapters/storage/LocalFileStorage.js
@@ -134,9 +134,16 @@ class LocalFileStore extends StorageBase {
         return new Promise(function (resolve, reject) {
             fs.readFile(targetPath, function (err, bytes) {
                 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({
                         err: err,
-                        message: 'Could not read image: ' + targetPath
+                        message: i18n.t('errors.errors.cannotReadImage', {img: options.path})
                     }));
                 }
 
diff --git a/core/server/translations/en.json b/core/server/translations/en.json
index 974f44c6da..f0442ab6d9 100644
--- a/core/server/translations/en.json
+++ b/core/server/translations/en.json
@@ -486,6 +486,8 @@
             "renderingErrorPage": "Rendering Error Page",
             "caughtProcessingError": "Ghost caught a processing error in the middleware layer.",
             "imageNotFound": "Image not found",
+            "imageNotFoundWithRef": "Image not found: {img}",
+            "cannotReadImage": "Could not read image: {img}",
             "pageNotFound": "Page not found",
             "resourceNotFound": "Resource not found"
         }
diff --git a/core/server/utils/image-size.js b/core/server/utils/image-size.js
index e4a7aaf713..839c6110a9 100644
--- a/core/server/utils/image-size.js
+++ b/core/server/utils/image-size.js
@@ -202,9 +202,10 @@ getImageSizeFromFilePath = function getImageSizeFromFilePath(imagePath) {
                 }
             }));
         }).catch(function (err) {
-            if (err instanceof errors.GhostError) {
+            if (errors.utils.isIgnitionError(err)) {
                 return Promise.reject(err);
             }
+
             return Promise.reject(new errors.InternalServerError({
                 message: err.message,
                 code: 'IMAGE_SIZE_STORAGE',
diff --git a/core/test/unit/adapters/storage/LocalFileStorage_spec.js b/core/test/unit/adapters/storage/LocalFileStorage_spec.js
index c462436a55..4ff3b45ef9 100644
--- a/core/test/unit/adapters/storage/LocalFileStorage_spec.js
+++ b/core/test/unit/adapters/storage/LocalFileStorage_spec.js
@@ -175,7 +175,7 @@ describe('Local File System Storage', function () {
                     done(new Error('image should not exist'));
                 })
                 .catch(function (err) {
-                    (err instanceof errors.GhostError).should.eql(true);
+                    (err instanceof errors.NotFoundError).should.eql(true);
                     err.code.should.eql('ENOENT');
                     done();
                 });
diff --git a/core/test/unit/utils/image-size_spec.js b/core/test/unit/utils/image-size_spec.js
index fef7e6ef54..ca5ab874ca 100644
--- a/core/test/unit/utils/image-size_spec.js
+++ b/core/test/unit/utils/image-size_spec.js
@@ -4,6 +4,7 @@ var should = require('should'),
     nock = require('nock'),
     configUtils = require('../../utils/configUtils'),
     utils = require('../../../server/utils'),
+    errors = require('../../../server/errors'),
     storage = require('../../../server/adapters/storage'),
     path = require('path'),
 
@@ -463,7 +464,7 @@ describe('Image Size', function () {
             result = imageSize.getImageSizeFromFilePath(url)
                 .catch(function (err) {
                     should.exist(err);
-                    err.message.should.match(/Could not read image:/);
+                    (err instanceof errors.NotFoundError).should.eql(true);
                     done();
                 });
         });