feat(store): only let image transformations fail on Sharp errors

compasjs · Aug 30, 2023 · 945136d · 945136d
1 parent 9868dc3
commit 945136d
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 69 deletions.
diff --git a/packages/store/src/file.js b/packages/store/src/file.js
@@ -192,7 +192,7 @@ export async function fileCreateOrUpdate(
  * @returns {Promise<Buffer>}
  */
 async function fileTransformInPlaceInternal(fileBuffer, operations) {
-  const sharpInstance = sharp(fileBuffer);
+  const sharpInstance = sharp(fileBuffer, { failOn: "error" });
 
   if (operations.stripMetadata !== true) {
     sharpInstance.withMetadata({});

diff --git a/packages/store/src/files-jobs.js b/packages/store/src/files-jobs.js
@@ -91,7 +91,7 @@ export function jobFileGeneratePlaceholderImage(s3Client, bucketName) {
       return;
     }
 
-    const sharpInstance = sharp(buffer).rotate();
+    const sharpInstance = sharp(buffer, { failOn: "error" }).rotate();
     const metadata = await sharpInstance.metadata();
 
     const placeholderImageBuffer = await sharpInstance
@@ -196,82 +196,70 @@ export function jobFileTransformImage(s3Client) {
       return;
     }
 
-    try {
-      const sharpInstance = sharp(buffer);
-      sharpInstance.rotate();
-
-      const metadataPromise = sharpInstance.metadata();
-
-      const originalWidth =
-        file.meta?.originalWidth ?? (await metadataPromise).width;
-      const originalHeight =
-        file.meta?.originalHeight ?? (await metadataPromise).height;
-
-      if (!isNil(originalWidth) && originalWidth > options.w) {
-        // Only resize if width is greater than the needed with, so we don't upscale
-        sharpInstance.resize(options.w);
-      }
-
-      if (options.contentType === "image/webp") {
-        sharpInstance.webp({ quality: options.q });
-      } else if (options.contentType === "image/avif") {
-        sharpInstance.avif({ quality: options.q });
-      } else if (options.contentType === "image/png") {
-        sharpInstance.png({ quality: options.q });
-      } else if (
-        options.contentType === "image/jpg" ||
-        options.contentType === "image/jpeg"
-      ) {
-        sharpInstance.jpeg({ quality: options.q });
-      } else if (options.contentType === "image/gif") {
-        sharpInstance.gif({});
-      }
-
-      const image = await fileCreateOrUpdate(
-        sql,
-        s3Client,
-        {
-          // @ts-expect-error
-          bucketName: file.bucketName,
-        },
-        {
-          name: transformKey,
-          contentType: options.contentType,
-          meta: {
-            transformedFromOriginal: file.id,
-          },
+    const sharpInstance = sharp(buffer, { failOn: "error" });
+    sharpInstance.rotate();
+
+    const metadataPromise = sharpInstance.metadata();
+
+    const originalWidth =
+      file.meta?.originalWidth ?? (await metadataPromise).width;
+    const originalHeight =
+      file.meta?.originalHeight ?? (await metadataPromise).height;
+
+    if (!isNil(originalWidth) && originalWidth > options.w) {
+      // Only resize if width is greater than the needed with, so we don't upscale
+      sharpInstance.resize(options.w);
+    }
+
+    if (options.contentType === "image/webp") {
+      sharpInstance.webp({ quality: options.q });
+    } else if (options.contentType === "image/avif") {
+      sharpInstance.avif({ quality: options.q });
+    } else if (options.contentType === "image/png") {
+      sharpInstance.png({ quality: options.q });
+    } else if (
+      options.contentType === "image/jpg" ||
+      options.contentType === "image/jpeg"
+    ) {
+      sharpInstance.jpeg({ quality: options.q });
+    } else if (options.contentType === "image/gif") {
+      sharpInstance.gif({});
+    }
+
+    const image = await fileCreateOrUpdate(
+      sql,
+      s3Client,
+      {
+        // @ts-expect-error
+        bucketName: file.bucketName,
+      },
+      {
+        name: transformKey,
+        contentType: options.contentType,
+        meta: {
+          transformedFromOriginal: file.id,
         },
-        await sharpInstance.toBuffer(),
-      );
+      },
+      await sharpInstance.toBuffer(),
+    );
 
-      // @ts-expect-error
-      await atomicSetTransformKey(sql, file.id, transformKey, image.id);
-
-      if (
-        (isNil(file.meta?.originalWidth) || isNil(file.meta?.originalHeight)) &&
-        !isNil(originalWidth) &&
-        !isNil(originalHeight)
-      ) {
-        // Update the original image to include the width and height.
-        await query`UPDATE "file"
+    // @ts-expect-error
+    await atomicSetTransformKey(sql, file.id, transformKey, image.id);
+
+    if (
+      (isNil(file.meta?.originalWidth) || isNil(file.meta?.originalHeight)) &&
+      !isNil(originalWidth) &&
+      !isNil(originalHeight)
+    ) {
+      // Update the original image to include the width and height.
+      await query`UPDATE "file"
                     SET
                       "meta" = jsonb_set(jsonb_set("meta", '{originalHeight}', ${String(
                         originalHeight,
                       )}::jsonb), '{originalWidth}',
                                          ${String(originalWidth)}::jsonb)
                     WHERE
                       id = ${file.id}`.exec(sql);
-      }
-    } catch (/** @type {any} */ e) {
-      if (
-        e.message === "VipsJpeg: Invalid SOS parameters for sequential JPEG"
-      ) {
-        // JPEG error, just return the original image and never try again.
-        // @ts-expect-error
-        await atomicSetTransformKey(sql, file.id, transformKey, file.id);
-      }
-
-      throw e;
     }
 
     eventStop(event);