Skip to content

Commit b25f88b

Browse files
committed
fix(cache): --fresh flag now updates cache with fresh response
The --fresh flag disabled both cache reads AND writes, so subsequent invocations without --fresh still served stale data until TTL expired. Split the bypass into read-only (--fresh flag) vs full disable (SENTRY_NO_CACHE=1 env var) so fresh API responses are written back to cache.
1 parent 33b21a9 commit b25f88b

File tree

2 files changed

+94
-11
lines changed

2 files changed

+94
-11
lines changed

src/lib/response-cache.ts

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -307,18 +307,20 @@ function buildResponseHeaders(
307307
// Cache bypass control
308308
// ---------------------------------------------------------------------------
309309

310-
let cacheDisabledFlag = false;
310+
let cacheReadBypassed = false;
311311

312312
/**
313-
* Disable the response cache for the current process.
314-
* Called when `--fresh` flag is passed to a command.
313+
* Bypass cache reads for the current process.
314+
*
315+
* Called when `--fresh` flag is passed to a command. Fresh API responses are
316+
* still written to cache so subsequent invocations serve updated data.
315317
*/
316318
export function disableResponseCache(): void {
317-
cacheDisabledFlag = true;
319+
cacheReadBypassed = true;
318320
}
319321

320322
/**
321-
* Re-enable the response cache after `disableResponseCache()` was called.
323+
* Re-enable cache reads after `disableResponseCache()` was called.
322324
*
323325
* This is only needed in tests to prevent one test's `--fresh` flag from
324326
* permanently disabling caching for subsequent tests in the same process.
@@ -327,17 +329,28 @@ export function disableResponseCache(): void {
327329
* @internal Exported for testing
328330
*/
329331
export function resetCacheState(): void {
330-
cacheDisabledFlag = false;
332+
cacheReadBypassed = false;
331333
}
332334

333335
/**
334-
* Check if response caching is disabled.
335-
* Cache is disabled when:
336-
* - `disableResponseCache()` was called (--refresh flag)
336+
* Check if cache reads are disabled.
337+
* Reads are skipped when:
338+
* - `disableResponseCache()` was called (`--fresh` flag)
337339
* - `SENTRY_NO_CACHE=1` environment variable is set
338340
*/
339341
export function isCacheDisabled(): boolean {
340-
return cacheDisabledFlag || getEnv().SENTRY_NO_CACHE === "1";
342+
return cacheReadBypassed || getEnv().SENTRY_NO_CACHE === "1";
343+
}
344+
345+
/**
346+
* Check if cache writes are disabled.
347+
*
348+
* Only the `SENTRY_NO_CACHE=1` env var disables writes. The `--fresh` flag
349+
* intentionally allows writes so that the freshly-fetched response replaces
350+
* any stale cache entry.
351+
*/
352+
function isCacheWriteDisabled(): boolean {
353+
return getEnv().SENTRY_NO_CACHE === "1";
341354
}
342355

343356
// ---------------------------------------------------------------------------
@@ -474,7 +487,7 @@ export async function storeCachedResponse(
474487
): Promise<void> {
475488
if (
476489
method !== "GET" ||
477-
isCacheDisabled() ||
490+
isCacheWriteDisabled() ||
478491
!response.ok ||
479492
classifyUrl(url) === "no-cache"
480493
) {

test/lib/response-cache.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { join } from "node:path";
1111
import {
1212
buildCacheKey,
1313
clearResponseCache,
14+
disableResponseCache,
1415
getCachedResponse,
1516
normalizeUrl,
1617
resetCacheState,
@@ -262,6 +263,75 @@ describe("cache bypass", () => {
262263
const cached = await getCachedResponse(TEST_METHOD, TEST_URL, {});
263264
expect(cached).toBeUndefined();
264265
});
266+
267+
test("--fresh bypasses cache reads", async () => {
268+
await storeCachedResponse(
269+
TEST_METHOD,
270+
TEST_URL,
271+
{},
272+
mockResponse(TEST_BODY)
273+
);
274+
275+
disableResponseCache();
276+
277+
const cached = await getCachedResponse(TEST_METHOD, TEST_URL, {});
278+
expect(cached).toBeUndefined();
279+
});
280+
281+
test("--fresh still allows cache writes", async () => {
282+
disableResponseCache();
283+
284+
const freshBody = { data: "fresh" };
285+
await storeCachedResponse(
286+
TEST_METHOD,
287+
TEST_URL,
288+
{},
289+
mockResponse(freshBody)
290+
);
291+
292+
// Re-enable cache reads to verify the write succeeded
293+
resetCacheState();
294+
295+
const cached = await getCachedResponse(TEST_METHOD, TEST_URL, {});
296+
expect(cached).toBeDefined();
297+
expect(await cached!.json()).toEqual(freshBody);
298+
});
299+
300+
test("--fresh round-trip: stale entry is replaced by fresh response", async () => {
301+
const staleBody = { data: "stale" };
302+
const freshBody = { data: "fresh" };
303+
304+
// Store initial stale entry
305+
await storeCachedResponse(
306+
TEST_METHOD,
307+
TEST_URL,
308+
{},
309+
mockResponse(staleBody)
310+
);
311+
312+
// Activate --fresh: reads are bypassed, but writes still go through
313+
disableResponseCache();
314+
315+
// Verify stale entry is not served
316+
const duringFresh = await getCachedResponse(TEST_METHOD, TEST_URL, {});
317+
expect(duringFresh).toBeUndefined();
318+
319+
// Store fresh response (overwrites the stale entry)
320+
await storeCachedResponse(
321+
TEST_METHOD,
322+
TEST_URL,
323+
{},
324+
mockResponse(freshBody)
325+
);
326+
327+
// Re-enable cache reads (simulates next invocation without --fresh)
328+
resetCacheState();
329+
330+
// Verify fresh data is served from cache
331+
const afterFresh = await getCachedResponse(TEST_METHOD, TEST_URL, {});
332+
expect(afterFresh).toBeDefined();
333+
expect(await afterFresh!.json()).toEqual(freshBody);
334+
});
265335
});
266336

267337
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)