fix: single-flight Spotify token + 401-refresh-retry to survive cold-cache fan-out (closes task/spotify-token-resilience)

Source: task/spotify-token-resilience (plan/steward-linker-quality) — cold-cache fan-out fired up to 100 concurrent token POSTs (429 storm) and search didn't refresh-retry on a 401. - service/spotify.js (new): makeSpotifyToken() single-flights the token fetch via an in-flight-promise cache so N concurrent cold/expired lookups share ONE POST /api/token; clears the promise on settle (success or failure) so a rejected fetch never poisons the cache. makeSpotifySearch() refreshes-and-retries once on a 401 and honors a bounded (capped) Retry-After on a 429, else throws (resolve-core nulls the candidate, as before). - service/resolver.js: wires the factories with the real fetch + Date.now; value-cache + (expires_in-60)*1000 expiry math unchanged; external behavior (result shapes, USE_SPOTIFY degradation, /health, /resolve) identical. - service/spotify.test.js (new): hermetic node:test coverage — single-flight (100 concurrent -> 1 fetch), re-fetch after expiry, failure clears in-flight, 401 refresh-retry + persistent-401 throw, bounded 429 backoff. Zero new deps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 18:30:12 +00:00
parent 8ac0666a71
commit 5ceea28948
3 changed files with 287 additions and 22 deletions
@@ -0,0 +1,177 @@
+// Unit tests for the Spotify token single-flight + search-with-retry helpers (run with
+// `node --test`). Zero-dep, hermetic: a counting fake fetchToken/doSearch + an injected clock,
+// no network and no real timers (sleep is injected as a zero-delay stub).
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { makeSpotifyToken, makeSpotifySearch, retryAfterMs } from "./spotify.js";
+
+// A controllable clock: starts at 0, advance() bumps it.
+function fakeClock(start = 0) {
+  let t = start;
+  return { now: () => t, advance: (ms) => { t += ms; } };
+}
+
+// A fetchToken that counts calls and yields a token + expires_in; optionally defers via a gate
+// so concurrent callers actually overlap on the in-flight promise before it settles.
+function countingFetchToken({ expiresIn = 3600, token = "tok", gate = null } = {}) {
+  const state = { calls: 0 };
+  const fetchToken = async () => {
+    state.calls += 1;
+    const n = state.calls;
+    if (gate) await gate; // hold all in-flight calls open until the test releases the gate
+    return { access_token: `${token}-${n}`, expires_in: expiresIn };
+  };
+  return { fetchToken, state };
+}
+
+test("single-flight: N concurrent cold getToken share exactly one fetch", async () => {
+  let release;
+  const gate = new Promise((r) => { release = r; });
+  const { fetchToken, state } = countingFetchToken({ gate });
+  const clock = fakeClock();
+  const { getToken } = makeSpotifyToken({ fetchToken, now: clock.now });
+
+  // Fire 100 concurrent cold lookups; they all enter while the fetch is gated open.
+  const pending = Array.from({ length: 100 }, () => getToken());
+  release(); // let the single in-flight fetch resolve
+  const tokens = await Promise.all(pending);
+
+  assert.equal(state.calls, 1, "exactly one POST /api/token for 100 concurrent cold callers");
+  assert.ok(tokens.every((t) => t === "tok-1"), "every caller got the same token");
+});
+
+test("warm hit serves the cached value without any fetch", async () => {
+  const { fetchToken, state } = countingFetchToken();
+  const clock = fakeClock();
+  const { getToken } = makeSpotifyToken({ fetchToken, now: clock.now });
+
+  const a = await getToken();
+  const b = await getToken(); // still fresh -> no new fetch
+  assert.equal(a, "tok-1");
+  assert.equal(b, "tok-1");
+  assert.equal(state.calls, 1);
+});
+
+test("re-fetch after expiry: advancing the clock past exp triggers exactly one more fetch", async () => {
+  const { fetchToken, state } = countingFetchToken({ expiresIn: 3600 });
+  const clock = fakeClock();
+  const { getToken } = makeSpotifyToken({ fetchToken, now: clock.now });
+
+  await getToken();
+  assert.equal(state.calls, 1);
+
+  // exp = now + (3600-60)*1000 = 3_540_000 ms. Advance just past it.
+  clock.advance(3_540_001);
+  const t2 = await getToken();
+  assert.equal(state.calls, 2, "one more fetch after expiry");
+  assert.equal(t2, "tok-2", "the fresh token is served");
+});
+
+test("failure clears the in-flight promise: a rejected fetch does not poison the cache", async () => {
+  let failNext = true;
+  const state = { calls: 0 };
+  const fetchToken = async () => {
+    state.calls += 1;
+    if (failNext) { failNext = false; throw new Error("token http 503"); }
+    return { access_token: "tok-ok", expires_in: 3600 };
+  };
+  const clock = fakeClock();
+  const { getToken } = makeSpotifyToken({ fetchToken, now: clock.now });
+
+  await assert.rejects(getToken(), /token http 503/);
+  // The poisoned promise must be cleared -> the next call retries and succeeds.
+  const t = await getToken();
+  assert.equal(t, "tok-ok");
+  assert.equal(state.calls, 2, "the rejected fetch did not stick — a fresh fetch ran");
+});
+
+// --- search-with-retry --------------------------------------------------------
+
+// A doSearch that returns a scripted sequence of { ok, status, headers?, body? } responses,
+// counting calls and recording which token each call saw.
+function scriptedSearch(responses) {
+  const state = { calls: 0, tokensSeen: [] };
+  const doSearch = async (_name, token) => {
+    state.tokensSeen.push(token);
+    const r = responses[state.calls] ?? responses[responses.length - 1];
+    state.calls += 1;
+    return r;
+  };
+  return { doSearch, state };
+}
+
+const okResp = (body) => ({ ok: true, status: 200, body });
+const errResp = (status, headers) => ({ ok: false, status, headers, body: null });
+const hdr = (obj) => ({ get: (k) => obj[k.toLowerCase()] ?? null });
+
+test("401 retry: a stale-token 401 invalidates, refreshes, and retries once with a fresh token", async () => {
+  const { fetchToken, state: tokState } = countingFetchToken();
+  const tokens = makeSpotifyToken({ fetchToken, now: fakeClock().now });
+  const { doSearch, state } = scriptedSearch([errResp(401), okResp({ hit: true })]);
+
+  const search = makeSpotifySearch({ doSearch, tokens, sleep: async () => {} });
+  const body = await search("Tool");
+
+  assert.deepEqual(body, { hit: true });
+  assert.equal(state.calls, 2, "exactly one retry after the 401");
+  assert.equal(tokState.calls, 2, "the token was invalidated and re-fetched");
+  assert.equal(state.tokensSeen[0], "tok-1");
+  assert.equal(state.tokensSeen[1], "tok-2", "the retry used the FRESH token");
+});
+
+test("persistent 401: a second 401 throws (no infinite retry), so resolve-core nulls the candidate", async () => {
+  const { fetchToken } = countingFetchToken();
+  const tokens = makeSpotifyToken({ fetchToken, now: fakeClock().now });
+  const { doSearch, state } = scriptedSearch([errResp(401), errResp(401)]);
+
+  const search = makeSpotifySearch({ doSearch, tokens, sleep: async () => {} });
+  await assert.rejects(search("Tool"), /spotify search http 401/);
+  assert.equal(state.calls, 2, "retried exactly once, then gave up");
+});
+
+test("429 backoff: a Retry-After 429 waits (bounded) then retries once and succeeds", async () => {
+  const { fetchToken } = countingFetchToken();
+  const tokens = makeSpotifyToken({ fetchToken, now: fakeClock().now });
+  const { doSearch, state } = scriptedSearch([errResp(429, hdr({ "retry-after": "1" })), okResp({ hit: true })]);
+
+  let slept = null;
+  const search = makeSpotifySearch({ doSearch, tokens, sleep: async (ms) => { slept = ms; } });
+  const body = await search("Tool");
+
+  assert.deepEqual(body, { hit: true });
+  assert.equal(state.calls, 2, "one retry after the 429");
+  assert.equal(slept, 1000, "honored Retry-After: 1 second");
+});
+
+test("429 retry-after is capped, and a persistent 429 throws without unbounded retry", async () => {
+  const { fetchToken } = countingFetchToken();
+  const tokens = makeSpotifyToken({ fetchToken, now: fakeClock().now });
+  // Retry-After: 9999s would be huge — must cap at maxRetryAfterMs (2000 by default).
+  const { doSearch, state } = scriptedSearch([errResp(429, hdr({ "retry-after": "9999" })), errResp(429)]);
+
+  let slept = null;
+  const search = makeSpotifySearch({ doSearch, tokens, sleep: async (ms) => { slept = ms; } });
+  await assert.rejects(search("Tool"), /spotify search http 429/);
+  assert.equal(slept, 2000, "the wait was capped at maxRetryAfterMs");
+  assert.equal(state.calls, 2, "retried exactly once, then gave up");
+});
+
+test("happy path: a 200 on the first try returns the body with no retry", async () => {
+  const { fetchToken } = countingFetchToken();
+  const tokens = makeSpotifyToken({ fetchToken, now: fakeClock().now });
+  const { doSearch, state } = scriptedSearch([okResp({ artists: { items: [] } })]);
+
+  const search = makeSpotifySearch({ doSearch, tokens, sleep: async () => {} });
+  const body = await search("Tool");
+  assert.deepEqual(body, { artists: { items: [] } });
+  assert.equal(state.calls, 1, "no retry on a clean 200");
+});
+
+test("retryAfterMs parses, caps, and rejects garbage", () => {
+  assert.equal(retryAfterMs(hdr({ "retry-after": "2" }), 5000), 2000);
+  assert.equal(retryAfterMs(hdr({ "retry-after": "100" }), 5000), 5000, "capped at maxMs");
+  assert.equal(retryAfterMs(hdr({ "retry-after": "junk" }), 5000), null);
+  assert.equal(retryAfterMs(hdr({ "retry-after": "-1" }), 5000), null);
+  assert.equal(retryAfterMs(hdr({}), 5000), null, "absent header -> null");
+  assert.equal(retryAfterMs(null, 5000), null, "no headers object -> null");
+});