fix(spotify): guard non-numeric 429 Retry-After (no immediate retry) [corner-case]

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/music/spotify/webapi.test.ts

@@ -176,6 +176,121 @@ describe("SpotifyWebApi rate-limit handling", () => {
     }
   });
 
+  // Corner-case: a non-numeric Retry-After (HTTP-date) must NOT coerce to NaN and
+  // fire the retry at 0ms. `Number("Wed, 21 Oct 2025 07:28:00 GMT")` is NaN, so the
+  // pre-fix `Math.min(NaN,10)*1000` schedules an immediate (or never-firing) retry
+  // that ignores the advised backoff. Post-fix falls back to a finite 1s wait: the
+  // retry stays scheduled at exactly the fallback, still bounded to ONE retry.
+  it("falls back to a finite 1s wait when Retry-After is an HTTP-date (not NaN/immediate)", async () => {
+    vi.useFakeTimers();
+    try {
+      const auth = {
+        post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+      } as any;
+      let call = 0;
+      const http = {
+        get: vi.fn().mockImplementation(() => {
+          call += 1;
+          if (call === 1) {
+            return Promise.reject({
+              response: {
+                status: 429,
+                headers: { "retry-after": "Wed, 21 Oct 2025 07:28:00 GMT" },
+              },
+            });
+          }
+          return Promise.resolve({
+            data: { tracks: { items: [{ id: "t1", name: "n", artists: [], duration_ms: 1000 }] } },
+          });
+        }),
+      } as any;
+      const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+      const p = api.search("queen");
+
+      // Let the token fetch + first (429) call settle and schedule the wait.
+      await vi.advanceTimersByTimeAsync(999); // < 1s fallback → retry has NOT fired yet
+      expect(http.get).toHaveBeenCalledTimes(1);
+
+      await vi.advanceTimersByTimeAsync(1); // now 1s total → finite fallback reached, retry fires
+      const out = await p;
+      expect(http.get).toHaveBeenCalledTimes(2); // exactly one bounded retry
+      expect(out.songs[0].id).toBe("t1");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  // Corner-case sibling: an empty Retry-After coerces to 0 (`Number("")` === 0), so
+  // pre-fix `Math.min(0,10)*1000` fires the retry immediately at 0ms. Post-fix guards
+  // `raw > 0`, so it falls back to the same finite 1s wait rather than firing at 0.
+  it("falls back to a finite 1s wait when Retry-After is empty (not 0/immediate)", async () => {
+    vi.useFakeTimers();
+    try {
+      const auth = {
+        post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+      } as any;
+      let call = 0;
+      const http = {
+        get: vi.fn().mockImplementation(() => {
+          call += 1;
+          if (call === 1) {
+            return Promise.reject({ response: { status: 429, headers: { "retry-after": "" } } });
+          }
+          return Promise.resolve({
+            data: { tracks: { items: [{ id: "t1", name: "n", artists: [], duration_ms: 1000 }] } },
+          });
+        }),
+      } as any;
+      const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+      const p = api.search("queen");
+
+      await vi.advanceTimersByTimeAsync(999); // < 1s fallback → retry must NOT have fired at 0ms
+      expect(http.get).toHaveBeenCalledTimes(1);
+
+      await vi.advanceTimersByTimeAsync(1); // 1s total → fallback reached, retry fires
+      const out = await p;
+      expect(http.get).toHaveBeenCalledTimes(2);
+      expect(out.songs[0].id).toBe("t1");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  // Regression guard: a NORMAL numeric Retry-After ("3") is finite/positive, so the
+  // guard is transparent — the advised ~3s wait (below the 10s cap) is honored intact.
+  it("still honors a normal numeric Retry-After (~3s, below the 10s cap)", async () => {
+    vi.useFakeTimers();
+    try {
+      const auth = {
+        post: vi.fn().mockResolvedValue({ data: { access_token: "t", expires_in: 3600 } }),
+      } as any;
+      let call = 0;
+      const http = {
+        get: vi.fn().mockImplementation(() => {
+          call += 1;
+          if (call === 1) {
+            return Promise.reject({ response: { status: 429, headers: { "retry-after": "3" } } });
+          }
+          return Promise.resolve({
+            data: { tracks: { items: [{ id: "t1", name: "n", artists: [], duration_ms: 1000 }] } },
+          });
+        }),
+      } as any;
+      const api = new SpotifyWebApi(() => ({ clientId: "a", clientSecret: "b" }), { http, auth });
+      const p = api.search("queen");
+
+      await vi.advanceTimersByTimeAsync(2_999); // < 3s advised → retry has NOT fired yet
+      expect(http.get).toHaveBeenCalledTimes(1);
+
+      await vi.advanceTimersByTimeAsync(1); // 3s total → advised wait reached, retry fires
+      const out = await p;
+      expect(http.get).toHaveBeenCalledTimes(2);
+      expect(out.songs[0].id).toBe("t1");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it("returns empty results when unconfigured (no creds → no token)", async () => {
     const api = new SpotifyWebApi(() => ({ clientId: "", clientSecret: "" }));
     expect(await api.search("queen")).toEqual({ songs: [], playlists: [], albums: [] });

src/music/spotify/webapi.ts

@@ -127,7 +127,11 @@ export class SpotifyWebApi {
       // Spotify rate-limits on a rolling 30s window (429 + Retry-After seconds).
       // Retry once after the advised delay before giving up.
       if (retryOn429 && err?.response?.status === 429) {
-        const retryAfter = Number(err.response.headers?.["retry-after"] ?? 1);
+        // Retry-After may be a non-numeric HTTP-date or empty string; Number(...)
+        // then yields NaN/0 and fires the single retry at 0ms, ignoring the advised
+        // backoff. Guard for a finite positive value (mirrors connect-api.ts).
+        const raw = Number(err.response?.headers?.["retry-after"]);
+        const retryAfter = Number.isFinite(raw) && raw > 0 ? raw : 1;
         await new Promise((r) => setTimeout(r, Math.min(retryAfter, 10) * 1000));
         return this.get(path, params, false);
       }