Skip to content

Commit c2817fe

Browse files
authored
Handle partial failure in ISR revalidation (#410)
* fix header on HEAD requests * handle partial failure * Create slow-camels-swim.md
1 parent ee57945 commit c2817fe

File tree

6 files changed

+70
-21
lines changed

6 files changed

+70
-21
lines changed

.changeset/slow-camels-swim.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"open-next": patch
3+
---
4+
5+
Handle partial failure in ISR revalidation

examples/sst/stacks/OpenNextReferenceImplementation.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,12 @@ export class OpenNextCdkReferenceImplementation extends Construct {
292292
runtime: Runtime.NODEJS_18_X,
293293
timeout: Duration.seconds(30),
294294
});
295-
consumer.addEventSource(new SqsEventSource(queue, { batchSize: 5 }));
295+
consumer.addEventSource(
296+
new SqsEventSource(queue, {
297+
batchSize: 5,
298+
reportBatchItemFailures: true,
299+
}),
300+
);
296301
return queue;
297302
}
298303

packages/open-next/src/adapters/revalidate.ts

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@ export interface RevalidateEvent {
2121
records: {
2222
host: string;
2323
url: string;
24+
id: string;
2425
}[];
2526
}
2627

2728
const defaultHandler = async (event: RevalidateEvent) => {
29+
const failedRecords: RevalidateEvent["records"] = [];
2830
for (const record of event.records) {
2931
const { host, url } = record;
3032
debug(`Revalidating stale page`, { host, url });
@@ -37,27 +39,53 @@ const defaultHandler = async (event: RevalidateEvent) => {
3739
// - "previewModeId" is used to ensure the page is revalidated in a
3840
// blocking way in lambda
3941
// https://github.com/vercel/next.js/blob/1088b3f682cbe411be2d1edc502f8a090e36dee4/packages/next/src/server/api-utils/node.ts#L353
40-
await new Promise<IncomingMessage>((resolve, reject) => {
41-
const req = https.request(
42-
`https://${host}${url}`,
43-
{
44-
method: "HEAD",
45-
headers: {
46-
"x-prerender-revalidate": prerenderManifest.preview.previewModeId,
47-
"x-isr": "1",
42+
try {
43+
await new Promise<IncomingMessage>((resolve, reject) => {
44+
const req = https.request(
45+
`https://${host}${url}`,
46+
{
47+
method: "HEAD",
48+
headers: {
49+
"x-prerender-revalidate": prerenderManifest.preview.previewModeId,
50+
"x-isr": "1",
51+
},
4852
},
49-
},
50-
(res) => resolve(res),
51-
);
52-
req.on("error", (err) => {
53-
error(`Error revalidating page`, { host, url });
54-
reject(err);
53+
(res) => {
54+
debug("revalidating", {
55+
url,
56+
host,
57+
headers: res.headers,
58+
statusCode: res.statusCode,
59+
});
60+
if (
61+
res.statusCode !== 200 ||
62+
res.headers["x-nextjs-cache"] !== "REVALIDATED"
63+
) {
64+
failedRecords.push(record);
65+
}
66+
resolve(res);
67+
},
68+
);
69+
req.on("error", (err) => {
70+
error(`Error revalidating page`, { host, url });
71+
reject(err);
72+
});
73+
req.end();
5574
});
56-
req.end();
75+
} catch (err) {
76+
failedRecords.push(record);
77+
}
78+
}
79+
if (failedRecords.length > 0) {
80+
error(`Failed to revalidate ${failedRecords.length} pages`, {
81+
failedRecords,
5782
});
5883
}
84+
5985
return {
6086
type: "revalidate",
87+
// Records returned here are the ones that failed to revalidate
88+
records: failedRecords,
6189
};
6290
};
6391

packages/open-next/src/converters/sqs-revalidate.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,23 @@ import { Converter } from "types/open-next";
33

44
import { RevalidateEvent } from "../adapters/revalidate";
55

6-
const converter: Converter<RevalidateEvent, { type: "revalidate" }> = {
6+
const converter: Converter<RevalidateEvent, RevalidateEvent> = {
77
convertFrom(event: SQSEvent) {
88
const records = event.Records.map((record) => {
99
const { host, url } = JSON.parse(record.body);
10-
return { host, url };
10+
return { host, url, id: record.messageId };
1111
});
1212
return Promise.resolve({
1313
type: "revalidate",
1414
records,
1515
});
1616
},
17-
convertTo() {
17+
convertTo(revalidateEvent) {
1818
return Promise.resolve({
1919
type: "revalidate",
20+
batchItemFailures: revalidateEvent.records.map((record) => ({
21+
itemIdentifier: record.id,
22+
})),
2023
});
2124
},
2225
name: "sqs-revalidate",

packages/open-next/src/core/routing/util.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ export function getUrlParts(url: string, isExternal: boolean) {
5656
export function convertRes(res: OpenNextNodeResponse) {
5757
// Format Next.js response to Lambda response
5858
const statusCode = res.statusCode || 200;
59-
const headers = parseHeaders(res.headers);
59+
// When using HEAD requests, it seems that flushHeaders is not called, not sure why
60+
// Probably some kind of race condition
61+
const headers = parseHeaders(res.getFixedHeaders());
6062
const isBase64Encoded = isBinaryContentType(
6163
Array.isArray(headers["content-type"])
6264
? headers["content-type"][0]

packages/open-next/src/http/openNextResponse.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,12 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse {
145145
return this.headers;
146146
}
147147

148+
getFixedHeaders(): OutgoingHttpHeaders {
149+
// Do we want to apply this on writeHead?
150+
this.fixHeaders(this.headers);
151+
return this.headers;
152+
}
153+
148154
getHeader(name: string): OutgoingHttpHeader | undefined {
149155
return this.headers[name.toLowerCase()];
150156
}
@@ -156,7 +162,6 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse {
156162
// Only used directly in next@14+
157163
flushHeaders() {
158164
this.headersSent = true;
159-
this.fixHeaders(this.headers);
160165
// Initial headers should be merged with the new headers
161166
// These initial headers are the one created either in the middleware or in next.config.js
162167
// We choose to override response headers with middleware headers
@@ -170,6 +175,7 @@ export class OpenNextNodeResponse extends Transform implements ServerResponse {
170175
...this.initialHeaders,
171176
};
172177
}
178+
this.fixHeaders(this.headers);
173179
if (this._cookies.length > 0) {
174180
// For cookies we cannot do the same as for other headers
175181
// We need to merge the cookies, and in this case, cookies generated by the routes or pages

0 commit comments

Comments
 (0)