brackt/app/lib/__tests__/error-reporting.test.ts
Claude ed9c62571b
All checks were successful
🚀 Deploy / 🧪 Test (pull_request) Successful in 3m21s
🚀 Deploy / ʦ🔍 Typecheck & Lint (pull_request) Successful in 1m20s
🚀 Deploy / 🐳 Build (pull_request) Has been skipped
🚀 Deploy / 🚀 Deploy (pull_request) Has been skipped
Fix three defects in the Sentry 404 filter
Code review of 41a0237 found the same-origin Referer exception never
fires in production, which defeats the guarantee that change was built
on: broken internal links stay visible while scanners are dropped.

@react-router/express builds request.url from req.protocol and nothing
sets `trust proxy`, so request.url reads http:// inside the container
while browsers send an https:// Referer. Comparing full origins never
matched, so every router-internal 4xx was dropped. Compare host instead
-- protocol says nothing about whether a link was ours, and this is the
same mismatch $leagueId.server.ts already works around for invite URLs.
Adding `trust proxy` would fix it too but changes req.ip and req.secure
server-wide for one comparison.

Removing ignoreErrors also dropped the .css/.js filters with nothing to
replace them. Stale clients request the previous deploy's hashed bundles
with a same-host Referer, so once the Referer check worked, every deploy
would spike Sentry. Assets now never report, referer or not.

Narrow the dropped statuses from all 4xx to 404 and 405, the two that
mean nothing matched the request. React Router's internal 400 (route
missing a loader) and 403 (route does not match URL) are real
misconfigurations and were being swallowed whenever no Referer was sent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ESrxBMZMx2BD9rcKTCgLe4
2026-09-14 21:18:33 +00:00

230 lines
6.8 KiB
TypeScript

