From 32c29d1bf8cc9cccf6aa63a8ffef1b07327bb03e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:54:48 +0000 Subject: [PATCH 1/2] ref(toolkit-core): Share the response cache policy with the MCP Moves the CLI's cache policy into `@sentry/toolkit-core/response-cache` so the MCP can use the same rules (task 4.1 of the unify-sentry-api-layer proposal, #1669): - `classifyUrl` and `FALLBACK_TTL_MS`: the volatile/stable/no-cache tiers for responses without server cache headers. - `normalizeUrl` and `buildCacheKey`: identity-scoped keys over the method and full URL. Key derivation now uses WebCrypto HMAC-SHA256 instead of `node:crypto`, so it runs in Workers and browsers. The output is byte-identical, so existing CLI cache files still hit, and an empty identity is rejected instead of producing an unscoped key. - `computeInvalidationPrefixes`: moved unchanged from `cli/src/lib/cache-keys.ts`, with its tests. The module has no storage or I/O. The CLI's disk cache imports it, and its `buildCacheKey` wrapper (which defaults to the active identity) is now async, as are the tests that call it. WebCrypto HMAC runs on the libuv threadpool, so parallel requests can finish their cache lookups in either order. The `getLogs` batching test no longer assumes which batch reaches fetch first. Co-Authored-By: BYK --- packages/cli/src/lib/response-cache.ts | 134 +++--------------- packages/cli/src/lib/sentry-client.ts | 2 +- packages/cli/test/lib/api-client.test.ts | 19 ++- .../test/lib/response-cache.property.test.ts | 58 ++++---- packages/cli/test/lib/response-cache.test.ts | 24 ++-- packages/toolkit-core/README.md | 19 +++ packages/toolkit-core/package.json | 4 + .../toolkit-core/src/response-cache/index.ts | 9 ++ .../src/response-cache/invalidation.test.ts} | 2 +- .../src/response-cache/invalidation.ts} | 21 ++- .../src/response-cache/keys.test.ts | 63 ++++++++ .../toolkit-core/src/response-cache/keys.ts | 68 +++++++++ .../src/response-cache/tiers.test.ts | 73 ++++++++++ .../toolkit-core/src/response-cache/tiers.ts | 71 ++++++++++ 14 files changed, 389 insertions(+), 178 deletions(-) create mode 100644 packages/toolkit-core/src/response-cache/index.ts rename packages/{cli/test/lib/cache-keys.test.ts => toolkit-core/src/response-cache/invalidation.test.ts} (99%) rename packages/{cli/src/lib/cache-keys.ts => toolkit-core/src/response-cache/invalidation.ts} (88%) create mode 100644 packages/toolkit-core/src/response-cache/keys.test.ts create mode 100644 packages/toolkit-core/src/response-cache/keys.ts create mode 100644 packages/toolkit-core/src/response-cache/tiers.test.ts create mode 100644 packages/toolkit-core/src/response-cache/tiers.ts diff --git a/packages/cli/src/lib/response-cache.ts b/packages/cli/src/lib/response-cache.ts index 2ada16ce9..cc00534ac 100644 --- a/packages/cli/src/lib/response-cache.ts +++ b/packages/cli/src/lib/response-cache.ts @@ -14,7 +14,7 @@ * @module */ -import { createHmac, randomUUID } from "node:crypto"; +import { randomUUID } from "node:crypto"; import { mkdir, readdir, @@ -26,6 +26,11 @@ import { writeFile, } from "node:fs/promises"; import { join } from "node:path"; +import { + buildCacheKey as buildIdentityCacheKey, + classifyUrl, + FALLBACK_TTL_MS, +} from "@sentry/toolkit-core/response-cache"; import CachePolicy from "http-cache-semantics"; import pLimit from "p-limit"; @@ -39,127 +44,20 @@ import { recordCacheHit, withCacheSpan } from "./telemetry.js"; const log = logger.withTag("response-cache"); // --------------------------------------------------------------------------- -// TTL tiers — used as fallback when the server sends no cache headers -// --------------------------------------------------------------------------- - -/** - * TTL tier classification for URLs. - * - * - `stable`: data that changes infrequently (orgs, projects, teams, - * organization event details, trace-item details) - * - `volatile`: data that changes often (issues and their events, project - * event details, full traces, log and span queries) - * - `no-cache`: never cache (polling endpoints like autofix state) - * - * No tier treats a response as immutable. Event payloads embed mutable - * fields (`nextEventID`, user reports, release data), `latest`/`oldest`/ - * `recommended` resolve to different events over time, and traces gain - * spans as they arrive (getsentry/sentry#128818). - */ -type TtlTier = "stable" | "volatile" | "no-cache"; - -/** Fallback TTL durations by tier (milliseconds). `no-cache` uses 0 as a sentinel. */ -const FALLBACK_TTL_MS: Record = { - stable: 5 * 60 * 1000, // 5 minutes - volatile: 60 * 1000, // 60 seconds - "no-cache": 0, -}; - -/** - * URL patterns grouped by TTL tier. - * - * Checked in tier priority order (no-cache → volatile). - * "stable" has no patterns — it is the default fallback when nothing else matches. - * - * TODO: Classify by generated operation metadata from `@sentry/api` instead - * of URL regexes, once it ships per-operation metadata - * (getsentry/sentry-api-schema#81) and a route-keyed operation map - * (getsentry/sentry-api-schema#102). - */ -const URL_TIER_REGEXPS: Readonly> = { - // Polling endpoints where state changes rapidly - "no-cache": [/\/(?:autofix|root-cause)\//], - volatile: [ - // Issue endpoints: lists, detail views, and issue events such as - // `/issues/{id}/events/latest/` - /\/issues\//, - /[?&]dataset=(?:logs|spans)/, - /\/trace-logs\//, - // Full traces gain spans as they arrive - /\/trace\/[0-9a-f]{32}\//, - // Project event details embed `nextEventID`/`previousEventID` and user - // reports, and reprocessing can change them for the same ID - /\/projects\/[^/]+\/[^/]+\/events\/[^/?]+\/?(?:\?|$)/, - ], - // Default fallback — no patterns needed - stable: [], -}; - -/** Tier check order — stable is the default and has no patterns to check. */ -const TIER_CHECK_ORDER: readonly TtlTier[] = ["no-cache", "volatile"]; - -/** - * Classify a URL into a TTL tier for fallback caching. - * - * @param url - Full URL string (with query params) - * @returns The TTL tier - * @internal Exported for testing - */ -export function classifyUrl(url: string): TtlTier { - for (const tier of TIER_CHECK_ORDER) { - for (const pattern of URL_TIER_REGEXPS[tier]) { - if (pattern.test(url)) { - return tier; - } - } - } - return "stable"; -} - -// --------------------------------------------------------------------------- -// Cache key generation +// Cache keys — tiers, keys, and invalidation prefixes live in toolkit-core // --------------------------------------------------------------------------- /** - * Build a deterministic cache key from the active identity + method + URL. - * - * Query params are sorted alphabetically so `?a=1&b=2` and `?b=2&a=1` - * produce the same key. The identity fingerprint scopes entries per - * bearer token so switching accounts never serves another user's data. + * Build the cache key for the active identity + method + URL. * * @internal Exported for testing */ -export function buildCacheKey( +export async function buildCacheKey( method: string, url: string, identity = getIdentityFingerprint(), -): string { - const normalized = normalizeUrl(method, url); - return createHmac("sha256", identity).update(normalized).digest("hex"); -} - -/** - * 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 { - const parsed = new URL(url); - const sortedParams = new URLSearchParams( - [...parsed.searchParams.entries()].sort(([a], [b]) => { - if (a < b) { - return -1; - } - if (a > b) { - return 1; - } - return 0; - }), - ); - parsed.search = sortedParams.toString() ? `?${sortedParams.toString()}` : ""; - return `${method.toUpperCase()}|${parsed.toString()}`; +): Promise { + return await buildIdentityCacheKey(method, url, identity); } // --------------------------------------------------------------------------- @@ -451,10 +349,10 @@ export async function getCachedResponse( let key: string; // oxlint-disable-next-line sentry-cli/no-silent-catch -- grandfathered silent catch — see #1531; drain by adding log.debug()/log.warn() or re-throwing. try { - key = buildCacheKey(method, url, identity); + key = await buildCacheKey(method, url, identity); } catch { - // Malformed URL — skip cache lookup. The request itself will surface - // any real URL error. + // Malformed URL or empty identity — skip cache lookup. The request + // itself will surface any real URL error. return; } @@ -570,9 +468,9 @@ export async function storeCachedResponse( let key: string; // oxlint-disable-next-line sentry-cli/no-silent-catch -- grandfathered silent catch — see #1531; drain by adding log.debug()/log.warn() or re-throwing. try { - key = buildCacheKey(method, url, identity); + key = await buildCacheKey(method, url, identity); } catch { - // Malformed URL — skip caching this response + // Malformed URL or empty identity — skip caching this response return; } diff --git a/packages/cli/src/lib/sentry-client.ts b/packages/cli/src/lib/sentry-client.ts index 6ef6b2198..045cb20a8 100644 --- a/packages/cli/src/lib/sentry-client.ts +++ b/packages/cli/src/lib/sentry-client.ts @@ -11,13 +11,13 @@ import { setTimeout as sleepMs } from "node:timers/promises"; import type { SentryClient } from "@sentry/api"; import { getTraceData } from "@sentry/node-core/light"; +import { computeInvalidationPrefixes } from "@sentry/toolkit-core/response-cache"; import { formatAuthHeader, normalizeAuthToken, trimAuthToken, } from "./auth-header.js"; import { maybeWarnEnvTokenIgnored } from "./auth-hint.js"; -import { computeInvalidationPrefixes } from "./cache-keys.js"; import { DEFAULT_SENTRY_URL, getConfiguredSentryUrl, diff --git a/packages/cli/test/lib/api-client.test.ts b/packages/cli/test/lib/api-client.test.ts index 2f103a1fe..9e2e7ce51 100644 --- a/packages/cli/test/lib/api-client.test.ts +++ b/packages/cli/test/lib/api-client.test.ts @@ -1884,16 +1884,15 @@ describe("getLogs", () => { const result = await getLogs("my-org", "my-project", ids); - // Should have made 2 separate requests - expect(capturedUrls).toHaveLength(2); - - // First batch: 100 IDs - const url1 = new URL(capturedUrls[0]); - expect(url1.searchParams.get("per_page")).toBe(String(API_MAX_PER_PAGE)); - - // Second batch: 50 IDs - const url2 = new URL(capturedUrls[1]); - expect(url2.searchParams.get("per_page")).toBe("50"); + // One request of 100 IDs and one of 50. The batches run in parallel, so + // they can reach fetch in either order. + const perPage = capturedUrls.map((url) => + new URL(url).searchParams.get("per_page"), + ); + expect(perPage).toHaveLength(2); + expect(perPage).toEqual( + expect.arrayContaining([String(API_MAX_PER_PAGE), "50"]), + ); // Results from both batches should be flattened expect(result).toHaveLength(2); diff --git a/packages/cli/test/lib/response-cache.property.test.ts b/packages/cli/test/lib/response-cache.property.test.ts index c649b35a4..b393fc5b0 100644 --- a/packages/cli/test/lib/response-cache.property.test.ts +++ b/packages/cli/test/lib/response-cache.property.test.ts @@ -7,18 +7,16 @@ import { array, + asyncProperty, constantFrom, assert as fcAssert, property, string, tuple, } from "fast-check"; +import { classifyUrl, normalizeUrl } from "@sentry/toolkit-core/response-cache"; import { describe, expect, test } from "vitest"; -import { - buildCacheKey, - classifyUrl, - normalizeUrl, -} from "../../src/lib/response-cache.js"; +import { buildCacheKey } from "../../src/lib/response-cache.js"; import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; // --------------------------------------------------------------------------- @@ -66,52 +64,60 @@ const sentryUrlArb = tuple( // --------------------------------------------------------------------------- describe("property: buildCacheKey", () => { - test("produces a 64-char hex string (SHA-256)", () => { - fcAssert( - property(methodArb, sentryUrlArb, (method, url) => { - expect(buildCacheKey(method, url)).toMatch(/^[0-9a-f]{64}$/); + test("produces a 64-char hex string (SHA-256)", async () => { + await fcAssert( + asyncProperty(methodArb, sentryUrlArb, async (method, url) => { + expect(await buildCacheKey(method, url)).toMatch(/^[0-9a-f]{64}$/); }), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("is deterministic — same inputs produce same key", () => { - fcAssert( - property(methodArb, sentryUrlArb, (method, url) => { - expect(buildCacheKey(method, url)).toBe(buildCacheKey(method, url)); + test("is deterministic — same inputs produce same key", async () => { + await fcAssert( + asyncProperty(methodArb, sentryUrlArb, async (method, url) => { + expect(await buildCacheKey(method, url)).toBe( + await buildCacheKey(method, url), + ); }), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("different methods produce different keys for same URL", () => { - fcAssert( - property(sentryUrlArb, (url) => { - expect(buildCacheKey("GET", url)).not.toBe(buildCacheKey("POST", url)); + test("different methods produce different keys for same URL", async () => { + await fcAssert( + asyncProperty(sentryUrlArb, async (url) => { + expect(await buildCacheKey("GET", url)).not.toBe( + await buildCacheKey("POST", url), + ); }), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("query param order does not affect the key", () => { - fcAssert( - property( + test("query param order does not affect the key", async () => { + await fcAssert( + asyncProperty( constantFrom("https://us.sentry.io", "https://de.sentry.io"), pathSegmentArb, - (base, path) => { + async (base, path) => { const url1 = `${base}/api/0/${path}?a=1&b=2&c=3`; const url2 = `${base}/api/0/${path}?c=3&a=1&b=2`; - expect(buildCacheKey("GET", url1)).toBe(buildCacheKey("GET", url2)); + expect(await buildCacheKey("GET", url1)).toBe( + await buildCacheKey("GET", url2), + ); }, ), { numRuns: DEFAULT_NUM_RUNS }, ); }); - test("method comparison is case-insensitive", () => { - fcAssert( - property(sentryUrlArb, (url) => { - expect(buildCacheKey("get", url)).toBe(buildCacheKey("GET", url)); + test("method comparison is case-insensitive", async () => { + await fcAssert( + asyncProperty(sentryUrlArb, async (url) => { + expect(await buildCacheKey("get", url)).toBe( + await buildCacheKey("GET", url), + ); }), { numRuns: DEFAULT_NUM_RUNS }, ); diff --git a/packages/cli/test/lib/response-cache.test.ts b/packages/cli/test/lib/response-cache.test.ts index 23022c861..30b99d193 100644 --- a/packages/cli/test/lib/response-cache.test.ts +++ b/packages/cli/test/lib/response-cache.test.ts @@ -7,6 +7,7 @@ import { readdir } from "node:fs/promises"; import { join } from "node:path"; +import { normalizeUrl } from "@sentry/toolkit-core/response-cache"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { setAuthToken } from "../../src/lib/db/auth.js"; import { @@ -15,7 +16,6 @@ import { disableResponseCache, getCachedResponse, invalidateCachedResponsesMatching, - normalizeUrl, resetCacheState, storeCachedResponse, } from "../../src/lib/response-cache.js"; @@ -372,27 +372,29 @@ describe("normalizeUrl", () => { // --------------------------------------------------------------------------- describe("buildCacheKey", () => { - test("produces a 64-char hex string", () => { - expect(buildCacheKey("GET", TEST_URL)).toMatch(/^[0-9a-f]{64}$/); + test("produces a 64-char hex string", async () => { + expect(await buildCacheKey("GET", TEST_URL)).toMatch(/^[0-9a-f]{64}$/); }); - test("is deterministic", () => { - expect(buildCacheKey("GET", TEST_URL)).toBe(buildCacheKey("GET", TEST_URL)); + test("is deterministic", async () => { + expect(await buildCacheKey("GET", TEST_URL)).toBe( + await buildCacheKey("GET", TEST_URL), + ); }); - test("different methods produce different keys", () => { - expect(buildCacheKey("GET", TEST_URL)).not.toBe( - buildCacheKey("POST", TEST_URL), + test("different methods produce different keys", async () => { + expect(await buildCacheKey("GET", TEST_URL)).not.toBe( + await buildCacheKey("POST", TEST_URL), ); }); - test("different identities produce different keys for the same URL", () => { + test("different identities produce different keys for the same URL", async () => { // Switching accounts must route reads/writes through a different // namespace so users never see each other's cached data. setAuthToken("alice_access", 3600, "alice_refresh"); - const aliceKey = buildCacheKey("GET", TEST_URL); + const aliceKey = await buildCacheKey("GET", TEST_URL); setAuthToken("bob_access", 3600, "bob_refresh"); - const bobKey = buildCacheKey("GET", TEST_URL); + const bobKey = await buildCacheKey("GET", TEST_URL); expect(aliceKey).not.toBe(bobKey); }); }); diff --git a/packages/toolkit-core/README.md b/packages/toolkit-core/README.md index 72fb2905d..539b2ed60 100644 --- a/packages/toolkit-core/README.md +++ b/packages/toolkit-core/README.md @@ -32,6 +32,25 @@ the page cap, receives warnings, and turns failures into its own errors. The CLI's `paginate` and `fetchAllPages` in `packages/cli/src/lib/api/infrastructure.ts` are thin wrappers over it. +## Response cache + +`response-cache` holds the cache policy for Sentry API GET responses. +`classifyUrl` puts a URL in a fallback TTL tier for responses without HTTP +caching headers: `volatile` (60 seconds) for issues, project event details, +full traces, and log and span queries; `no-cache` for autofix and root-cause +polling; and `stable` (5 minutes) for everything else. No tier keeps a +response longer than 5 minutes. `buildCacheKey` derives a key from the +requesting identity, the method, and the full URL with sorted query params, +as a WebCrypto HMAC-SHA256 that matches the CLI's earlier `node:crypto` keys. +`computeInvalidationPrefixes` lists the URL prefixes a successful mutation +makes stale. + +The module has no storage. The host supplies an identity that is a stable +hash of the login (never a raw token), applies server caching headers ahead +of the tiers, stores entries, and deletes only the requesting identity's +entries under each invalidation prefix. The CLI's disk cache in +`packages/cli/src/lib/response-cache.ts` uses it. + ## Resource identity The `resource-identity` types describe the identifiers shared by organization, diff --git a/packages/toolkit-core/package.json b/packages/toolkit-core/package.json index 3405dd814..8ab356d6a 100644 --- a/packages/toolkit-core/package.json +++ b/packages/toolkit-core/package.json @@ -55,6 +55,10 @@ "types": "./src/resource-identity.ts", "default": "./src/resource-identity.ts" }, + "./response-cache": { + "types": "./src/response-cache/index.ts", + "default": "./src/response-cache/index.ts" + }, "./search-query": { "types": "./src/search-query.ts", "default": "./src/search-query.ts" diff --git a/packages/toolkit-core/src/response-cache/index.ts b/packages/toolkit-core/src/response-cache/index.ts new file mode 100644 index 000000000..a99661660 --- /dev/null +++ b/packages/toolkit-core/src/response-cache/index.ts @@ -0,0 +1,9 @@ +/** + * Response cache policy shared by the CLI and MCP: fallback TTL tiers, + * identity-scoped keys, and mutation invalidation prefixes. Pure logic with + * no storage or I/O; each host owns where entries live. + */ + +export { computeInvalidationPrefixes } from "./invalidation.js"; +export { buildCacheKey, normalizeUrl } from "./keys.js"; +export { classifyUrl, FALLBACK_TTL_MS, type TtlTier } from "./tiers.js"; diff --git a/packages/cli/test/lib/cache-keys.test.ts b/packages/toolkit-core/src/response-cache/invalidation.test.ts similarity index 99% rename from packages/cli/test/lib/cache-keys.test.ts rename to packages/toolkit-core/src/response-cache/invalidation.test.ts index ccb264f25..bfa4cd595 100644 --- a/packages/cli/test/lib/cache-keys.test.ts +++ b/packages/toolkit-core/src/response-cache/invalidation.test.ts @@ -3,7 +3,7 @@ */ import { describe, expect, test } from "vitest"; -import { computeInvalidationPrefixes as computeInvalidationPrefixesRaw } from "../../src/lib/cache-keys.js"; +import { computeInvalidationPrefixes as computeInvalidationPrefixesRaw } from "./invalidation.js"; const BASE = "https://us.sentry.io/api/0/"; const API_BASE_URL = "https://sentry.io"; diff --git a/packages/cli/src/lib/cache-keys.ts b/packages/toolkit-core/src/response-cache/invalidation.ts similarity index 88% rename from packages/cli/src/lib/cache-keys.ts rename to packages/toolkit-core/src/response-cache/invalidation.ts index c6a021221..074cefdd0 100644 --- a/packages/cli/src/lib/cache-keys.ts +++ b/packages/toolkit-core/src/response-cache/invalidation.ts @@ -1,9 +1,10 @@ /** * Compute cache-invalidation prefixes for a mutation URL. * - * The HTTP layer calls {@link computeInvalidationPrefixes} after every - * successful non-GET request and feeds the result into - * `invalidateCachedResponsesMatching`. Two rules apply: + * The host's HTTP layer calls {@link computeInvalidationPrefixes} after + * every successful non-GET request and deletes the requesting identity's + * cached entries whose URL starts with any returned prefix. Two rules + * apply: * * 1. **Hierarchy walk.** Sweep the URL's own path and every ancestor * up to `/api/0/`. A mutation on @@ -19,9 +20,8 @@ * (2 rules) and only grows when a new cross-tree relationship * appears in the API surface. * - * Prefixes are identity-scoped at the sweep layer - * (`invalidateCachedResponsesMatching` checks `entry.identity`), so a - * slightly broader sweep is safe — it can only touch the current + * Prefixes are identity-scoped at the sweep layer (the store checks each + * entry's identity before deleting it), so a slightly broader sweep is safe — it can only touch the current * identity's entries. Query strings on the mutation URL are dropped * from the prefix (a prefix sweep on the path naturally catches every * query-param variant cached under that path). @@ -91,11 +91,10 @@ const CROSS_ENDPOINT_RULES: CrossEndpointRule[] = [ * * @param fullUrl - Fully-qualified URL of the mutation (absolute, * including base). Query string is ignored. - * @param apiBaseUrl - The CLI's non-region API base URL (used by rules + * @param apiBaseUrl - The host's non-region API base URL (used by rules * that need to clear caches under a different origin — e.g. the - * legacy `/issues/{id}/` endpoint that `getIssue()` hits). - * @returns Array of full-URL prefixes ready to pass to - * `invalidateCachedResponsesMatching`. Returns `[]` if the URL is + * legacy `/issues/{id}/` endpoint the CLI's `getIssue()` reads). + * @returns Full-URL prefixes to sweep. Returns `[]` if the URL is * not under `/api/0/` (e.g. sourcemap chunk upload to an arbitrary * endpoint) or can't be parsed. */ @@ -104,10 +103,10 @@ export function computeInvalidationPrefixes( apiBaseUrl: string, ): string[] { let parsed: URL; - // oxlint-disable-next-line sentry-cli/no-silent-catch -- grandfathered silent catch — see #1531; drain by adding log.debug()/log.warn() or re-throwing. try { parsed = new URL(fullUrl); } catch { + // Not a URL, so nothing cached under it. return []; } diff --git a/packages/toolkit-core/src/response-cache/keys.test.ts b/packages/toolkit-core/src/response-cache/keys.test.ts new file mode 100644 index 000000000..dc1cc74fe --- /dev/null +++ b/packages/toolkit-core/src/response-cache/keys.test.ts @@ -0,0 +1,63 @@ +import { createHmac } from "node:crypto"; +import { describe, expect, it } from "vitest"; +import { buildCacheKey, normalizeUrl } from "./keys.js"; + +const URL_A = "https://us.sentry.io/api/0/organizations/acme/projects/?b=2&a=1"; + +describe("normalizeUrl", () => { + it("uppercases the method and sorts query params, keeping the host", () => { + expect(normalizeUrl("get", URL_A)).toBe( + "GET|https://us.sentry.io/api/0/organizations/acme/projects/?a=1&b=2", + ); + }); + + it("throws on a URL it cannot parse", () => { + expect(() => normalizeUrl("GET", "not a url")).toThrow(TypeError); + }); +}); + +describe("buildCacheKey", () => { + it("gives two identities different keys for the same URL", async () => { + expect(await buildCacheKey("GET", URL_A, "alice")).not.toBe( + await buildCacheKey("GET", URL_A, "bob"), + ); + }); + + it("gives the same path on two Sentry hosts different keys", async () => { + const path = "/api/0/organizations/acme/projects/"; + expect( + await buildCacheKey("GET", `https://us.sentry.io${path}`, "alice"), + ).not.toBe( + await buildCacheKey("GET", `https://sentry.example.com${path}`, "alice"), + ); + }); + + it("ignores query param order and method case", async () => { + expect( + await buildCacheKey( + "get", + "https://us.sentry.io/api/0/issues/?a=1&b=2", + "alice", + ), + ).toBe( + await buildCacheKey( + "GET", + "https://us.sentry.io/api/0/issues/?b=2&a=1", + "alice", + ), + ); + }); + + it("matches node:crypto HMAC-SHA256, so existing CLI cache entries still hit", async () => { + const identity = "oauth:3f2a9c"; + const expected = createHmac("sha256", identity) + .update(normalizeUrl("GET", URL_A)) + .digest("hex"); + + expect(await buildCacheKey("GET", URL_A, identity)).toBe(expected); + }); + + it("refuses an empty identity, which would not scope entries", async () => { + await expect(buildCacheKey("GET", URL_A, "")).rejects.toThrow(TypeError); + }); +}); diff --git a/packages/toolkit-core/src/response-cache/keys.ts b/packages/toolkit-core/src/response-cache/keys.ts new file mode 100644 index 000000000..be81411b7 --- /dev/null +++ b/packages/toolkit-core/src/response-cache/keys.ts @@ -0,0 +1,68 @@ +/** + * Identity-scoped cache keys for Sentry API responses. Uses WebCrypto, so it + * runs in Node, Cloudflare Workers, and browsers. + */ + +const encoder = new TextEncoder(); + +/** + * Normalize method + URL into a stable string for cache key derivation. + * Keeps the host, so the same path on two Sentry servers gets two keys, and + * sorts query params by name so `?a=1&b=2` and `?b=2&a=1` match. + * + * @throws {TypeError} If the URL cannot be parsed + */ +export function normalizeUrl(method: string, url: string): string { + const parsed = new URL(url); + const sortedParams = new URLSearchParams( + [...parsed.searchParams.entries()].sort(([a], [b]) => { + if (a < b) { + return -1; + } + if (a > b) { + return 1; + } + return 0; + }), + ); + parsed.search = sortedParams.toString() ? `?${sortedParams.toString()}` : ""; + return `${method.toUpperCase()}|${parsed.toString()}`; +} + +/** + * Build a cache key from the requesting identity, method, and URL: the + * hex HMAC-SHA256 of {@link normalizeUrl} keyed by `identity`. + * + * `identity` scopes entries so one user never receives another's response. + * It must be a stable hash of who is asking, never a raw token, and should + * not change when an access token is refreshed for the same login. Keys + * match `node:crypto`'s `createHmac("sha256", identity)`, so entries written + * before this module moved here are still found. + * + * @throws {TypeError} If `identity` is empty or the URL cannot be parsed + */ +export async function buildCacheKey( + method: string, + url: string, + identity: string, +): Promise { + if (!identity) { + throw new TypeError("A cache key needs a non-empty identity"); + } + const normalized = normalizeUrl(method, url); + const key = await crypto.subtle.importKey( + "raw", + encoder.encode(identity), + { name: "HMAC", hash: "SHA-256" }, + false, + ["sign"], + ); + const signature = await crypto.subtle.sign( + "HMAC", + key, + encoder.encode(normalized), + ); + return Array.from(new Uint8Array(signature), (byte) => + byte.toString(16).padStart(2, "0"), + ).join(""); +} diff --git a/packages/toolkit-core/src/response-cache/tiers.test.ts b/packages/toolkit-core/src/response-cache/tiers.test.ts new file mode 100644 index 000000000..2da56a084 --- /dev/null +++ b/packages/toolkit-core/src/response-cache/tiers.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { classifyUrl, FALLBACK_TTL_MS, type TtlTier } from "./tiers.js"; + +const API = "https://us.sentry.io/api/0"; +const TRACE_ID = "a".repeat(32); + +function ttlFor(url: string): number { + return FALLBACK_TTL_MS[classifyUrl(url)]; +} + +describe("classifyUrl", () => { + it("reuses organization event lookups for up to 5 minutes", () => { + expect(classifyUrl(`${API}/organizations/acme/events/web:deadbeef/`)).toBe( + "stable", + ); + expect(ttlFor(`${API}/organizations/acme/events/web:deadbeef/`)).toBe( + 5 * 60 * 1000, + ); + }); + + it("reuses issue event lookups for at most 60 seconds", () => { + for (const event of ["latest", "oldest", "recommended", "deadbeef"]) { + const url = `${API}/organizations/acme/issues/123/events/${event}/`; + expect(classifyUrl(url)).toBe("volatile"); + expect(ttlFor(url)).toBeLessThanOrEqual(60 * 1000); + } + }); + + it("reuses full traces for at most 60 seconds, since spans can still arrive", () => { + const url = `${API}/organizations/acme/trace/${TRACE_ID}/?limit=100`; + expect(classifyUrl(url)).toBe("volatile"); + expect(ttlFor(url)).toBeLessThanOrEqual(60 * 1000); + }); + + it("treats project event details, log queries, and span queries as volatile", () => { + for (const url of [ + `${API}/projects/acme/web/events/deadbeef/`, + `${API}/organizations/acme/events/?dataset=logs&query=foo`, + `${API}/organizations/acme/events/?field=id&dataset=spans`, + `${API}/organizations/acme/trace-logs/?traceId=${TRACE_ID}`, + ]) { + expect(classifyUrl(url)).toBe("volatile"); + } + }); + + it("never serves polling endpoints from cache", () => { + for (const url of [ + `${API}/organizations/acme/issues/123/autofix/`, + `${API}/organizations/acme/issues/123/root-cause/`, + ]) { + expect(classifyUrl(url)).toBe("no-cache"); + expect(ttlFor(url)).toBe(0); + } + }); + + it("defaults everything else to stable", () => { + for (const url of [ + `${API}/organizations/`, + `${API}/organizations/acme/projects/`, + `${API}/projects/acme/web/trace-items/abc123/?item_type=spans`, + ]) { + expect(classifyUrl(url)).toBe("stable"); + } + }); +}); + +describe("FALLBACK_TTL_MS", () => { + it("has no tier that reuses a response for more than 5 minutes", () => { + for (const tier of Object.keys(FALLBACK_TTL_MS) as TtlTier[]) { + expect(FALLBACK_TTL_MS[tier]).toBeLessThanOrEqual(5 * 60 * 1000); + } + }); +}); diff --git a/packages/toolkit-core/src/response-cache/tiers.ts b/packages/toolkit-core/src/response-cache/tiers.ts new file mode 100644 index 000000000..321836f9f --- /dev/null +++ b/packages/toolkit-core/src/response-cache/tiers.ts @@ -0,0 +1,71 @@ +/** + * Fallback TTL tiers for Sentry API GET responses that carry no HTTP caching + * headers. Responses with `Cache-Control`, `Expires`, or validators follow + * those headers instead; the host's cache decides which applies. + */ + +/** + * TTL tier for a URL. + * + * - `stable`: data that changes infrequently (orgs, projects, teams, + * organization event details, trace-item details) + * - `volatile`: data that changes often (issues and their events, project + * event details, full traces, log and span queries) + * - `no-cache`: never cache (polling endpoints like autofix state) + * + * No tier treats a response as immutable. Event payloads embed mutable + * fields (`nextEventID`, user reports, release data), `latest`/`oldest`/ + * `recommended` resolve to different events over time, and traces gain + * spans as they arrive (getsentry/sentry#128818). + */ +export type TtlTier = "stable" | "volatile" | "no-cache"; + +/** Fallback TTL durations by tier (milliseconds). `no-cache` uses 0 as a sentinel. */ +export const FALLBACK_TTL_MS: Readonly> = { + stable: 5 * 60 * 1000, + volatile: 60 * 1000, + "no-cache": 0, +}; + +/** + * URL patterns grouped by TTL tier, checked in priority order + * (no-cache → volatile). `stable` is the default when nothing matches. + * + * TODO: Classify by generated operation metadata from `@sentry/api` instead + * of URL regexes, once it ships per-operation metadata + * (getsentry/sentry-api-schema#81) and a route-keyed operation map + * (getsentry/sentry-api-schema#102). + */ +const URL_TIER_PATTERNS: readonly [TtlTier, readonly RegExp[]][] = [ + // Polling endpoints where state changes rapidly + ["no-cache", [/\/(?:autofix|root-cause)\//]], + [ + "volatile", + [ + // Issue endpoints: lists, detail views, and issue events such as + // `/issues/{id}/events/latest/` + /\/issues\//, + /[?&]dataset=(?:logs|spans)/, + /\/trace-logs\//, + // Full traces gain spans as they arrive + /\/trace\/[0-9a-f]{32}\//, + // Project event details embed `nextEventID`/`previousEventID` and user + // reports, and reprocessing can change them for the same ID + /\/projects\/[^/]+\/[^/]+\/events\/[^/?]+\/?(?:\?|$)/, + ], + ], +]; + +/** + * Classify a URL into a TTL tier for fallback caching. + * + * @param url - Full URL string (with query params) + */ +export function classifyUrl(url: string): TtlTier { + for (const [tier, patterns] of URL_TIER_PATTERNS) { + if (patterns.some((pattern) => pattern.test(url))) { + return tier; + } + } + return "stable"; +} From f506af4f41ac332fefd64e84b0a16948c181e33f Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 27 Oct 2026 23:18:43 +0000 Subject: [PATCH 2/2] test(toolkit-core): Move the pure cache policy tests out of the CLI The tier, key, and URL normalization property tests now live next to the code in `toolkit-core/src/response-cache/policy.property.test.ts`, with `fast-check` added as a toolkit-core dev dependency. The CLI keeps only the tests for its disk store and for its `buildCacheKey` wrapper's default identity, which comes from the active login. Cases the property tests already cover are dropped from `tiers.test.ts` and `keys.test.ts`. --- packages/cli/test/lib/response-cache.test.ts | 51 +------------------ packages/toolkit-core/package.json | 1 + .../src/response-cache/keys.test.ts | 22 +++----- .../response-cache/policy.property.test.ts} | 30 ++++++----- .../src/response-cache/tiers.test.ts | 23 ++------- pnpm-lock.yaml | 3 ++ 6 files changed, 34 insertions(+), 96 deletions(-) rename packages/{cli/test/lib/response-cache.property.test.ts => toolkit-core/src/response-cache/policy.property.test.ts} (90%) diff --git a/packages/cli/test/lib/response-cache.test.ts b/packages/cli/test/lib/response-cache.test.ts index 30b99d193..afecc824e 100644 --- a/packages/cli/test/lib/response-cache.test.ts +++ b/packages/cli/test/lib/response-cache.test.ts @@ -7,7 +7,6 @@ import { readdir } from "node:fs/promises"; import { join } from "node:path"; -import { normalizeUrl } from "@sentry/toolkit-core/response-cache"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { setAuthToken } from "../../src/lib/db/auth.js"; import { @@ -337,57 +336,11 @@ 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 +// buildCacheKey — key derivation is tested in toolkit-core; this covers the +// CLI wrapper's default identity, taken from the active login // --------------------------------------------------------------------------- describe("buildCacheKey", () => { - test("produces a 64-char hex string", async () => { - expect(await buildCacheKey("GET", TEST_URL)).toMatch(/^[0-9a-f]{64}$/); - }); - - test("is deterministic", async () => { - expect(await buildCacheKey("GET", TEST_URL)).toBe( - await buildCacheKey("GET", TEST_URL), - ); - }); - - test("different methods produce different keys", async () => { - expect(await buildCacheKey("GET", TEST_URL)).not.toBe( - await buildCacheKey("POST", TEST_URL), - ); - }); - test("different identities produce different keys for the same URL", async () => { // Switching accounts must route reads/writes through a different // namespace so users never see each other's cached data. diff --git a/packages/toolkit-core/package.json b/packages/toolkit-core/package.json index 8ab356d6a..70d7ef00c 100644 --- a/packages/toolkit-core/package.json +++ b/packages/toolkit-core/package.json @@ -82,6 +82,7 @@ "tsc": "tsc --noEmit" }, "devDependencies": { + "fast-check": "^4.8.0", "typescript": "catalog:", "vitest": "catalog:" } diff --git a/packages/toolkit-core/src/response-cache/keys.test.ts b/packages/toolkit-core/src/response-cache/keys.test.ts index dc1cc74fe..f759a2097 100644 --- a/packages/toolkit-core/src/response-cache/keys.test.ts +++ b/packages/toolkit-core/src/response-cache/keys.test.ts @@ -11,6 +11,12 @@ describe("normalizeUrl", () => { ); }); + it("keeps self-hosted hosts", () => { + expect( + normalizeUrl("GET", "https://sentry.mycompany.internal/api/0/issues/"), + ).toBe("GET|https://sentry.mycompany.internal/api/0/issues/"); + }); + it("throws on a URL it cannot parse", () => { expect(() => normalizeUrl("GET", "not a url")).toThrow(TypeError); }); @@ -32,22 +38,6 @@ describe("buildCacheKey", () => { ); }); - it("ignores query param order and method case", async () => { - expect( - await buildCacheKey( - "get", - "https://us.sentry.io/api/0/issues/?a=1&b=2", - "alice", - ), - ).toBe( - await buildCacheKey( - "GET", - "https://us.sentry.io/api/0/issues/?b=2&a=1", - "alice", - ), - ); - }); - it("matches node:crypto HMAC-SHA256, so existing CLI cache entries still hit", async () => { const identity = "oauth:3f2a9c"; const expected = createHmac("sha256", identity) diff --git a/packages/cli/test/lib/response-cache.property.test.ts b/packages/toolkit-core/src/response-cache/policy.property.test.ts similarity index 90% rename from packages/cli/test/lib/response-cache.property.test.ts rename to packages/toolkit-core/src/response-cache/policy.property.test.ts index b393fc5b0..ec9e08361 100644 --- a/packages/cli/test/lib/response-cache.property.test.ts +++ b/packages/toolkit-core/src/response-cache/policy.property.test.ts @@ -14,10 +14,14 @@ import { string, tuple, } from "fast-check"; -import { classifyUrl, normalizeUrl } from "@sentry/toolkit-core/response-cache"; import { describe, expect, test } from "vitest"; -import { buildCacheKey } from "../../src/lib/response-cache.js"; -import { DEFAULT_NUM_RUNS } from "../model-based/helpers.js"; +import { buildCacheKey, normalizeUrl } from "./keys.js"; +import { classifyUrl } from "./tiers.js"; + +const DEFAULT_NUM_RUNS = 50; + +/** Any non-empty identity; scoping across identities is in keys.test.ts. */ +const IDENTITY = "test-identity"; // --------------------------------------------------------------------------- // Arbitraries @@ -67,7 +71,9 @@ describe("property: buildCacheKey", () => { test("produces a 64-char hex string (SHA-256)", async () => { await fcAssert( asyncProperty(methodArb, sentryUrlArb, async (method, url) => { - expect(await buildCacheKey(method, url)).toMatch(/^[0-9a-f]{64}$/); + expect(await buildCacheKey(method, url, IDENTITY)).toMatch( + /^[0-9a-f]{64}$/, + ); }), { numRuns: DEFAULT_NUM_RUNS }, ); @@ -76,8 +82,8 @@ describe("property: buildCacheKey", () => { test("is deterministic — same inputs produce same key", async () => { await fcAssert( asyncProperty(methodArb, sentryUrlArb, async (method, url) => { - expect(await buildCacheKey(method, url)).toBe( - await buildCacheKey(method, url), + expect(await buildCacheKey(method, url, IDENTITY)).toBe( + await buildCacheKey(method, url, IDENTITY), ); }), { numRuns: DEFAULT_NUM_RUNS }, @@ -87,8 +93,8 @@ describe("property: buildCacheKey", () => { test("different methods produce different keys for same URL", async () => { await fcAssert( asyncProperty(sentryUrlArb, async (url) => { - expect(await buildCacheKey("GET", url)).not.toBe( - await buildCacheKey("POST", url), + expect(await buildCacheKey("GET", url, IDENTITY)).not.toBe( + await buildCacheKey("POST", url, IDENTITY), ); }), { numRuns: DEFAULT_NUM_RUNS }, @@ -103,8 +109,8 @@ describe("property: buildCacheKey", () => { async (base, path) => { const url1 = `${base}/api/0/${path}?a=1&b=2&c=3`; const url2 = `${base}/api/0/${path}?c=3&a=1&b=2`; - expect(await buildCacheKey("GET", url1)).toBe( - await buildCacheKey("GET", url2), + expect(await buildCacheKey("GET", url1, IDENTITY)).toBe( + await buildCacheKey("GET", url2, IDENTITY), ); }, ), @@ -115,8 +121,8 @@ describe("property: buildCacheKey", () => { test("method comparison is case-insensitive", async () => { await fcAssert( asyncProperty(sentryUrlArb, async (url) => { - expect(await buildCacheKey("get", url)).toBe( - await buildCacheKey("GET", url), + expect(await buildCacheKey("get", url, IDENTITY)).toBe( + await buildCacheKey("GET", url, IDENTITY), ); }), { numRuns: DEFAULT_NUM_RUNS }, diff --git a/packages/toolkit-core/src/response-cache/tiers.test.ts b/packages/toolkit-core/src/response-cache/tiers.test.ts index 2da56a084..0c504e756 100644 --- a/packages/toolkit-core/src/response-cache/tiers.test.ts +++ b/packages/toolkit-core/src/response-cache/tiers.test.ts @@ -32,15 +32,10 @@ describe("classifyUrl", () => { expect(ttlFor(url)).toBeLessThanOrEqual(60 * 1000); }); - it("treats project event details, log queries, and span queries as volatile", () => { - for (const url of [ - `${API}/projects/acme/web/events/deadbeef/`, - `${API}/organizations/acme/events/?dataset=logs&query=foo`, - `${API}/organizations/acme/events/?field=id&dataset=spans`, - `${API}/organizations/acme/trace-logs/?traceId=${TRACE_ID}`, - ]) { - expect(classifyUrl(url)).toBe("volatile"); - } + it("treats trace log queries as volatile", () => { + expect( + classifyUrl(`${API}/organizations/acme/trace-logs/?traceId=${TRACE_ID}`), + ).toBe("volatile"); }); it("never serves polling endpoints from cache", () => { @@ -52,16 +47,6 @@ describe("classifyUrl", () => { expect(ttlFor(url)).toBe(0); } }); - - it("defaults everything else to stable", () => { - for (const url of [ - `${API}/organizations/`, - `${API}/organizations/acme/projects/`, - `${API}/projects/acme/web/trace-items/abc123/?item_type=spans`, - ]) { - expect(classifyUrl(url)).toBe("stable"); - } - }); }); describe("FALLBACK_TTL_MS", () => { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 47ea8b669..743cfca49 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1017,6 +1017,9 @@ importers: packages/toolkit-core: devDependencies: + fast-check: + specifier: ^4.8.0 + version: 4.10.1 typescript: specifier: 'catalog:' version: 5.8.3