diff --git a/.changeset/frictionless-stop-after-destroy.md b/.changeset/frictionless-stop-after-destroy.md new file mode 100644 index 0000000000..683122a005 --- /dev/null +++ b/.changeset/frictionless-stop-after-destroy.md @@ -0,0 +1,5 @@ +--- +"@prosopo/procaptcha-frictionless": patch +--- + +Removing or resetting a frictionless widget while its bot-detection request is still running no longer lets that request finish the job. Before, a detection that came back after the widget was gone could still mount the Web Bot Auth badge and call `onHuman` with a token, call `onError`, keep retrying against other providers, or schedule a restart timer that nothing could cancel and that later called `onReset`. The widget now stops as soon as it is destroyed. diff --git a/packages/procaptcha-frictionless/src/procaptchaFrictionless.ts b/packages/procaptcha-frictionless/src/procaptchaFrictionless.ts index 0cc24b5c5f..2fa7addb4d 100644 --- a/packages/procaptcha-frictionless/src/procaptchaFrictionless.ts +++ b/packages/procaptcha-frictionless/src/procaptchaFrictionless.ts @@ -265,6 +265,7 @@ export const mountProcaptchaFrictionless = ( let restartCount = 0; const restartComponentTimeout = () => { + if (destroyed) return; const delay = getRestartDelayMs(restartCount); restartCount += 1; const timer = setTimeout(() => { @@ -324,6 +325,7 @@ export const mountProcaptchaFrictionless = ( autoStart = false, escalationCoords?: RetryCoords, ): Promise => { + if (destroyed) return; // Audio is mounted against the visual session /frictionless just minted. audioChosen = audioChosen && offersAudioAlternative(captchaType); @@ -520,6 +522,7 @@ export const mountProcaptchaFrictionless = ( }; const start = async (): Promise => { + if (destroyed) return; // Procaptcha cannot run over plain HTTP (no SubtleCrypto etc.), which // would otherwise fail later with a cryptic provider-selection error. // Surface a clear, non-retrying message instead. @@ -549,6 +552,7 @@ export const mountProcaptchaFrictionless = ( nextSwitchToImage, widgetSlot, ); + if (destroyed) return; const guard = evaluateFrictionlessResult(result); if ("error" === guard.kind) { @@ -603,6 +607,7 @@ export const mountProcaptchaFrictionless = ( state.attemptCount, 5, ).finally(() => { + if (destroyed) return; if (state.attemptCount >= 5) { // Retries swallow the underlying error, so without this a site's // error callback never fires for a failure that retried — it would diff --git a/packages/procaptcha-frictionless/src/tests/procaptchaFrictionless.test.ts b/packages/procaptcha-frictionless/src/tests/procaptchaFrictionless.test.ts index 790b789ac6..dfb39ca7b6 100644 --- a/packages/procaptcha-frictionless/src/tests/procaptchaFrictionless.test.ts +++ b/packages/procaptcha-frictionless/src/tests/procaptchaFrictionless.test.ts @@ -621,6 +621,86 @@ describe("tearing down", () => { }); }); +describe("a detection that lands after destroy", () => { + const deferred = (): { + detectBot: BotDetectionFunction; + release: (result: BotDetectionFunctionResult) => void; + fail: (error: Error) => void; + calls: () => number; + } => { + let release: (result: BotDetectionFunctionResult) => void = () => undefined; + let fail: (error: Error) => void = () => undefined; + const detectBot = vi.fn( + () => + new Promise( + ( + resolve: (result: BotDetectionFunctionResult) => void, + reject: (error: Error) => void, + ) => { + release = resolve; + fail = reject; + }, + ), + ); + return { + detectBot, + release: (result: BotDetectionFunctionResult) => release(result), + fail: (error: Error) => fail(error), + calls: () => detectBot.mock.calls.length, + }; + }; + + test("does not pass a dead widget as human", async () => { + const onHuman = vi.fn<(token: string) => void>(); + const detection$ = deferred(); + widget = mountProcaptchaFrictionless( + container, + props(detection$.detectBot, { callbacks: { onHuman } }), + ); + widget.destroy(); + widget = undefined; + detection$.release(detection(CaptchaType.authenticated)); + await settle(); + expect(onHuman).not.toHaveBeenCalled(); + expect(container.textContent).toBe(""); + }); + + test("does not report an error for a dead widget", async () => { + const onError = vi.fn<(error: Error) => void>(); + const detection$ = deferred(); + widget = mountProcaptchaFrictionless( + container, + props(detection$.detectBot, { callbacks: { onError } }), + ); + widget.destroy(); + widget = undefined; + detection$.release({ + sessionId: "session-1", + } as unknown as BotDetectionFunctionResult); + await settle(); + expect(onError).not.toHaveBeenCalled(); + }); + + test("does not retry detection for a dead widget", async () => { + const longestRetryBackoffMs = 10_000; + vi.useFakeTimers(); + try { + const detection$ = deferred(); + widget = mountProcaptchaFrictionless( + container, + props(detection$.detectBot), + ); + widget.destroy(); + widget = undefined; + detection$.fail(new Error("provider down")); + await vi.advanceTimersByTimeAsync(longestRetryBackoffMs); + expect(detection$.calls()).toBe(1); + } finally { + vi.useRealTimers(); + } + }); +}); + describe("several widgets on one page", () => { test("gives each widget for a site key its own slot, and reuses a freed one", async () => { const detectBot = vi