import { describe, it, expect } from "vitest";
import { createStaticHandler } from "react-router";
import { shouldReportServerError } from "../error-reporting";
const ORIGIN = "https://brackt.com";
/** Shaped like the ErrorResponse React Router hands to `handleError`. */
function routeError(
status: number,
internal: boolean,
statusText = "Not Found",
) {
return {
status,
statusText,
internal,
data: `Error: No route matches URL "/blog/wp/v2/posts/999999"`,
};
}
function request(path: string, referer?: string, method = "GET") {
return new Request(`${ORIGIN}${path}`, {
method,
headers: referer ? { referer } : {},
});
}
describe("shouldReportServerError", () => {
it("drops a router 404 for a scanner hitting a URL cold", () => {
expect(
shouldReportServerError(
routeError(404, true),
request("/blog/wp/v2/posts/999999"),
),
).toBe(false);
});
it("drops a router 404 linked from another site", () => {
expect(
shouldReportServerError(
routeError(404, true),
request("/blog/", "https://evil.example/"),
),
).toBe(false);
});
it("reports a router 404 linked from one of our own pages", () => {
expect(
shouldReportServerError(
routeError(404, true),
request("/leagues/gone", `${ORIGIN}/leagues`),
),
).toBe(true);
});
it("drops the 405 from a POST to a route with no action", () => {
expect(
shouldReportServerError(
routeError(405, true, "Method Not Allowed"),
request("/", undefined, "POST"),
),
).toBe(false);
});
it("reports a 404 the app threw deliberately", () => {
expect(
shouldReportServerError(
routeError(404, false),
request("/leagues/missing"),
),
).toBe(true);
});
it("reports a 403 the app threw from an ownership check", () => {
expect(
shouldReportServerError(
routeError(403, false, "Forbidden"),
request("/admin/sports"),
),
).toBe(true);
});
it("reports a router-internal 500", () => {
expect(
shouldReportServerError(
routeError(500, true, "Internal Server Error"),
request("/leagues"),
),
).toBe(true);
});
it("reports a plain exception", () => {
expect(
shouldReportServerError(new Error("boom"), request("/leagues")),
).toBe(true);
});
it("reports anything that is not a route error response", () => {
expect(shouldReportServerError("just a string", request("/leagues"))).toBe(
true,
);
expect(shouldReportServerError(null, request("/leagues"))).toBe(true);
});
it("drops a router 404 whose referer header is not a URL", () => {
expect(
shouldReportServerError(
routeError(404, true),
request("/blog/", "not a url"),
),
).toBe(false);
});
});
/**
* The unit tests above use hand-written error objects. These drive real requests
* through React Router so the suite fails if the shape it throws ever changes.
*/
describe("shouldReportServerError against real React Router errors", () => {
const handler = createStaticHandler([
{
id: "root",
path: "/",
children: [{ id: "home", index: true, loader: () => null }],
},
]);
async function errorFor(req: Request) {
const ctx = await handler.query(req);
if (ctx instanceof Response) return null;
return Object.values(ctx.errors ?? {})[0] ?? null;
}
it('drops the 404 for an unmatched URL (No route matches URL "...")', async () => {
const req = request("/blog/wp/v2/posts/999999");
const error = await errorFor(req);
expect(error).toMatchObject({ status: 404, internal: true });
expect(shouldReportServerError(error, req)).toBe(false);
});
it("reports the same 404 when it came from a link on our own site", async () => {
const req = request("/nope", `${ORIGIN}/leagues`);
expect(shouldReportServerError(await errorFor(req), req)).toBe(true);
});
it("drops the 405 from a POST to a route with no action", async () => {
const req = request("/", undefined, "POST");
const error = await errorFor(req);
expect(error).toMatchObject({ status: 405, internal: true });
expect(shouldReportServerError(error, req)).toBe(false);
});
});
describe("static asset 404s", () => {
it("drops a stale hashed bundle even with a same-host referer", () => {
// Every deploy leaves clients requesting the previous build's assets.
const req = request("/assets/index-OLDHASH.js", `${ORIGIN}/leagues`);
expect(shouldReportServerError(routeError(404, true), req)).toBe(false);
});
it.each([
"/assets/app-x1.css",
"/fonts/inter.woff2",
"/images/logo.png",
"/favicon.ico",
])("drops a 404 for %s", (path) => {
expect(
shouldReportServerError(
routeError(404, true),
request(path, `${ORIGIN}/`),
),
).toBe(false);
});
it("still follows the referer rule for a non-asset path containing a dot", () => {
expect(
shouldReportServerError(
routeError(404, true),
request("/leagues/v1.2", `${ORIGIN}/leagues`),
),
).toBe(true);
expect(
shouldReportServerError(routeError(404, true), request("/leagues/v1.2")),
).toBe(false);
});
});
describe("React Router internal statuses that are not 404/405", () => {
it("reports an internal 400 (route is missing a loader)", () => {
expect(
shouldReportServerError(
routeError(400, true, "Bad Request"),
request("/leagues"),
),
).toBe(true);
});
it("reports an internal 403 (route does not match URL)", () => {
expect(
shouldReportServerError(
routeError(403, true, "Forbidden"),
request("/leagues"),
),
).toBe(true);
});
});
describe("production shape: TLS terminated upstream", () => {
it("reports a 404 linked from our own site when the proxy strips https", () => {
// Express builds request.url from req.protocol, which is `http` inside the
// container. Real browsers send an https referer. Comparing full origins
// would never match, silencing every broken internal link.
const req = new Request("http://brackt.com/leagues/gone", {
headers: { referer: "https://brackt.com/leagues" },
});
expect(shouldReportServerError(routeError(404, true), req)).toBe(true);
});
it("still drops a cold scanner hit under that same shape", () => {
const req = new Request("http://brackt.com/blog/wp/v2/posts/999999");
expect(shouldReportServerError(routeError(404, true), req)).toBe(false);
});
it("still drops a 404 linked from another site under that same shape", () => {
const req = new Request("http://brackt.com/nope", {
headers: { referer: "https://evil.example/" },
});
expect(shouldReportServerError(routeError(404, true), req)).toBe(false);
});
});