From 1c064fa684ec8abc744614343b4abf20bbbc7c80 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 23 Mar 2026 13:43:34 +0000 Subject: [PATCH] fix: skip response cache for unparseable URLs instead of crashing (CLI-GC) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getCachedResponse() and storeCachedResponse() called buildCacheKey() (which uses `new URL()`) outside their try-catch guards. On self-hosted instances with malformed base URLs this threw TypeError: Invalid URL, crashing the entire command. Fix: guard the buildCacheKey() call in both functions. When the URL is unparseable, skip caching and let the request proceed — fetch() itself will surface the real error if the URL is truly broken. normalizeUrl() still throws on invalid URLs (correct behavior) — the callers are now responsible for handling failures gracefully since caching is non-essential infrastructure. Fixes CLI-GC (3 events, 1 user on self-hosted). --- src/lib/response-cache.ts | 19 ++++++++++-- test/lib/response-cache.test.ts | 53 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/lib/response-cache.ts b/src/lib/response-cache.ts index 2d3dae100..f2dafaf90 100644 --- a/src/lib/response-cache.ts +++ b/src/lib/response-cache.ts @@ -123,6 +123,7 @@ export function buildCacheKey(method: string, url: string): string { * Normalize method + URL into a stable string for cache key derivation. * Sorts query params alphabetically for deterministic key generation. * + * @throws {TypeError} If the URL cannot be parsed * @internal Exported for testing */ export function normalizeUrl(method: string, url: string): string { @@ -367,7 +368,15 @@ export async function getCachedResponse( return; } - const key = buildCacheKey(method, url); + let key: string; + try { + key = buildCacheKey(method, url); + } catch { + // Malformed URL (e.g., self-hosted with bad base URL) — skip cache lookup. + // The request will proceed without caching; fetch() itself will surface + // the real error if the URL is truly broken. + return; + } return await withCacheSpan( url, @@ -467,7 +476,13 @@ export async function storeCachedResponse( return; } - const key = buildCacheKey(method, url); + let key: string; + try { + key = buildCacheKey(method, url); + } catch { + // Malformed URL — skip caching this response + return; + } try { await withCacheSpan( diff --git a/test/lib/response-cache.test.ts b/test/lib/response-cache.test.ts index b25f6ea1f..b17fbb378 100644 --- a/test/lib/response-cache.test.ts +++ b/test/lib/response-cache.test.ts @@ -12,6 +12,7 @@ import { buildCacheKey, clearResponseCache, getCachedResponse, + normalizeUrl, resetCacheState, storeCachedResponse, } from "../../src/lib/response-cache.js"; @@ -263,6 +264,37 @@ describe("cache bypass", () => { }); }); +// --------------------------------------------------------------------------- +// normalizeUrl +// --------------------------------------------------------------------------- + +describe("normalizeUrl", () => { + test("sorts query params alphabetically", () => { + const result = normalizeUrl( + "GET", + "https://sentry.io/api/0/issues/?b=2&a=1" + ); + expect(result).toBe("GET|https://sentry.io/api/0/issues/?a=1&b=2"); + }); + + test("uppercases the method", () => { + const result = normalizeUrl("get", "https://sentry.io/api/0/issues/"); + expect(result).toMatch(/^GET\|/); + }); + + test("throws on invalid URLs", () => { + expect(() => normalizeUrl("GET", "not-a-valid-url")).toThrow(); + }); + + test("handles self-hosted URLs with unusual schemes", () => { + const result = normalizeUrl( + "GET", + "https://sentry.mycompany.internal/api/0/issues/" + ); + expect(result).toBe("GET|https://sentry.mycompany.internal/api/0/issues/"); + }); +}); + // --------------------------------------------------------------------------- // buildCacheKey // --------------------------------------------------------------------------- @@ -286,6 +318,27 @@ describe("buildCacheKey", () => { }); }); +// --------------------------------------------------------------------------- +// Invalid URL handling (CLI-GC) +// --------------------------------------------------------------------------- + +describe("invalid URL handling", () => { + test("getCachedResponse skips cache for malformed URLs", async () => { + const result = await getCachedResponse("GET", "not-a-valid-url", {}); + expect(result).toBeUndefined(); + }); + + test("storeCachedResponse skips cache for malformed URLs", async () => { + // Should not throw — just silently skip + await storeCachedResponse( + "GET", + "not-a-valid-url", + {}, + mockResponse({ ok: true }) + ); + }); +}); + // --------------------------------------------------------------------------- // No-cache tier (polling endpoints) // ---------------------------------------------------------------------------