Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix 404 when no route match at all #426

Merged
merged 3 commits into from
Jun 9, 2024
Merged

fix 404 when no route match at all #426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8407861
fix 404 when no route match at all
conico974 May 30, 2024
82e00cf
fix some issue with rewrite not being checked for 404 verification
conico974 May 31, 2024
2c177d2
Create tame-pets-juggle.md
conico974 Jun 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/open-next/src/core/routing/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ export function fixCacheHeaderForHtmlPages(
rawPath: string,
headers: OutgoingHttpHeaders,
) {
// We don't want to cache error pages
if (rawPath === "/404" || rawPath === "/500") {
headers[CommonHeaders.CACHE_CONTROL] =
"private, no-cache, no-store, max-age=0, must-revalidate";
return;
}
// WORKAROUND: `NextServer` does not set cache headers for HTML pages — https://github.com/serverless-stack/open-next#workaround-nextserver-does-not-set-cache-headers-for-html-pages
if (HtmlPages.includes(rawPath)) {
headers[CommonHeaders.CACHE_CONTROL] =
Expand Down
39 changes: 33 additions & 6 deletions packages/open-next/src/core/routingHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export default async function routingHandler(
internalEvent = middleware;
}

// At this point internalEvent is an InternalEvent or a MiddlewareOutputEvent

let isExternalRewrite = middleware.externalRewrite ?? false;
if (!isExternalRewrite) {
// First rewrite to be applied
Expand All @@ -57,9 +59,11 @@ export default async function routingHandler(
internalEvent = beforeRewrites.internalEvent;
isExternalRewrite = beforeRewrites.isExternalRewrite;
}
const isStaticRoute = RoutesManifest.routes.static.some((route) =>
new RegExp(route.regex).test(event.rawPath),
);
const isStaticRoute =
!isExternalRewrite &&
RoutesManifest.routes.static.some((route) =>
new RegExp(route.regex).test((internalEvent as InternalEvent).rawPath),
);

if (!isStaticRoute && !isExternalRewrite) {
// Second rewrite to be applied
Expand All @@ -74,9 +78,11 @@ export default async function routingHandler(
// We want to run this just before the dynamic route check
internalEvent = handleFallbackFalse(internalEvent, PrerenderManifest);

const isDynamicRoute = RoutesManifest.routes.dynamic.some((route) =>
new RegExp(route.regex).test(event.rawPath),
);
const isDynamicRoute =
!isExternalRewrite &&
RoutesManifest.routes.dynamic.some((route) =>
new RegExp(route.regex).test((internalEvent as InternalEvent).rawPath),
);
if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) {
// Fallback rewrite to be applied
const fallbackRewrites = handleRewrites(
Expand All @@ -87,6 +93,27 @@ export default async function routingHandler(
isExternalRewrite = fallbackRewrites.isExternalRewrite;
}

// If we still haven't found a route, we show the 404 page
// Api routes are not present in the routes manifest except if they're not behind /api
// Ideally we would need to also check api routes here
if (
!isDynamicRoute &&
!isStaticRoute &&
!isExternalRewrite &&
!internalEvent.rawPath.startsWith("/api/")
) {
internalEvent = {
...internalEvent,
rawPath: "/404",
url: "/404",
headers: {
...internalEvent.headers,
"x-middleware-response-cache-control":
"private, no-cache, no-store, max-age=0, must-revalidate",
},
};
}

// We apply the headers from the middleware response last
Object.entries({
...middlewareResponseHeaders,
Expand Down
Loading