Skip to content

Commit 7db4f7c

Browse files
authored
fix: handle invalid URLs gracefully in response cache (CLI-GC) (#528)
## Summary Fixes a crash in the response cache layer when self-hosted Sentry instances have malformed base URLs. ### Bug (CLI-GC — 3 events, 1 user) `getCachedResponse()` and `storeCachedResponse()` both call `buildCacheKey()` — which internally runs `new URL(url)` — **outside** their try-catch guards. On self-hosted instances with malformed base URLs, this throws `TypeError: Invalid URL`, crashing the entire command before the actual API request even fires. ### Fix Guard the `buildCacheKey()` call in both functions. When the URL can't be parsed: - **Cache lookup**: skip and return `undefined` (treat as cache miss) - **Cache store**: skip silently (response was already returned to the caller) The actual `fetch()` call will surface the real error if the URL is truly broken — the cache layer shouldn't be the one to crash. `normalizeUrl()` itself **still throws** on invalid URLs (correct behavior for a low-level utility) — the callers handle the failure since caching is non-essential infrastructure. ### Tests added - `normalizeUrl()` throws on invalid URL (verifies it doesn't silently swallow) - `getCachedResponse()` returns undefined for malformed URLs (no crash) - `storeCachedResponse()` completes without throwing for malformed URLs
1 parent bc6c5fc commit 7db4f7c

File tree

2 files changed

+70
-2
lines changed

2 files changed

+70
-2
lines changed

src/lib/response-cache.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ export function buildCacheKey(method: string, url: string): string {
123123
* Normalize method + URL into a stable string for cache key derivation.
124124
* Sorts query params alphabetically for deterministic key generation.
125125
*
126+
* @throws {TypeError} If the URL cannot be parsed
126127
* @internal Exported for testing
127128
*/
128129
export function normalizeUrl(method: string, url: string): string {
@@ -367,7 +368,15 @@ export async function getCachedResponse(
367368
return;
368369
}
369370

370-
const key = buildCacheKey(method, url);
371+
let key: string;
372+
try {
373+
key = buildCacheKey(method, url);
374+
} catch {
375+
// Malformed URL (e.g., self-hosted with bad base URL) — skip cache lookup.
376+
// The request will proceed without caching; fetch() itself will surface
377+
// the real error if the URL is truly broken.
378+
return;
379+
}
371380

372381
return await withCacheSpan(
373382
url,
@@ -467,7 +476,13 @@ export async function storeCachedResponse(
467476
return;
468477
}
469478

470-
const key = buildCacheKey(method, url);
479+
let key: string;
480+
try {
481+
key = buildCacheKey(method, url);
482+
} catch {
483+
// Malformed URL — skip caching this response
484+
return;
485+
}
471486

472487
try {
473488
await withCacheSpan(

test/lib/response-cache.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
buildCacheKey,
1313
clearResponseCache,
1414
getCachedResponse,
15+
normalizeUrl,
1516
resetCacheState,
1617
storeCachedResponse,
1718
} from "../../src/lib/response-cache.js";
@@ -263,6 +264,37 @@ describe("cache bypass", () => {
263264
});
264265
});
265266

267+
// ---------------------------------------------------------------------------
268+
// normalizeUrl
269+
// ---------------------------------------------------------------------------
270+
271+
describe("normalizeUrl", () => {
272+
test("sorts query params alphabetically", () => {
273+
const result = normalizeUrl(
274+
"GET",
275+
"https://sentry.io/api/0/issues/?b=2&a=1"
276+
);
277+
expect(result).toBe("GET|https://sentry.io/api/0/issues/?a=1&b=2");
278+
});
279+
280+
test("uppercases the method", () => {
281+
const result = normalizeUrl("get", "https://sentry.io/api/0/issues/");
282+
expect(result).toMatch(/^GET\|/);
283+
});
284+
285+
test("throws on invalid URLs", () => {
286+
expect(() => normalizeUrl("GET", "not-a-valid-url")).toThrow();
287+
});
288+
289+
test("handles self-hosted URLs with unusual schemes", () => {
290+
const result = normalizeUrl(
291+
"GET",
292+
"https://sentry.mycompany.internal/api/0/issues/"
293+
);
294+
expect(result).toBe("GET|https://sentry.mycompany.internal/api/0/issues/");
295+
});
296+
});
297+
266298
// ---------------------------------------------------------------------------
267299
// buildCacheKey
268300
// ---------------------------------------------------------------------------
@@ -286,6 +318,27 @@ describe("buildCacheKey", () => {
286318
});
287319
});
288320

321+
// ---------------------------------------------------------------------------
322+
// Invalid URL handling (CLI-GC)
323+
// ---------------------------------------------------------------------------
324+
325+
describe("invalid URL handling", () => {
326+
test("getCachedResponse skips cache for malformed URLs", async () => {
327+
const result = await getCachedResponse("GET", "not-a-valid-url", {});
328+
expect(result).toBeUndefined();
329+
});
330+
331+
test("storeCachedResponse skips cache for malformed URLs", async () => {
332+
// Should not throw — just silently skip
333+
await storeCachedResponse(
334+
"GET",
335+
"not-a-valid-url",
336+
{},
337+
mockResponse({ ok: true })
338+
);
339+
});
340+
});
341+
289342
// ---------------------------------------------------------------------------
290343
// No-cache tier (polling endpoints)
291344
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)