Skip to content

🪟 🐛 Poll backend for Free Connector Program enrollment success #22289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions airbyte-webapp/src/core/request/pollUntil.test.ts
Original file line number Diff line number Diff line change
@@ -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 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, { intervalMs: 1 })).resolves.toBe(7);
});
});

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, { intervalMs: 1, maxTimeoutMs: 100 })).resolves.toBe(7);
});
});

describe("when maxTimeoutMs is reached before condition returns true", () => {
it("resolves to false", () => {
const pollableFn = fourZerosAndThenSeven();

return expect(pollUntil(pollableFn, truthyResponse, { intervalMs: 100, maxTimeoutMs: 1 })).resolves.toBe(false);
});

// 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 intervalMs/maxTimeoutMs bogging
// down the test suite.
it("calls its apiFn arg no more than (maxTimeoutMs / intervalMs) 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, { intervalMs: 20, maxTimeoutMs: 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 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 intervalMs, but who wants slow tests?
expect(lastCalledValue > 2).toBe(true);
expect(lastCalledValue <= 4).toBe(true);
});
});
});
20 changes: 20 additions & 0 deletions airbyte-webapp/src/core/request/pollUntil.ts
Original file line number Diff line number Diff line change
@@ -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<ResponseType>(
apiFn: () => Promise<ResponseType>,
condition: (res: ResponseType) => boolean,
options: { intervalMs: number; maxTimeoutMs?: number }
) {
const { intervalMs, maxTimeoutMs } = options;
const poll$ = timer(0, intervalMs).pipe(
concatMap(() => from(apiFn())),
takeWhile((result) => !condition(result), true),
last()
);

const timeout$ = maxTimeoutMs ? from([false]).pipe(delay(maxTimeoutMs)) : NEVER;

return lastValueFrom(poll$.pipe(raceWith(timeout$)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -30,16 +32,32 @@ 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(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have pretty similar code in

useEffectOnce(() => {
// If we are coming back from a successfull stripe checkout ...
if (searchParams.has(STRIPE_SUCCESS_QUERY)) {
// Remove the stripe parameter from the URL
setSearchParams({}, { replace: true });
// If the workspace doesn't have a recent increase in credits our server has not yet
// received the Stripe callback or updated the workspace information. We're going to
// switch into a loading mode and relaod the workspace every 3s from now on until
// the workspace has received the credit update (see useEffect below)
if (!hasRecentCreditIncrease(cloudWorkspace)) {
setIsWaitingForCredits(true);
retryIntervalId.current = window.setInterval(() => {
invalidateCloudWorkspace();
}, 3000);
}
}
return () => clearInterval(retryIntervalId.current);
});
Maybe it's worth switching that also over to use the pollUntil utility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout. We also do a bunch of manual interval wrangling for the health check endpoint, too; it would be good to consolidate our polling approaches.

That said, it would be nice to get this change merged to close an annoying edge case first, so I'm inclined to ship those updates as a follow up PR.

() => webBackendGetFreeConnectorProgramInfoForWorkspace({ workspaceId }, requestOptions),
({ hasPaymentAccountSaved }) => hasPaymentAccountSaved,
{ intervalMs: 1000, maxTimeoutMs: 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,
});
}
});
}
});
Expand Down
1 change: 1 addition & 0 deletions airbyte-webapp/src/packages/cloud/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@
"freeConnectorProgram.enrollNow": "Enroll now!",
"freeConnectorProgram.enroll.description": "Enroll in the <b>Free Connector Program</b> to use Alpha and Beta connectors for <b>free</b>.",
"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": "<p1>Alpha and Beta Connectors are free while you're in the program.</p1><p2>The whole Connection is free until both Connectors have moved into General Availability (GA)</p2>",
"freeConnectorProgram.enrollmentModal.emailNotification": "We will email you before your connection will start being charged.",
Expand Down