Switched to throwing error upon failed image processing

ref https://linear.app/tryghost/issue/ENG-740/http-500-error-when-image-processing-fails
refs 4aad551c72

- upon further discussion, we've decided it's better to throw an error
  in this case because the uploaded image is deemed invalid and storing
  it on the filesystem might cause more issues with resizing/further
  processing in the future
- this commit implements that and alters the tests

ghost/core/core/server/api/endpoints/images.js

@@ -1,8 +1,10 @@
 /* eslint-disable ghost/ghost-custom/max-api-complexity */
-const storage = require('../../adapters/storage');
-const imageTransform = require('@tryghost/image-transform');
-const config = require('../../../shared/config');
 const path = require('path');
+const errors = require('@tryghost/errors');
+const imageTransform = require('@tryghost/image-transform');
+
+const storage = require('../../adapters/storage');
+const config = require('../../../shared/config');
 
 module.exports = {
     docName: 'images',
@@ -36,8 +38,12 @@ module.exports = {
                 try {
                     await imageTransform.resizeFromPath(options);
                 } catch (err) {
-                    // If the image processing fails, we just want to store the original image
-                    return store.save(frame.file);
+                    // If the image processing fails, we don't want to store the image because it's corrupted/invalid
+                    throw new errors.BadRequestError({
+                        message: 'Image processing failed',
+                        context: err.message,
+                        help: 'Please verify that the image is valid'
+                    });
                 }
 
                 // Store the processed/optimized image

ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap

@@ -72,6 +72,24 @@ Object {
 }
 `;
 
+exports[`Images API Does not return HTTP 500 when image processing fails 1: [body] 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "code": null,
+      "context": "Image processing failed Image processing failed",
+      "details": null,
+      "ghostErrorCode": null,
+      "help": "Please verify that the image is valid",
+      "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/,
+      "message": "Request not understood error, cannot upload image.",
+      "property": null,
+      "type": "BadRequestError",
+    },
+  ],
+}
+`;
+
 exports[`Images API Will error when filename is too long 1: [body] 1`] = `
 Object {
   "errors": Array [

ghost/core/test/e2e-api/admin/images.test.js

@@ -308,7 +308,14 @@ describe('Images API', function () {
         const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png');
         const fileContents = await fs.readFile(originalFilePath);
 
+        const loggingStub = sinon.stub(logging, 'error');
         await uploadImageRequest({fileContents, filename: 'test.png', contentType: 'image/png'})
-            .expectStatus(201);
+            .expectStatus(400)
+            .matchBodySnapshot({
+                errors: [{
+                    id: anyErrorId
+                }]
+            });
+        sinon.assert.calledOnce(loggingStub);
     });
 });