From 92d6c998b321ef60f32c8131f06bfe0a30ba80e1 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Fri, 5 Aug 2022 16:43:14 +0100
Subject: [PATCH] Fixed error handling bug in image size cache

refs https://github.com/TryGhost/Toolbox/issues/364

- Doing the `.catch(errors.NotFoundError...` was throwing another error as this syntax did not work with native promises. Checking `instanceof` works 100% and is way more explicit/readable way to handle this type of error differently
---
 .../lib/image/cached-image-size-from-url.js   | 29 +++++++++----------
 .../image/cached-image-size-from-url.test.js  | 25 +++++++++++++++-
 2 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/ghost/core/core/server/lib/image/cached-image-size-from-url.js b/ghost/core/core/server/lib/image/cached-image-size-from-url.js
index 0f5d7a5fc8..23641dd74f 100644
--- a/ghost/core/core/server/lib/image/cached-image-size-from-url.js
+++ b/ghost/core/core/server/lib/image/cached-image-size-from-url.js
@@ -22,28 +22,31 @@ class CachedImageSizeFromUrl {
      * @description Takes a url and returns image width and height from cache if available.
      * If not in cache, `getImageSizeFromUrl` is called and returns the dimensions in a Promise.
      */
-    getCachedImageSizeFromUrl(url) {
+    async getCachedImageSizeFromUrl(url) {
         if (!url || url === undefined || url === null) {
             return;
         }
 
-        // image size is not in cache
-        if (!this.cache.get(url)) {
+        const cachedImageSize = this.cache.get(url);
+
+        if (cachedImageSize) {
+            debug('Read image from cache:', url);
+
+            return cachedImageSize;
+        } else {
             return this.imageSize.getImageSizeFromUrl(url).then((res) => {
                 this.cache.set(url, res);
 
                 debug('Cached image:', url);
 
-                return this.cache.get(url);
-            }).catch(errors.NotFoundError, () => {
-                debug('Cached image (not found):', url);
-                // in case of error we just attach the url
-                this.cache.set(url, url);
-
                 return this.cache.get(url);
             }).catch((err) => {
-                debug('Cached image (error):', url);
-                logging.error(err);
+                if (err instanceof errors.NotFoundError) {
+                    debug('Cached image (not found):', url);
+                } else {
+                    debug('Cached image (error):', url);
+                    logging.error(err);
+                }
 
                 // in case of error we just attach the url
                 this.cache.set(url, url);
@@ -51,10 +54,6 @@ class CachedImageSizeFromUrl {
                 return this.cache.get(url);
             });
         }
-
-        debug('Read image from cache:', url);
-        // returns image size from cache
-        return this.cache.get(url);
     }
 }
 
diff --git a/ghost/core/test/unit/server/lib/image/cached-image-size-from-url.test.js b/ghost/core/test/unit/server/lib/image/cached-image-size-from-url.test.js
index 8ec39c5887..2ce7412bdc 100644
--- a/ghost/core/test/unit/server/lib/image/cached-image-size-from-url.test.js
+++ b/ghost/core/test/unit/server/lib/image/cached-image-size-from-url.test.js
@@ -1,3 +1,4 @@
+const errors = require('@tryghost/errors');
 const should = require('should');
 const sinon = require('sinon');
 const CachedImageSizeFromUrl = require('../../../../../core/server/lib/image/cached-image-size-from-url');
@@ -60,7 +61,7 @@ describe('lib/image: image size cache', function () {
         image2.height.should.be.equal(50);
     });
 
-    it('can handle image-size errors', async function () {
+    it('can handle generic image-size errors', async function () {
         const url = 'http://mysite.com/content/image/mypostcoverimage.jpg';
 
         sizeOfStub.rejects('error');
@@ -82,6 +83,28 @@ describe('lib/image: image size cache', function () {
         should.not.exist(image.height);
     });
 
+    it('can handle NotFoundError error', async function () {
+        const url = 'http://mysite.com/content/image/mypostcoverimage.jpg';
+
+        sizeOfStub.rejects(new errors.NotFoundError('it iz gone mate!'));
+
+        const cachedImageSizeFromUrl = new CachedImageSizeFromUrl({
+            imageSize: {
+                getImageSizeFromUrl: sizeOfStub
+            },
+            cache: new Map()
+        });
+
+        await cachedImageSizeFromUrl.getCachedImageSizeFromUrl(url);
+
+        cachedImagedSize = cachedImageSizeFromUrl.cache;
+        should.exist(cachedImagedSize);
+        cachedImagedSize.has(url).should.be.true;
+        const image = cachedImagedSize.get(url);
+        should.not.exist(image.width);
+        should.not.exist(image.height);
+    });
+
     it('should return null if url is undefined', async function () {
         const cachedImageSizeFromUrl = new CachedImageSizeFromUrl({
             imageSize: {},