mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-03-25 02:31:59 -05:00
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
This commit is contained in:
parent
7910779143
commit
92d6c998b3
2 changed files with 38 additions and 16 deletions
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -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: {},
|
||||
|
|
Loading…
Add table
Reference in a new issue