Skip to content

Commit fdc1f4b

Browse files
author
Nicolas Dorseuil
committed
update image error
1 parent 5bd6fbc commit fdc1f4b

File tree

2 files changed

+65
-63
lines changed

2 files changed

+65
-63
lines changed

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

Lines changed: 14 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { emptyReadableStream, toReadableStream } from "utils/stream.js";
2828
import type { OpenNextHandlerOptions } from "types/overrides.js";
2929
import { createGenericHandler } from "../core/createGenericHandler.js";
3030
import { resolveImageLoader } from "../core/resolve.js";
31-
import { IgnorableError } from "../utils/error.js";
31+
import { FatalError, IgnorableError } from "../utils/error.js";
3232
import { debug, error } from "./logger.js";
3333
import { optimizeImage } from "./plugins/image-optimization/image-optimization.js";
3434
import { setNodeEnv } from "./util.js";
@@ -258,39 +258,18 @@ async function downloadHandler(
258258
res: ServerResponse,
259259
isInternalImage: boolean,
260260
) {
261-
let originalStatus = e.statusCode || e.$metadata?.httpStatusCode || 500;
262-
let message = e.message || "Failed to process image request";
263-
264-
// Special handling for S3 ListBucket permission errors
265-
// AWS SDK v3 nests error details deeply within the error object
266-
const isListBucketError =
267-
(message.includes("s3:ListBucket") && message.includes("AccessDenied")) ||
268-
e.error?.message?.includes("s3:ListBucket") ||
269-
(e.Code === "AccessDenied" && e.Message?.includes("s3:ListBucket"));
270-
271-
if (isListBucketError) {
272-
message = "Image not found or access denied";
273-
// For S3 ListBucket errors, ensure we're using 403 (the actual AWS error)
274-
if (originalStatus === 500 && e.$metadata?.httpStatusCode === 403) {
275-
originalStatus = 403;
276-
}
261+
const originalStatus = e.statusCode || e.$metadata?.httpStatusCode || 500;
262+
const message = e.message || "Failed to process image request";
277263

278-
// Log using IgnorableError to classify as client error
279-
const clientError = new IgnorableError(message, originalStatus);
280-
error("S3 ListBucket permission error", clientError);
281-
} else {
282-
// Log all other errors as client errors
283-
const clientError = new IgnorableError(message, originalStatus);
284-
error("Failed to process image", clientError);
285-
}
264+
// Log all other errors as client errors
265+
const clientError = new IgnorableError(message, originalStatus);
266+
error("Failed to process image", clientError);
286267

287-
// For external images, throw if not ListBucket error
268+
// For external images we throw with the status code
288269
// Next.js will preserve the status code for external images
289-
if (!isInternalImage && !isListBucketError) {
290-
const formattedError = new Error(message);
291-
// @ts-ignore: Add statusCode property to Error
292-
formattedError.statusCode = originalStatus >= 500 ? 400 : originalStatus;
293-
throw formattedError;
270+
if (!isInternalImage) {
271+
const statusCode = originalStatus >= 500 ? 400 : originalStatus;
272+
throw new FatalError(message, statusCode);
294273
}
295274

296275
// Different handling for internal vs external images
@@ -303,18 +282,15 @@ async function downloadHandler(
303282
// This should result in "url parameter is valid but internal response is invalid"
304283

305284
// Still include error details in headers for debugging only
306-
const errorMessage = isListBucketError ? "Access denied" : message;
307-
res.setHeader("x-nextjs-internal-error", errorMessage);
285+
res.setHeader("x-nextjs-internal-error", message);
308286
res.end();
309287
} else {
310288
// For external images, maintain existing behavior with text/plain
311289
res.setHeader("Content-Type", "text/plain");
312290

313-
if (isListBucketError) {
314-
res.end("Access denied");
315-
} else {
316-
res.end(message);
317-
}
291+
// We should **never** send the error message to the client
292+
// This is to prevent leaking sensitive information
293+
res.end("Failed to process image request");
318294
}
319295
}
320296

packages/open-next/src/overrides/imageLoader/s3.ts

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,44 +2,70 @@ import type { Readable } from "node:stream";
22

33
import { GetObjectCommand, S3Client } from "@aws-sdk/client-s3";
44
import type { ImageLoader } from "types/overrides";
5-
import { FatalError } from "utils/error";
5+
import { FatalError, IgnorableError } from "utils/error";
66

7-
import { awsLogger } from "../../adapters/logger";
7+
import { awsLogger, error } from "../../adapters/logger";
88

99
const { BUCKET_NAME, BUCKET_KEY_PREFIX } = process.env;
1010

1111
function ensureBucketExists() {
1212
if (!BUCKET_NAME) {
13-
throw new Error("Bucket name must be defined!");
13+
throw new FatalError("Bucket name must be defined!");
1414
}
1515
}
1616

1717
const s3Loader: ImageLoader = {
1818
name: "s3",
1919
load: async (key: string) => {
20-
const s3Client = new S3Client({ logger: awsLogger });
21-
22-
ensureBucketExists();
23-
const keyPrefix = BUCKET_KEY_PREFIX?.replace(/^\/|\/$/g, "");
24-
const response = await s3Client.send(
25-
new GetObjectCommand({
26-
Bucket: BUCKET_NAME,
27-
Key: keyPrefix
28-
? `${keyPrefix}/${key.replace(/^\//, "")}`
29-
: key.replace(/^\//, ""),
30-
}),
31-
);
32-
const body = response.Body as Readable | undefined;
33-
34-
if (!body) {
35-
throw new FatalError("No body in S3 response");
36-
}
20+
try {
21+
const s3Client = new S3Client({ logger: awsLogger });
22+
23+
ensureBucketExists();
24+
const keyPrefix = BUCKET_KEY_PREFIX?.replace(/^\/|\/$/g, "");
25+
const response = await s3Client.send(
26+
new GetObjectCommand({
27+
Bucket: BUCKET_NAME,
28+
Key: keyPrefix
29+
? `${keyPrefix}/${key.replace(/^\//, "")}`
30+
: key.replace(/^\//, ""),
31+
}),
32+
);
33+
const body = response.Body as Readable | undefined;
34+
35+
if (!body) {
36+
throw new FatalError("No body in S3 response");
37+
}
3738

38-
return {
39-
body: body,
40-
contentType: response.ContentType,
41-
cacheControl: response.CacheControl,
42-
};
39+
return {
40+
body: body,
41+
contentType: response.ContentType,
42+
cacheControl: response.CacheControl,
43+
};
44+
} catch (e: any) {
45+
if (e instanceof FatalError) {
46+
throw e;
47+
}
48+
// Special handling for S3 ListBucket permission errors
49+
// AWS SDK v3 nests error details deeply within the error object
50+
const isListBucketError =
51+
(e.message.includes("s3:ListBucket") &&
52+
e.message.includes("AccessDenied")) ||
53+
e.error?.message?.includes("s3:ListBucket") ||
54+
(e.Code === "AccessDenied" && e.Message?.includes("s3:ListBucket"));
55+
56+
if (isListBucketError) {
57+
const statusCode =
58+
e.statusCode === 500 && e.$metadata?.httpStatusCode === 403
59+
? 403
60+
: 500;
61+
throw new IgnorableError(
62+
"Image not found or access denied",
63+
statusCode,
64+
);
65+
}
66+
error("Failed to load image from S3", e);
67+
throw new FatalError("Failed to load image from S3");
68+
}
4369
},
4470
};
4571

0 commit comments

Comments
 (0)