refactor: improve error handling for image optimization to distinguish between client and server errors

Author: iDVB

packages/open-next/src/adapters/image-optimization-adapter.ts

@@ -28,6 +28,7 @@ import { emptyReadableStream, toReadableStream } from "utils/stream.js";
 import type { OpenNextHandlerOptions } from "types/overrides.js";
 import { createGenericHandler } from "../core/createGenericHandler.js";
 import { resolveImageLoader } from "../core/resolve.js";
+import { IgnorableError } from "../utils/error.js";
 import { debug, error } from "./logger.js";
 import { optimizeImage } from "./plugins/image-optimization/image-optimization.js";
 import { setNodeEnv } from "./util.js";
@@ -114,44 +115,54 @@ export async function defaultHandler(
     );
     return buildSuccessResponse(result, options?.streamCreator, etag);
   } catch (e: any) {
-    error("Failed to optimize image", e);
-
-    // Determine appropriate status code based on error
+    // Determine if this is a client error (4xx) or server error (5xx)
     let statusCode = 500; // Default to 500 for unknown errors
-    let errorMessage = "Internal server error";
+    const isClientError =
+      e &&
+      typeof e === "object" &&
+      (("statusCode" in e &&
+        typeof e.statusCode === "number" &&
+        e.statusCode >= 400 &&
+        e.statusCode < 500) ||
+        e.code === "ENOTFOUND" ||
+        e.code === "ECONNREFUSED" ||
+        (e.message &&
+          (e.message.includes("403") ||
+            e.message.includes("404") ||
+            e.message.includes("Access Denied") ||
+            e.message.includes("Not Found"))));
+
+    // Only log actual server errors as errors, log client errors as debug
+    if (isClientError) {
+      debug("Client error in image optimization", e);
+    } else {
+      error("Failed to optimize image", e);
+    }
 
-    // Check if error has HTTP status information
+    // Determine appropriate status code based on error type
     if (e && typeof e === "object") {
       if ("statusCode" in e && typeof e.statusCode === "number") {
         statusCode = e.statusCode;
-        errorMessage = `HTTP Error: ${statusCode}`;
       } else if ("code" in e) {
         const code = e.code as string;
         if (code === "ENOTFOUND" || code === "ECONNREFUSED") {
           statusCode = 404;
-          errorMessage = `Image not found: ${e.message}`;
         }
-      }
-
-      if (e.message && typeof e.message === "string") {
-        // Try to extract status codes from error messages
+      } else if (e.message) {
         if (e.message.includes("403") || e.message.includes("Access Denied")) {
           statusCode = 403;
-          errorMessage = `Access denied: ${e.message}`;
         } else if (
           e.message.includes("404") ||
           e.message.includes("Not Found")
         ) {
           statusCode = 404;
-          errorMessage = `Image not found: ${e.message}`;
-        } else {
-          errorMessage = e.message;
         }
       }
     }
 
+    // Pass through the original error message from Next.js
     return buildFailureResponse(
-      errorMessage,
+      e.message || "Internal server error",
       options?.streamCreator,
       statusCode,
     );
@@ -318,18 +329,25 @@ async function downloadHandler(
           });
       });
 
-      request.on("error", (err: NodeJS.ErrnoException) => {
-        error("Failed to get image", err);
-        // Handle common network errors
-        if (err && typeof err === "object" && "code" in err) {
-          if (err.code === "ENOTFOUND" || err.code === "ECONNREFUSED") {
-            res.statusCode = 404;
-          } else {
-            res.statusCode = 400;
-          }
+      request.on("error", (err: Error & { code?: string }) => {
+        // For network errors, these are typically client errors (bad URL, etc.)
+        // so log as debug instead of error to avoid false alarms
+        const isClientError =
+          err.code === "ENOTFOUND" || err.code === "ECONNREFUSED";
+
+        if (isClientError) {
+          // Log the full error for debugging but don't expose it to the client
+          debug("Client error fetching image", {
+            code: err.code,
+            message: err.message,
+          });
+          res.statusCode = 404; // Not Found for DNS or connection errors
         } else {
-          res.statusCode = 400;
+          error("Failed to get image", err);
+          res.statusCode = 400; // Bad Request for other errors
         }
+
+        // Don't send the error message back to the client
         res.end();
       });
     }
@@ -357,6 +375,48 @@ async function downloadHandler(
       }
     }
   } catch (e: any) {
+    // Check if this is a client error (like 404, 403, etc.)
+    const isClientError =
+      e &&
+      typeof e === "object" &&
+      (("statusCode" in e &&
+        typeof e.statusCode === "number" &&
+        e.statusCode >= 400 &&
+        e.statusCode < 500) ||
+        e.code === "ENOTFOUND" ||
+        e.code === "ECONNREFUSED" ||
+        (e.message &&
+          (e.message.includes("403") ||
+            e.message.includes("404") ||
+            e.message.includes("Access Denied") ||
+            e.message.includes("Not Found"))));
+
+    if (isClientError) {
+      debug("Client error downloading image", e);
+      // Just pass through the original error to preserve Next.js's error handling
+      // but wrap it in IgnorableError to prevent it from being logged as an error
+      const clientError = new IgnorableError(
+        e.message || "Client error downloading image",
+      );
+
+      // Preserve the original status code or set an appropriate one
+      if (e && typeof e === "object") {
+        if ("statusCode" in e && typeof e.statusCode === "number") {
+          (clientError as any).statusCode = e.statusCode;
+        } else if (e.code === "ENOTFOUND" || e.code === "ECONNREFUSED") {
+          (clientError as any).statusCode = 404;
+        } else if (e.message?.includes("403")) {
+          (clientError as any).statusCode = 403;
+        } else if (e.message?.includes("404")) {
+          (clientError as any).statusCode = 404;
+        } else {
+          (clientError as any).statusCode = 400;
+        }
+      }
+
+      throw clientError;
+    }
+
     error("Failed to download image", e);
     throw e;
   }