From 563a52185c9a70f7347ed217312e2a29e69a1a0d Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Wed, 1 Feb 2023 16:32:19 -0800 Subject: [PATCH 1/4] Add pollUntil utility for polling Promises --- .../src/core/request/pollUntil.test.ts | 67 +++++++++++++++++++ airbyte-webapp/src/core/request/pollUntil.ts | 20 ++++++ 2 files changed, 87 insertions(+) create mode 100644 airbyte-webapp/src/core/request/pollUntil.test.ts create mode 100644 airbyte-webapp/src/core/request/pollUntil.ts diff --git a/airbyte-webapp/src/core/request/pollUntil.test.ts b/airbyte-webapp/src/core/request/pollUntil.test.ts new file mode 100644 index 0000000000000..3d164fbbf0d80 --- /dev/null +++ b/airbyte-webapp/src/core/request/pollUntil.test.ts @@ -0,0 +1,67 @@ +import { pollUntil } from "./pollUntil"; + +// a toy promise that can be polled for a specific response +const fourZerosAndThenSeven = () => { + let _callCount = 0; + return () => Promise.resolve([0, 0, 0, 0, 7][_callCount++]); +}; +// eslint-disable-next-line +const truthyResponse = (x: any) => !!x; + +describe("pollUntil", () => { + describe("when maxTimeout is not provided", () => { + it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { + const pollableFn = fourZerosAndThenSeven(); + + return expect(pollUntil(pollableFn, truthyResponse, 1)).resolves.toBe(7); + }); + }); + + describe("when condition returns true before maxTimeout is reached", () => { + it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { + const pollableFn = fourZerosAndThenSeven(); + + return expect(pollUntil(pollableFn, truthyResponse, 1, 100)).resolves.toBe(7); + }); + }); + + describe("when maxTimeout is reached before condition returns true", () => { + it("resolves to false", () => { + const pollableFn = fourZerosAndThenSeven(); + + return expect(pollUntil(pollableFn, truthyResponse, 100, 1)).resolves.toBe(false); + }); + + // Because the timing of the polling depends on both the provided `interval` and the + // execution time of `apiFn`, the timing of polling iterations isn't entirely + // deterministic; it's precise enough for its job, but it's difficult to make precise + // test assertions about polling behavior without long interval/maxTimeout bogging + // down the test suite. + it("calls its apiFn arg no more than (maxTimeout / interval) times", async () => { + let _callCount = 0; + let lastCalledValue = 999; + const pollableFn = () => + Promise.resolve([1, 2, 3, 4, 5][_callCount++]).then((val) => { + lastCalledValue = val; + return val; + }); + + await pollUntil(pollableFn, (_) => false, 20, 78); + + // In theory, this is what just happened: + // | time elapsed | value (source) | + // |--------------+-----------------| + // | 0ms | 1 (poll) | + // | 20ms | 2 (poll) | + // | 40ms | 3 (poll) | + // | 60ms | 4 (poll) | + // | 78ms | false (timeout) | + // + // In practice, since the polling interval isn't started until after `apiFn` + // resolves to a value, the actual call counts are slightly nondeterministic. We + // could ignore that fact with a slow enough interval, but who wants slow tests? + expect(lastCalledValue > 2).toBe(true); + expect(lastCalledValue <= 4).toBe(true); + }); + }); +}); diff --git a/airbyte-webapp/src/core/request/pollUntil.ts b/airbyte-webapp/src/core/request/pollUntil.ts new file mode 100644 index 0000000000000..756e54dfacde4 --- /dev/null +++ b/airbyte-webapp/src/core/request/pollUntil.ts @@ -0,0 +1,20 @@ +import { timer, delay, from, concatMap, takeWhile, last, raceWith, lastValueFrom, NEVER } from "rxjs"; + +// Known issues: +// - the case where `apiFn` returns `false` and `condition(false) === true` is impossible to distinguish from a timeout +export function pollUntil( + apiFn: () => Promise, + condition: (res: ResponseType) => boolean, + interval: number, + maxTimeout?: number +) { + const poll$ = timer(0, interval).pipe( + concatMap((_) => from(apiFn())), + takeWhile((result) => !condition(result), true), + last() + ); + + const timeout$ = maxTimeout ? from([false]).pipe(delay(maxTimeout)) : NEVER; + + return lastValueFrom(poll$.pipe(raceWith(timeout$))); +} From 36256749c8a738078dce03d9b92383ce54caa5c6 Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Wed, 1 Feb 2023 17:40:56 -0800 Subject: [PATCH 2/4] poll backend for confirmed enrollment before showing success toast --- .../hooks/useFreeConnectorProgram.ts | 31 +++++++++++++++---- .../src/packages/cloud/locales/en.json | 1 + 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts index 9afc925ea1b3e..7065f315a4a1b 100644 --- a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts +++ b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts @@ -7,6 +7,8 @@ import { useEffectOnce } from "react-use"; import { ToastType } from "components/ui/Toast"; import { MissingConfigError, useConfig } from "config"; +import { pollUntil } from "core/request/pollUntil"; +import { useAppMonitoringService } from "hooks/services/AppMonitoringService"; import { useExperiment } from "hooks/services/Experiment"; import { useNotificationService } from "hooks/services/Notification"; import { useDefaultRequestMiddlewares } from "services/useDefaultRequestMiddlewares"; @@ -30,16 +32,33 @@ export const useFreeConnectorProgram = () => { const [userDidEnroll, setUserDidEnroll] = useState(false); const { formatMessage } = useIntl(); const { registerNotification } = useNotificationService(); + const { trackError } = useAppMonitoringService(); useEffectOnce(() => { if (searchParams.has(STRIPE_SUCCESS_QUERY)) { // Remove the stripe parameter from the URL - setSearchParams({}, { replace: true }); - setUserDidEnroll(true); - registerNotification({ - id: "fcp/enrolled", - text: formatMessage({ id: "freeConnectorProgram.enroll.success" }), - type: ToastType.SUCCESS, + pollUntil( + () => webBackendGetFreeConnectorProgramInfoForWorkspace({ workspaceId }, requestOptions), + ({ hasPaymentAccountSaved }) => hasPaymentAccountSaved, + 1000, + 10000 + ).then((maybeFcpInfo) => { + if (maybeFcpInfo) { + setSearchParams({}, { replace: true }); + setUserDidEnroll(true); + registerNotification({ + id: "fcp/enrollment-success", + text: formatMessage({ id: "freeConnectorProgram.enroll.success" }), + type: ToastType.SUCCESS, + }); + } else { + trackError(new Error("Unable to confirm Free Connector Program enrollment before timeout"), { workspaceId }); + registerNotification({ + id: "fcp/enrollment-failure", + text: formatMessage({ id: "freeConnectorProgram.enroll.failure" }), + type: ToastType.ERROR, + }); + } }); } }); diff --git a/airbyte-webapp/src/packages/cloud/locales/en.json b/airbyte-webapp/src/packages/cloud/locales/en.json index 7858df4591e32..c55750732ec9d 100644 --- a/airbyte-webapp/src/packages/cloud/locales/en.json +++ b/airbyte-webapp/src/packages/cloud/locales/en.json @@ -182,6 +182,7 @@ "freeConnectorProgram.enrollNow": "Enroll now!", "freeConnectorProgram.enroll.description": "Enroll in the Free Connector Program to use Alpha and Beta connectors for free.", "freeConnectorProgram.enroll.success": "Successfully enrolled in the Free Connector Program", + "freeConnectorProgram.enroll.failure": "Unable to verify that payment details were saved successfully. Please contact support for additional help.", "freeConnectorProgram.enrollmentModal.title": "Free connector program", "freeConnectorProgram.enrollmentModal.free": "Alpha and Beta Connectors are free while you're in the program.The whole Connection is free until both Connectors have moved into General Availability (GA)", "freeConnectorProgram.enrollmentModal.emailNotification": "We will email you before your connection will start being charged.", From 3a31dce6df7fef30b2285709f86c0ad9431d8eaf Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Wed, 1 Feb 2023 17:53:59 -0800 Subject: [PATCH 3/4] Put interval and maxTimeout inside options arg --- airbyte-webapp/src/core/request/pollUntil.test.ts | 8 ++++---- airbyte-webapp/src/core/request/pollUntil.ts | 4 ++-- .../FreeConnectorProgram/hooks/useFreeConnectorProgram.ts | 3 +-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/airbyte-webapp/src/core/request/pollUntil.test.ts b/airbyte-webapp/src/core/request/pollUntil.test.ts index 3d164fbbf0d80..0cc6a66faa7c8 100644 --- a/airbyte-webapp/src/core/request/pollUntil.test.ts +++ b/airbyte-webapp/src/core/request/pollUntil.test.ts @@ -13,7 +13,7 @@ describe("pollUntil", () => { it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, 1)).resolves.toBe(7); + return expect(pollUntil(pollableFn, truthyResponse, { interval: 1 })).resolves.toBe(7); }); }); @@ -21,7 +21,7 @@ describe("pollUntil", () => { it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, 1, 100)).resolves.toBe(7); + return expect(pollUntil(pollableFn, truthyResponse, { interval: 1, maxTimeout: 100 })).resolves.toBe(7); }); }); @@ -29,7 +29,7 @@ describe("pollUntil", () => { it("resolves to false", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, 100, 1)).resolves.toBe(false); + return expect(pollUntil(pollableFn, truthyResponse, { interval: 100, maxTimeout: 1 })).resolves.toBe(false); }); // Because the timing of the polling depends on both the provided `interval` and the @@ -46,7 +46,7 @@ describe("pollUntil", () => { return val; }); - await pollUntil(pollableFn, (_) => false, 20, 78); + await pollUntil(pollableFn, (_) => false, { interval: 20, maxTimeout: 78 }); // In theory, this is what just happened: // | time elapsed | value (source) | diff --git a/airbyte-webapp/src/core/request/pollUntil.ts b/airbyte-webapp/src/core/request/pollUntil.ts index 756e54dfacde4..04b4ee08a7180 100644 --- a/airbyte-webapp/src/core/request/pollUntil.ts +++ b/airbyte-webapp/src/core/request/pollUntil.ts @@ -5,9 +5,9 @@ import { timer, delay, from, concatMap, takeWhile, last, raceWith, lastValueFrom export function pollUntil( apiFn: () => Promise, condition: (res: ResponseType) => boolean, - interval: number, - maxTimeout?: number + options: { interval: number; maxTimeout?: number } ) { + const { interval, maxTimeout } = options; const poll$ = timer(0, interval).pipe( concatMap((_) => from(apiFn())), takeWhile((result) => !condition(result), true), diff --git a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts index 7065f315a4a1b..a36098a78cd38 100644 --- a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts +++ b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts @@ -40,8 +40,7 @@ export const useFreeConnectorProgram = () => { pollUntil( () => webBackendGetFreeConnectorProgramInfoForWorkspace({ workspaceId }, requestOptions), ({ hasPaymentAccountSaved }) => hasPaymentAccountSaved, - 1000, - 10000 + { interval: 1000, maxTimeout: 10000 } ).then((maybeFcpInfo) => { if (maybeFcpInfo) { setSearchParams({}, { replace: true }); From db88c72c346ad2df171a7b27c5571a10d59e60f8 Mon Sep 17 00:00:00 2001 From: Alex Birdsall Date: Mon, 6 Feb 2023 16:29:16 -0800 Subject: [PATCH 4/4] Give units w/ polling options: intervalMs and maxTimeoutMs --- .../src/core/request/pollUntil.test.ts | 24 +++++++++---------- airbyte-webapp/src/core/request/pollUntil.ts | 10 ++++---- .../hooks/useFreeConnectorProgram.ts | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/airbyte-webapp/src/core/request/pollUntil.test.ts b/airbyte-webapp/src/core/request/pollUntil.test.ts index 0cc6a66faa7c8..796e84a3e6a55 100644 --- a/airbyte-webapp/src/core/request/pollUntil.test.ts +++ b/airbyte-webapp/src/core/request/pollUntil.test.ts @@ -9,35 +9,35 @@ const fourZerosAndThenSeven = () => { const truthyResponse = (x: any) => !!x; describe("pollUntil", () => { - describe("when maxTimeout is not provided", () => { + describe("when maxTimeoutMs is not provided", () => { it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, { interval: 1 })).resolves.toBe(7); + return expect(pollUntil(pollableFn, truthyResponse, { intervalMs: 1 })).resolves.toBe(7); }); }); - describe("when condition returns true before maxTimeout is reached", () => { + describe("when condition returns true before maxTimeoutMs is reached", () => { it("calls the provided apiFn until condition returns true and resolves to its final return value", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, { interval: 1, maxTimeout: 100 })).resolves.toBe(7); + return expect(pollUntil(pollableFn, truthyResponse, { intervalMs: 1, maxTimeoutMs: 100 })).resolves.toBe(7); }); }); - describe("when maxTimeout is reached before condition returns true", () => { + describe("when maxTimeoutMs is reached before condition returns true", () => { it("resolves to false", () => { const pollableFn = fourZerosAndThenSeven(); - return expect(pollUntil(pollableFn, truthyResponse, { interval: 100, maxTimeout: 1 })).resolves.toBe(false); + return expect(pollUntil(pollableFn, truthyResponse, { intervalMs: 100, maxTimeoutMs: 1 })).resolves.toBe(false); }); - // Because the timing of the polling depends on both the provided `interval` and the + // Because the timing of the polling depends on both the provided `intervalMs` and the // execution time of `apiFn`, the timing of polling iterations isn't entirely // deterministic; it's precise enough for its job, but it's difficult to make precise - // test assertions about polling behavior without long interval/maxTimeout bogging + // test assertions about polling behavior without long intervalMs/maxTimeoutMs bogging // down the test suite. - it("calls its apiFn arg no more than (maxTimeout / interval) times", async () => { + it("calls its apiFn arg no more than (maxTimeoutMs / intervalMs) times", async () => { let _callCount = 0; let lastCalledValue = 999; const pollableFn = () => @@ -46,7 +46,7 @@ describe("pollUntil", () => { return val; }); - await pollUntil(pollableFn, (_) => false, { interval: 20, maxTimeout: 78 }); + await pollUntil(pollableFn, (_) => false, { intervalMs: 20, maxTimeoutMs: 78 }); // In theory, this is what just happened: // | time elapsed | value (source) | @@ -57,9 +57,9 @@ describe("pollUntil", () => { // | 60ms | 4 (poll) | // | 78ms | false (timeout) | // - // In practice, since the polling interval isn't started until after `apiFn` + // In practice, since the polling intervalMs isn't started until after `apiFn` // resolves to a value, the actual call counts are slightly nondeterministic. We - // could ignore that fact with a slow enough interval, but who wants slow tests? + // could ignore that fact with a slow enough intervalMs, but who wants slow tests? expect(lastCalledValue > 2).toBe(true); expect(lastCalledValue <= 4).toBe(true); }); diff --git a/airbyte-webapp/src/core/request/pollUntil.ts b/airbyte-webapp/src/core/request/pollUntil.ts index 04b4ee08a7180..7349cf0459e0b 100644 --- a/airbyte-webapp/src/core/request/pollUntil.ts +++ b/airbyte-webapp/src/core/request/pollUntil.ts @@ -5,16 +5,16 @@ import { timer, delay, from, concatMap, takeWhile, last, raceWith, lastValueFrom export function pollUntil( apiFn: () => Promise, condition: (res: ResponseType) => boolean, - options: { interval: number; maxTimeout?: number } + options: { intervalMs: number; maxTimeoutMs?: number } ) { - const { interval, maxTimeout } = options; - const poll$ = timer(0, interval).pipe( - concatMap((_) => from(apiFn())), + const { intervalMs, maxTimeoutMs } = options; + const poll$ = timer(0, intervalMs).pipe( + concatMap(() => from(apiFn())), takeWhile((result) => !condition(result), true), last() ); - const timeout$ = maxTimeout ? from([false]).pipe(delay(maxTimeout)) : NEVER; + const timeout$ = maxTimeoutMs ? from([false]).pipe(delay(maxTimeoutMs)) : NEVER; return lastValueFrom(poll$.pipe(raceWith(timeout$))); } diff --git a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts index a36098a78cd38..d21e145da9a86 100644 --- a/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts +++ b/airbyte-webapp/src/packages/cloud/components/experiments/FreeConnectorProgram/hooks/useFreeConnectorProgram.ts @@ -40,7 +40,7 @@ export const useFreeConnectorProgram = () => { pollUntil( () => webBackendGetFreeConnectorProgramInfoForWorkspace({ workspaceId }, requestOptions), ({ hasPaymentAccountSaved }) => hasPaymentAccountSaved, - { interval: 1000, maxTimeout: 10000 } + { intervalMs: 1000, maxTimeoutMs: 10000 } ).then((maybeFcpInfo) => { if (maybeFcpInfo) { setSearchParams({}, { replace: true });