mirror of
https://github.com/jakejarvis/sofa.git
synced 2026-10-15 19:45:42 -04:00
fix(imports): poll device codes one request at a time
The Trakt/Simkl sign-in dialog polled with setInterval and no in-flight guard, so while the first "authorized" poll was still fetching the library, the next tick re-sent the redeemed device code. Trakt answers 409, which was mapped to "expired", producing a false "Device code expired" error. - public-api: map Trakt 409 (already redeemed) to pending - web: add a single-flight device-code poller that drops late responses after stop() and aborts the in-flight request - web: use it in the imports dialog and ignore device-code responses that arrive after the dialog was closed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
5 files changed
+226
-29
No files matched your search
@@ -214,3 +214,15 @@ describe("fitToBudget", () => {
|
||||
expect(result.warnings).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("trakt.pollForToken", () => {
|
||||
test("treats 409 (code already redeemed) as pending", async () => {
|
||||
stubTrakt((path) => (path === "/oauth/device/token" ? { status: 409 } : undefined));
|
||||
expect(await trakt.pollForToken("id", "secret", "code")).toEqual({ status: "pending" });
|
||||
});
|
||||
|
||||
test("treats 410 as expired", async () => {
|
||||
stubTrakt((path) => (path === "/oauth/device/token" ? { status: 410 } : undefined));
|
||||
expect(await trakt.pollForToken("id", "secret", "code")).toEqual({ status: "expired" });
|
||||
});
|
||||
});
|
||||
@@ -254,6 +254,9 @@ export const trakt: ImportProvider = {
|
||||
if (res.status === 410) return { status: "expired" };
|
||||
if (res.status === 418) return { status: "denied" };
|
||||
if (res.status === 429) return { status: "pending" };
|
||||
// 409: this code was already redeemed (an earlier poll got the token and is still fetching
|
||||
// the library). Keep the client waiting for that response instead of reporting expiry.
|
||||
if (res.status === 409) return { status: "pending" };
|
||||
// 5xx: likely transient — keep polling
|
||||
if (res.status >= 500) return { status: "pending" };
|
||||
// Unknown 4xx: likely permanent — treat as expired
|
||||
|
||||
@@ -21,6 +21,7 @@ import {
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { Progress } from "@/components/ui/progress";
|
||||
import { Spinner } from "@/components/ui/spinner";
|
||||
import { startDevicePoll } from "@/lib/device-code-poll";
|
||||
import { getErrorMessage } from "@/lib/error-messages";
|
||||
import { client, orpc } from "@/lib/orpc/client";
|
||||
import { invalidateTrackingQueries } from "@/lib/orpc/invalidate";
|
||||
@@ -210,7 +211,8 @@ function ImportSourceCard({ config }: { config: SourceConfig }) {
|
||||
// OAuth state
|
||||
const [deviceCode, setDeviceCode] = useState<DeviceCodeInfo | null>(null);
|
||||
const [oauthError, setOauthError] = useState<string | null>(null);
|
||||
const pollTimerRef = useRef<ReturnType<typeof setInterval> | null>(null);
|
||||
const pollRef = useRef<{ stop: () => void } | null>(null);
|
||||
const flowIdRef = useRef(0);
|
||||
const importAbortRef = useRef<AbortController | null>(null);
|
||||
|
||||
const parseMutation = useMutation(
|
||||
@@ -344,14 +346,13 @@ function ImportSourceCard({ config }: { config: SourceConfig }) {
|
||||
|
||||
// Clean up poll timer on unmount or dialog close
|
||||
const stopPolling = useCallback(() => {
|
||||
if (pollTimerRef.current) {
|
||||
clearInterval(pollTimerRef.current);
|
||||
pollTimerRef.current = null;
|
||||
}
|
||||
pollRef.current?.stop();
|
||||
pollRef.current = null;
|
||||
}, []);
|
||||
|
||||
function handleClose() {
|
||||
stopPolling();
|
||||
flowIdRef.current += 1;
|
||||
importAbortRef.current?.abort();
|
||||
importAbortRef.current = null;
|
||||
setDialogOpen(false);
|
||||
@@ -370,6 +371,7 @@ function ImportSourceCard({ config }: { config: SourceConfig }) {
|
||||
// ─── OAuth: Start device code flow ────────────────────────
|
||||
|
||||
async function startDeviceCodeFlow() {
|
||||
const flowId = ++flowIdRef.current;
|
||||
setOauthError(null);
|
||||
setStep("device-code");
|
||||
const sourceLabel = config.label;
|
||||
@@ -382,12 +384,14 @@ function ImportSourceCard({ config }: { config: SourceConfig }) {
|
||||
throw new Error("device-code request failed");
|
||||
}
|
||||
const data = (await res.json()) as DeviceCodeInfo;
|
||||
if (flowIdRef.current !== flowId) return;
|
||||
setDeviceCode(data);
|
||||
setDialogOpen(true);
|
||||
|
||||
// Start polling
|
||||
startPolling(data);
|
||||
} catch {
|
||||
if (flowIdRef.current !== flowId) return;
|
||||
setOauthError(t`Failed to start ${sourceLabel} connection`);
|
||||
setStep("choose");
|
||||
setDialogOpen(true);
|
||||
@@ -396,58 +400,61 @@ function ImportSourceCard({ config }: { config: SourceConfig }) {
|
||||
|
||||
function startPolling(code: DeviceCodeInfo) {
|
||||
stopPolling();
|
||||
const interval = (code.interval || 5) * 1000;
|
||||
const expiresAt = Date.now() + code.expires_in * 1000;
|
||||
|
||||
pollTimerRef.current = setInterval(async () => {
|
||||
if (Date.now() > expiresAt) {
|
||||
stopPolling();
|
||||
setOauthError(t`Device code expired. Please try again.`);
|
||||
setStep("choose");
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
pollRef.current = startDevicePoll({
|
||||
intervalMs: (code.interval || 5) * 1000,
|
||||
expiresAt: Date.now() + code.expires_in * 1000,
|
||||
poll: async (signal) => {
|
||||
const res = await fetch(`${publicApiUrl}/v1/import/${config.source}/poll`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ device_code: code.device_code }),
|
||||
signal,
|
||||
});
|
||||
if (!res.ok) return;
|
||||
if (!res.ok) return null;
|
||||
|
||||
const data = (await res.json()) as {
|
||||
return (await res.json()) as {
|
||||
status: string;
|
||||
data?: Record<string, unknown>;
|
||||
error?: string;
|
||||
};
|
||||
},
|
||||
onResponse: (data) => {
|
||||
if (!data) return false;
|
||||
|
||||
if (data.status === "authorized" && data.data) {
|
||||
stopPolling();
|
||||
setStep("fetching");
|
||||
parsePayloadMutation.mutate({
|
||||
source: config.source as "trakt" | "simkl",
|
||||
rawPayload: data.data,
|
||||
});
|
||||
} else if (data.status === "denied") {
|
||||
stopPolling();
|
||||
return true;
|
||||
}
|
||||
if (data.status === "denied") {
|
||||
setOauthError(t`Authorization was denied. Please try again.`);
|
||||
setStep("choose");
|
||||
} else if (data.status === "expired") {
|
||||
stopPolling();
|
||||
return true;
|
||||
}
|
||||
if (data.status === "expired") {
|
||||
setOauthError(t`Device code expired. Please try again.`);
|
||||
setStep("choose");
|
||||
} else if (data.status === "fetch_error") {
|
||||
stopPolling();
|
||||
return true;
|
||||
}
|
||||
if (data.status === "fetch_error") {
|
||||
setOauthError(
|
||||
t`Authorization succeeded but failed to fetch your library. Please try again.`,
|
||||
);
|
||||
setStep("choose");
|
||||
return true;
|
||||
}
|
||||
// "pending" → keep polling
|
||||
} catch {
|
||||
// Network error, keep polling
|
||||
}
|
||||
}, interval);
|
||||
return false;
|
||||
},
|
||||
onExpired: () => {
|
||||
setOauthError(t`Device code expired. Please try again.`);
|
||||
setStep("choose");
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
// Clean up on unmount
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
|
||||
|
||||
import { startDevicePoll } from "./device-code-poll";
|
||||
|
||||
const INTERVAL = 1000;
|
||||
|
||||
function deferred<T>() {
|
||||
let resolve!: (value: T) => void;
|
||||
const promise = new Promise<T>((r) => {
|
||||
resolve = r;
|
||||
});
|
||||
return { promise, resolve };
|
||||
}
|
||||
|
||||
describe("startDevicePoll", () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
test("only has one request in flight at a time", async () => {
|
||||
const pending = deferred<string>();
|
||||
const poll = vi.fn<(signal: AbortSignal) => Promise<string>>(() => pending.promise);
|
||||
const poller = startDevicePoll({
|
||||
poll,
|
||||
onResponse: () => false,
|
||||
onExpired: vi.fn<() => void>(),
|
||||
intervalMs: INTERVAL,
|
||||
expiresAt: Date.now() + 60 * INTERVAL,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL * 3);
|
||||
expect(poll).toHaveBeenCalledTimes(1);
|
||||
poller.stop();
|
||||
});
|
||||
|
||||
test("polls again after a response that is not final", async () => {
|
||||
const first = deferred<string>();
|
||||
const poll = vi.fn<(signal: AbortSignal) => Promise<string>>();
|
||||
poll.mockReturnValueOnce(first.promise);
|
||||
poll.mockResolvedValue("pending");
|
||||
const onResponse = vi.fn<(response: string) => boolean>(() => false);
|
||||
const poller = startDevicePoll({
|
||||
poll,
|
||||
onResponse,
|
||||
onExpired: vi.fn<() => void>(),
|
||||
intervalMs: INTERVAL,
|
||||
expiresAt: Date.now() + 60 * INTERVAL,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL);
|
||||
expect(poll).toHaveBeenCalledTimes(1);
|
||||
|
||||
first.resolve("pending");
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL);
|
||||
expect(onResponse).toHaveBeenCalledTimes(2);
|
||||
expect(poll).toHaveBeenCalledTimes(2);
|
||||
poller.stop();
|
||||
});
|
||||
|
||||
test("drops late responses after stop()", async () => {
|
||||
const pending = deferred<string>();
|
||||
let received: AbortSignal | undefined;
|
||||
const poll = vi.fn<(signal: AbortSignal) => Promise<string>>((signal) => {
|
||||
received = signal;
|
||||
return pending.promise;
|
||||
});
|
||||
const onResponse = vi.fn<(response: string) => boolean>(() => false);
|
||||
const poller = startDevicePoll({
|
||||
poll,
|
||||
onResponse,
|
||||
onExpired: vi.fn<() => void>(),
|
||||
intervalMs: INTERVAL,
|
||||
expiresAt: Date.now() + 60 * INTERVAL,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL);
|
||||
expect(poll).toHaveBeenCalledTimes(1);
|
||||
|
||||
poller.stop();
|
||||
pending.resolve("late");
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL * 3);
|
||||
|
||||
expect(onResponse).not.toHaveBeenCalled();
|
||||
expect(received?.aborted).toBe(true);
|
||||
expect(poll).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test("calls onExpired once when the code expires before the first poll", async () => {
|
||||
const poll = vi.fn<(signal: AbortSignal) => Promise<string>>(async () => "pending");
|
||||
const onExpired = vi.fn<() => void>();
|
||||
startDevicePoll({
|
||||
poll,
|
||||
onResponse: () => false,
|
||||
onExpired,
|
||||
intervalMs: INTERVAL,
|
||||
expiresAt: Date.now() + INTERVAL / 2,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL * 3);
|
||||
expect(onExpired).toHaveBeenCalledTimes(1);
|
||||
expect(poll).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("keeps polling after a request error", async () => {
|
||||
const poll = vi.fn<(signal: AbortSignal) => Promise<string>>();
|
||||
poll.mockRejectedValueOnce(new Error("network"));
|
||||
poll.mockResolvedValue("ok");
|
||||
const onResponse = vi.fn<(response: string) => boolean>(() => true);
|
||||
startDevicePoll({
|
||||
poll,
|
||||
onResponse,
|
||||
onExpired: vi.fn<() => void>(),
|
||||
intervalMs: INTERVAL,
|
||||
expiresAt: Date.now() + 60 * INTERVAL,
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL * 2);
|
||||
expect(poll).toHaveBeenCalledTimes(2);
|
||||
expect(onResponse).toHaveBeenCalledTimes(1);
|
||||
expect(onResponse).toHaveBeenCalledWith("ok");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
export interface DevicePollOptions<T> {
|
||||
/** One poll request. Receives an AbortSignal that fires when the poller stops. */
|
||||
poll: (signal: AbortSignal) => Promise<T>;
|
||||
/** Called with each response; return true to stop polling. Never called after stop(). */
|
||||
onResponse: (response: T) => boolean;
|
||||
/** Called once when expiresAt passes without onResponse returning true. */
|
||||
onExpired: () => void;
|
||||
intervalMs: number;
|
||||
expiresAt: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Polls one request at a time: the next request is scheduled only after the previous one
|
||||
* settles, and nothing is delivered after stop(). Request errors are ignored (keep polling).
|
||||
*/
|
||||
export function startDevicePoll<T>(options: DevicePollOptions<T>): { stop: () => void } {
|
||||
const controller = new AbortController();
|
||||
let timer: ReturnType<typeof setTimeout> | null = null;
|
||||
let stopped = false;
|
||||
|
||||
const stop = () => {
|
||||
stopped = true;
|
||||
if (timer) clearTimeout(timer);
|
||||
controller.abort();
|
||||
};
|
||||
|
||||
const tick = async () => {
|
||||
if (stopped) return;
|
||||
if (Date.now() > options.expiresAt) {
|
||||
stop();
|
||||
options.onExpired();
|
||||
return;
|
||||
}
|
||||
try {
|
||||
const response = await options.poll(controller.signal);
|
||||
if (stopped) return;
|
||||
if (options.onResponse(response)) {
|
||||
stop();
|
||||
return;
|
||||
}
|
||||
} catch {
|
||||
if (stopped) return;
|
||||
}
|
||||
timer = setTimeout(tick, options.intervalMs);
|
||||
};
|
||||
|
||||
timer = setTimeout(tick, options.intervalMs);
|
||||
return { stop };
|
||||
}
|
||||
Reference in new issue
Block a user