Skip to content

Commit 10d5bce

Browse files
edrplsclaude
andcommitted
fix: attribute redirect-driven content misses to the requested path
The template pattern for a content miss is Astro.redirect("/404"): the first pass answers 302 — which the 404 logger never saw — and only the follow-up /404 render logged, as one aggregate "/404" row. The real missed path was never recorded, and excluding /404 alone would have removed the only signal those misses had. Log the miss on the first pass instead, keyed by the requested path, when a route answers with a redirect whose target is the error page. Direct hits on /404 stay unlogged — the route answers 404 by design and carries no path information. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017m3BaSx6e1DQcU77hBLTxU
1 parent a636351 commit 10d5bce

3 files changed

Lines changed: 51 additions & 7 deletions

File tree

.changeset/404-log-error-page.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"emdash": patch
33
---
44

5-
Fixes the 404 log counting the site's own /404 error page as a missed path. Every content miss that redirects to /404 was logged twice — once for the real missing path and once for "/404" itself — inflating the 404 summary with a meaningless top entry. Hits on /404 are no longer logged.
5+
Fixes 404 logging for content misses. Templates answer a missing entry with a redirect to /404, which previously left the real missed path unrecorded — the log only ever accumulated one aggregate "/404" row. Misses are now logged under the path the visitor actually requested, and hits on the /404 error page itself are no longer logged.

packages/core/src/astro/middleware/redirect.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,18 @@ export const onRequest = defineMiddleware(async (context, next) => {
112112
// No redirect matched -- proceed and check for 404
113113
const response = await next();
114114

115-
// Log 404s for unmatched paths (fire-and-forget). Skip /404 itself — Astro
116-
// serves the site error page there with status 404 on every render, so logging
117-
// it would double-count the original miss after templates redirect here.
118-
if (response.status === 404 && pathname !== "/404" && pathname !== "/404/") {
115+
// Log misses (fire-and-forget) under the path the visitor requested.
116+
// Two shapes count as a miss: an unmatched route rendering the error
117+
// page with status 404, and a matched route answering a content miss
118+
// with a redirect to /404 (the documented template pattern) — there the
119+
// missed path exists only on this first pass, before the browser
120+
// follows the redirect. The error page itself is never logged: /404
121+
// answers 404 by design and carries no path information.
122+
const location = response.headers.get("location");
123+
const missedByRedirect =
124+
isRedirectCode(response.status) && (location === "/404" || location === "/404/");
125+
const missedDirectly = response.status === 404 && pathname !== "/404" && pathname !== "/404/";
126+
if (missedDirectly || missedByRedirect) {
119127
const referrer = context.request.headers.get("referer") ?? null;
120128
const userAgent = context.request.headers.get("user-agent") ?? null;
121129
repo

packages/core/tests/unit/astro/middleware-redirect.test.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ describe("redirect middleware — issue #808", () => {
178178
});
179179
});
180180

181-
describe("redirect middleware — 404 logging excludes the site error page", () => {
181+
describe("redirect middleware — 404 logging attributes misses to the requested path", () => {
182182
let db: Kysely<Database>;
183183

184184
beforeEach(async () => {
@@ -192,7 +192,7 @@ describe("redirect middleware — 404 logging excludes the site error page", ()
192192
await teardownTestDatabase(db);
193193
});
194194

195-
it("logs a real missing path exactly once", async () => {
195+
it("logs an unmatched path exactly once", async () => {
196196
const log404 = vi.spyOn(RedirectRepository.prototype, "log404");
197197
const { context } = buildContext({ pathname: "/no-such-page" });
198198
const next = vi.fn(async () => new Response("not found", { status: 404 }));
@@ -207,6 +207,42 @@ describe("redirect middleware — 404 logging excludes the site error page", ()
207207
log404.mockRestore();
208208
});
209209

210+
it("logs a content miss under its real path across the redirect-to-/404 flow", async () => {
211+
// The documented template pattern answers a content miss with
212+
// Astro.redirect("/404"): the first request is a 302, the browser then
213+
// requests /404, which renders with status 404.
214+
const log404 = vi.spyOn(RedirectRepository.prototype, "log404");
215+
216+
const miss = buildContext({ pathname: "/posts/deleted-post" });
217+
const redirectNext = vi.fn(
218+
async () => new Response(null, { status: 302, headers: { Location: "/404" } }),
219+
);
220+
await onRequest(miss.context, redirectNext);
221+
222+
const errorPage = buildContext({ pathname: "/404" });
223+
const errorNext = vi.fn(async () => new Response("not found", { status: 404 }));
224+
await onRequest(errorPage.context, errorNext);
225+
226+
expect(log404).toHaveBeenCalledTimes(1);
227+
expect(log404).toHaveBeenCalledWith(expect.objectContaining({ path: "/posts/deleted-post" }));
228+
await log404.mock.results[0]!.value;
229+
const rows = await db.selectFrom("_emdash_404_log").select("path").execute();
230+
expect(rows.map((r) => r.path)).toEqual(["/posts/deleted-post"]);
231+
log404.mockRestore();
232+
});
233+
234+
it("does not log ordinary redirects", async () => {
235+
const log404 = vi.spyOn(RedirectRepository.prototype, "log404");
236+
const { context } = buildContext({ pathname: "/moved" });
237+
const next = vi.fn(
238+
async () => new Response(null, { status: 302, headers: { Location: "/new-home" } }),
239+
);
240+
await onRequest(context, next);
241+
242+
expect(log404).not.toHaveBeenCalled();
243+
log404.mockRestore();
244+
});
245+
210246
it("does not log the site's own /404 error page render", async () => {
211247
const log404 = vi.spyOn(RedirectRepository.prototype, "log404");
212248
for (const pathname of ["/404", "/404/"]) {

0 commit comments

Comments
 (0)