🐛 Fixed Infinite 404s for images (#8869)

refs #8868

- Improve the error returned from local file store
- Use the new code to differentiate between static & non-static errors

core/server/adapters/storage/LocalFileStorage.js

@@ -84,7 +84,11 @@ class LocalFileStore extends StorageBase {
             return serveStatic(self.storagePath, {maxAge: utils.ONE_YEAR_MS, fallthrough: false})(req, res, function (err) {
                 if (err) {
                     if (err.statusCode === 404) {
-                        return next(new errors.NotFoundError({message: i18n.t('errors.errors.pageNotFound')}));
+                        return next(new errors.NotFoundError({
+                            message: i18n.t('errors.errors.imageNotFound'),
+                            code: 'STATIC_FILE_NOT_FOUND',
+                            property: err.path
+                        }));
                     }
 
                     return next(new errors.GhostError({err: err}));

core/server/middleware/error-handler.js

@@ -71,22 +71,32 @@ _private.JSONErrorRenderer = function JSONErrorRenderer(err, req, res, /*jshint
     });
 };
 
-_private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint unused:false */ next) {
+_private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, next) {
+    // If the error code is explicitly set to STATIC_FILE_NOT_FOUND,
+    // Skip trying to render an HTML error, and move on to the basic error renderer
+    // I looked at doing this with accepts headers, but the internet is a crazy place...
+    // A better long term solution might be to do this based on extension
+    if (err.code === 'STATIC_FILE_NOT_FOUND') {
+        return next(err);
+    }
+
     var templateData = {
         message: err.message,
         code: err.statusCode,
         errorDetails: err.errorDetails || []
-    };
+    },
+    template = templates.error(err.statusCode);
 
     // It can be that something went wrong with the theme or otherwise loading handlebars
     // This ensures that no matter what res.render will work here
     if (_.isEmpty(req.app.engines)) {
+        template = 'error';
         req.app.engine('hbs', _private.createHbsEngine());
         req.app.set('view engine', 'hbs');
         req.app.set('views', config.get('paths').defaultViews);
     }
 
-    res.render(templates.error(err.statusCode), templateData, function renderResponse(err, html) {
+    res.render(template, templateData, function renderResponse(err, html) {
         if (!err) {
             return res.send(html);
         }
@@ -103,6 +113,10 @@ _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint un
     });
 };
 
+_private.BasicErorRenderer = function BasicErrorRenderer(err, req, res, /*jshint unused:false */ next) {
+    return res.send(res.statusCode + ' ' + err.message);
+};
+
 errorHandler.resourceNotFound = function resourceNotFound(req, res, next) {
     // TODO, handle unknown resources & methods differently, so that we can also produce
     // 405 Method Not Allowed
@@ -124,7 +138,9 @@ errorHandler.handleHTMLResponse = [
     // Make sure the error can be served
     _private.prepareError,
     // Render the error using HTML format
-    _private.HTMLErrorRenderer
+    _private.HTMLErrorRenderer,
+    // Fall back to basic if HTML is not explicitly accepted
+    _private.BasicErorRenderer
 ];
 
 module.exports = errorHandler;

core/server/translations/en.json

@@ -473,6 +473,7 @@
             "whilstTryingToRender": "whilst trying to render an error page for the error: ",
             "renderingErrorPage": "Rendering Error Page",
             "caughtProcessingError": "Ghost caught a processing error in the middleware layer.",
+            "imageNotFound": "Image not found",
             "pageNotFound": "Page not found",
             "resourceNotFound": "Resource not found"
         }

core/test/functional/routes/frontend_spec.js

@@ -179,7 +179,7 @@ describe('Frontend Routing', function () {
                     request.get('/content/images/some/file/that/doesnt-exist.jpg')
                         .expect('Cache-Control', testUtils.cacheRules['private'])
                         .expect(404)
-                        .expect(/Page not found/)
+                        .expect(/404 Image not found/)
                         .end(doEnd(done));
                 });
             });