feat(image-adapter): improve error handling and status codes

.changeset/big-laws-jam.md

@@ -0,0 +1,5 @@
+---
+"@opennextjs/aws": major
+---
+
+Improved error handling in image optimization adapter for S3 ListBucket permission errors

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,10 +115,19 @@ export async function defaultHandler(
     );
     return buildSuccessResponse(result, options?.streamCreator, etag);
   } catch (e: any) {
-    error("Failed to optimize image", e);
+    // All image-related errors should be treated as client errors
+    // Extract status code from error or default to 400 Bad Request
+    const statusCode = e.statusCode || 400;
+    const errorMessage = e.message || "Failed to process image request";
+
+    // Create an IgnorableError for proper monitoring classification
+    const clientError = new IgnorableError(errorMessage, statusCode);
+    error("Failed to optimize image", clientError);
+
     return buildFailureResponse(
-      "Internal server error",
+      errorMessage,
       options?.streamCreator,
+      statusCode, // Use the appropriate status code (preserving original when available)
     );
   }
 }
@@ -238,50 +248,140 @@ async function downloadHandler(
   // directly.
   debug("downloadHandler url", url);
 
-  // Reads the output from the Writable and writes to the response
-  const pipeRes = (w: Writable, res: ServerResponse) => {
-    w.pipe(res)
+  /**
+   * Helper function to handle image errors consistently with appropriate response
+   * @param e The error object
+   * @param res The server response object
+   * @param isInternalImage Whether the error is from an internal image (S3) or external image
+   */
+  function handleImageError(
+    e: any,
+    res: ServerResponse,
+    isInternalImage: boolean,
+  ) {
+    let originalStatus = e.statusCode || e.$metadata?.httpStatusCode || 500;
+    let message = e.message || "Failed to process image request";
+
+    // Special handling for S3 ListBucket permission errors
+    // AWS SDK v3 nests error details deeply within the error object
+    const isListBucketError =
+      (message.includes("s3:ListBucket") && message.includes("AccessDenied")) ||
+      e.error?.message?.includes("s3:ListBucket") ||
+      (e.Code === "AccessDenied" && e.Message?.includes("s3:ListBucket"));
+
+    if (isListBucketError) {
+      message = "Image not found or access denied";
+      // For S3 ListBucket errors, ensure we're using 403 (the actual AWS error)
+      if (originalStatus === 500 && e.$metadata?.httpStatusCode === 403) {
+        originalStatus = 403;
+      }
+
+      // Log using IgnorableError to classify as client error
+      const clientError = new IgnorableError(message, originalStatus);
+      error("S3 ListBucket permission error", clientError);
+    } else {
+      // Log all other errors as client errors
+      const clientError = new IgnorableError(message, originalStatus);
+      error("Failed to process image", clientError);
+    }
+
+    // For external images, throw if not ListBucket error
+    // Next.js will preserve the status code for external images
+    if (!isInternalImage && !isListBucketError) {
+      const formattedError = new Error(message);
+      // @ts-ignore: Add statusCode property to Error
+      formattedError.statusCode = originalStatus >= 500 ? 400 : originalStatus;
+      throw formattedError;
+    }
+
+    // Different handling for internal vs external images
+    const finalStatus = originalStatus >= 500 ? 400 : originalStatus;
+    res.statusCode = finalStatus;
+
+    // For internal images, we want to trigger Next.js's "internal response invalid" message
+    if (isInternalImage) {
+      // For internal images, don't set Content-Type to trigger Next.js's default error handling
+      // This should result in "url parameter is valid but internal response is invalid"
+
+      // Still include error details in headers for debugging only
+      const errorMessage = isListBucketError ? "Access denied" : message;
+      res.setHeader("x-nextjs-internal-error", errorMessage);
+      res.end();
+    } else {
+      // For external images, maintain existing behavior with text/plain
+      res.setHeader("Content-Type", "text/plain");
+
+      if (isListBucketError) {
+        res.end("Access denied");
+      } else {
+        res.end(message);
+      }
+    }
+  }
+
+  // Pipes data from a writable stream to the server response
+  function pipeStream(
+    stream: Writable,
+    res: ServerResponse,
+    isInternalImage: boolean,
+  ) {
+    stream
+      .pipe(res)
       .once("close", () => {
         res.statusCode = 200;
         res.end();
       })
       .once("error", (err) => {
-        error("Failed to get image", err);
-        res.statusCode = 400;
-        res.end();
+        error("Error streaming image data", err);
+        handleImageError(err, res, isInternalImage);
       });
-  };
+  }
 
+  // Main handler logic with clearer error paths
   try {
-    // Case 1: remote image URL => download the image from the URL
+    // EXTERNAL IMAGE HANDLING
     if (url.href.toLowerCase().match(/^https?:\/\//)) {
-      pipeRes(https.get(url), res);
+      try {
+        pipeStream(https.get(url), res, false);
+      } catch (e: any) {
+        handleImageError(e, res, false);
+      }
+      return;
     }
-    // Case 2: local image => download the image from S3
-    else {
-      // Download image from S3
-      // note: S3 expects keys without leading `/`
 
+    // INTERNAL IMAGE HANDLING (S3)
+    try {
       const response = await loader.load(url.href);
 
+      // Handle empty response body
       if (!response.body) {
-        throw new Error("Empty response body from the S3 request.");
-      }
+        const message = "Empty response body from the S3 request.";
+        const clientError = new IgnorableError(message, 400);
+        error("Empty response from S3", clientError);
 
-      // @ts-ignore
-      pipeRes(response.body, res);
+        res.statusCode = 400;
+        res.setHeader("Content-Type", "text/plain");
+        res.end(message);
+        return;
+      }
 
-      // Respect the bucket file's content-type and cache-control
-      // imageOptimizer will use this to set the results.maxAge
+      // Set headers from the response
       if (response.contentType) {
         res.setHeader("Content-Type", response.contentType);
       }
       if (response.cacheControl) {
         res.setHeader("Cache-Control", response.cacheControl);
       }
+
+      // Stream the image to the client
+      // @ts-ignore
+      pipeStream(response.body, res, true);
+    } catch (e: any) {
+      // Direct response for all internal image errors
+      handleImageError(e, res, true);
     }
   } catch (e: any) {
-    error("Failed to download image", e);
-    throw e;
+    // Catch-all for any unexpected errors
+    handleImageError(e, res, true);
   }
 }

packages/open-next/src/utils/error.ts

@@ -3,6 +3,7 @@ export interface BaseOpenNextError {
   readonly canIgnore: boolean;
   // 0 - debug, 1 - warn, 2 - error
   readonly logLevel: 0 | 1 | 2;
+  readonly statusCode?: number;
 }
 
 // This is an error that can be totally ignored
@@ -11,9 +12,12 @@ export class IgnorableError extends Error implements BaseOpenNextError {
   readonly __openNextInternal = true;
   readonly canIgnore = true;
   readonly logLevel = 0;
-  constructor(message: string) {
+  readonly statusCode: number;
+
+  constructor(message: string, statusCode = 400) {
     super(message);
     this.name = "IgnorableError";
+    this.statusCode = statusCode;
   }
 }
 
@@ -23,9 +27,12 @@ export class RecoverableError extends Error implements BaseOpenNextError {
   readonly __openNextInternal = true;
   readonly canIgnore = true;
   readonly logLevel = 1;
-  constructor(message: string) {
+  readonly statusCode: number;
+
+  constructor(message: string, statusCode = 400) {
     super(message);
     this.name = "RecoverableError";
+    this.statusCode = statusCode;
   }
 }
 
@@ -34,9 +41,12 @@ export class FatalError extends Error implements BaseOpenNextError {
   readonly __openNextInternal = true;
   readonly canIgnore = false;
   readonly logLevel = 2;
-  constructor(message: string) {
+  readonly statusCode: number;
+
+  constructor(message: string, statusCode = 500) {
     super(message);
     this.name = "FatalError";
+    this.statusCode = statusCode;
   }
 }