Skip to content

Commit a96c951

Browse files
committed
retry logic
1 parent 4e38e6d commit a96c951

File tree

3 files changed

+97
-22
lines changed

3 files changed

+97
-22
lines changed

site/src/components/GitDeviceAuth/GitDeviceAuth.tsx

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import CircularProgress from "@mui/material/CircularProgress";
55
import Link from "@mui/material/Link";
66
import type { ApiErrorResponse } from "api/errors";
77
import type { ExternalAuthDevice } from "api/typesGenerated";
8+
import { isAxiosError } from "axios";
89
import { Alert, AlertDetail } from "components/Alert/Alert";
910
import { CopyButton } from "components/CopyButton/CopyButton";
1011
import type { FC } from "react";
@@ -14,6 +15,59 @@ interface GitDeviceAuthProps {
1415
deviceExchangeError?: ApiErrorResponse;
1516
}
1617

18+
const DeviceExchangeError = {
19+
AuthorizationPending: "authorization_pending",
20+
SlowDown: "slow_down",
21+
ExpiredToken: "expired_token",
22+
AccessDenied: "access_denied",
23+
} as const;
24+
25+
export const isExchangeErrorRetryable = (_: number, error: unknown) => {
26+
if (!isAxiosError(error)) {
27+
return false;
28+
}
29+
const detail = error.response?.data?.detail;
30+
return (
31+
detail === DeviceExchangeError.AuthorizationPending ||
32+
detail === DeviceExchangeError.SlowDown
33+
);
34+
};
35+
36+
/**
37+
* The OAuth2 specification (https://datatracker.ietf.org/doc/html/rfc8628)
38+
* describes how the client should handle retries. This function returns a
39+
* closure that implements the retry logic described in the specification.
40+
* The closure should be memoized because it stores state.
41+
*/
42+
export const newRetryDelay = (initialInterval: number | undefined) => {
43+
// "If no value is provided, clients MUST use 5 as the default."
44+
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.2
45+
let interval = initialInterval ?? 5;
46+
let lastFailureCountHandled = 0;
47+
return (failureCount: number, error: unknown) => {
48+
const isSlowDown =
49+
isAxiosError(error) &&
50+
error.response?.data.detail === DeviceExchangeError.SlowDown;
51+
// We check the failure count to ensure we increase the interval
52+
// at most once per failure.
53+
if (isSlowDown && lastFailureCountHandled < failureCount) {
54+
lastFailureCountHandled = failureCount;
55+
// https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
56+
// "the interval MUST be increased by 5 seconds for this and all subsequent requests"
57+
interval += 5;
58+
}
59+
let extraDelay = 0;
60+
if (isSlowDown) {
61+
// I found GitHub is very strict about their rate limits, and they'll block
62+
// even if the request is 500ms earlier than they expect. This may happen due to
63+
// e.g. network latency, so it's best to cool down for longer if GitHub just
64+
// rejected our request.
65+
extraDelay = 5;
66+
}
67+
return (interval + extraDelay) * 1000;
68+
};
69+
};
70+
1771
export const GitDeviceAuth: FC<GitDeviceAuthProps> = ({
1872
externalAuthDevice,
1973
deviceExchangeError,
@@ -27,16 +81,26 @@ export const GitDeviceAuth: FC<GitDeviceAuthProps> = ({
2781
if (deviceExchangeError) {
2882
// See https://datatracker.ietf.org/doc/html/rfc8628#section-3.5
2983
switch (deviceExchangeError.detail) {
30-
case "authorization_pending":
84+
case DeviceExchangeError.AuthorizationPending:
85+
break;
86+
case DeviceExchangeError.SlowDown:
87+
status = (
88+
<div>
89+
{status}
90+
<Alert severity="warning">
91+
Rate limit reached. Waiting a few seconds before retrying...
92+
</Alert>
93+
</div>
94+
);
3195
break;
32-
case "expired_token":
96+
case DeviceExchangeError.ExpiredToken:
3397
status = (
3498
<Alert severity="error">
3599
The one-time code has expired. Refresh to get a new one!
36100
</Alert>
37101
);
38102
break;
39-
case "access_denied":
103+
case DeviceExchangeError.AccessDenied:
40104
status = (
41105
<Alert severity="error">Access to the Git provider was denied.</Alert>
42106
);

site/src/pages/ExternalAuthPage/ExternalAuthPage.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,16 @@ import {
55
externalAuthDevice,
66
externalAuthProvider,
77
} from "api/queries/externalAuth";
8+
import {
9+
isExchangeErrorRetryable,
10+
newRetryDelay,
11+
} from "components/GitDeviceAuth/GitDeviceAuth";
812
import { isAxiosError } from "axios";
913
import { SignInLayout } from "components/SignInLayout/SignInLayout";
1014
import { Welcome } from "components/Welcome/Welcome";
1115
import { useAuthenticated } from "contexts/auth/RequireAuth";
1216
import type { FC } from "react";
17+
import { useMemo } from "react";
1318
import { useQuery, useQueryClient } from "react-query";
1419
import { useParams, useSearchParams } from "react-router-dom";
1520
import ExternalAuthPageView from "./ExternalAuthPageView";
@@ -32,17 +37,22 @@ const ExternalAuthPage: FC = () => {
3237
Boolean(externalAuthProviderQuery.data?.device),
3338
refetchOnMount: false,
3439
});
40+
const retryDelay = useMemo(
41+
() => newRetryDelay(externalAuthDeviceQuery.data?.interval),
42+
[externalAuthDeviceQuery.data],
43+
);
3544
const exchangeExternalAuthDeviceQuery = useQuery({
3645
...exchangeExternalAuthDevice(
3746
provider,
3847
externalAuthDeviceQuery.data?.device_code ?? "",
3948
queryClient,
4049
),
4150
enabled: Boolean(externalAuthDeviceQuery.data),
42-
retry: true,
43-
retryDelay: (externalAuthDeviceQuery.data?.interval || 5) * 1000,
44-
refetchOnWindowFocus: (query) =>
45-
query.state.status === "success" ? false : "always",
51+
retry: isExchangeErrorRetryable,
52+
retryDelay,
53+
// We don't want to refetch the query outside of the standard retry
54+
// logic, because the device auth flow is very strict about rate limits.
55+
refetchOnWindowFocus: false,
4656
});
4757

4858
if (externalAuthProviderQuery.isLoading || !externalAuthProviderQuery.data) {

site/src/pages/LoginOAuthDevicePage/LoginOAuthDevicePage.tsx

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,15 @@ import {
66
import { isAxiosError } from "axios";
77
import { SignInLayout } from "components/SignInLayout/SignInLayout";
88
import { Welcome } from "components/Welcome/Welcome";
9-
import { useEffect } from "react";
9+
import { useEffect, useMemo } from "react";
1010
import type { FC } from "react";
1111
import { useQuery } from "react-query";
1212
import { useSearchParams } from "react-router-dom";
1313
import LoginOAuthDevicePageView from "./LoginOAuthDevicePageView";
14-
15-
const isErrorRetryable = (error: unknown) => {
16-
if (!isAxiosError(error)) {
17-
return false;
18-
}
19-
return error.response?.data?.detail === "authorization_pending";
20-
};
14+
import {
15+
newRetryDelay,
16+
isExchangeErrorRetryable,
17+
} from "components/GitDeviceAuth/GitDeviceAuth";
2118

2219
// The page is hardcoded to only use GitHub,
2320
// as that's the only OAuth2 login provider in our backend
@@ -38,19 +35,23 @@ const LoginOAuthDevicePage: FC = () => {
3835
...getGitHubDevice(),
3936
refetchOnMount: false,
4037
});
38+
39+
const retryDelay = useMemo(
40+
() => newRetryDelay(externalAuthDeviceQuery.data?.interval),
41+
[externalAuthDeviceQuery.data],
42+
);
43+
4144
const exchangeExternalAuthDeviceQuery = useQuery({
4245
...getGitHubDeviceFlowCallback(
4346
externalAuthDeviceQuery.data?.device_code ?? "",
4447
state,
4548
),
4649
enabled: Boolean(externalAuthDeviceQuery.data),
47-
retry: (_, error) => isErrorRetryable(error),
48-
retryDelay: (externalAuthDeviceQuery.data?.interval || 5) * 1000,
49-
refetchOnWindowFocus: (query) =>
50-
query.state.status === "success" ||
51-
(query.state.error != null && !isErrorRetryable(query.state.error))
52-
? false
53-
: "always",
50+
retry: isExchangeErrorRetryable,
51+
retryDelay,
52+
// We don't want to refetch the query outside of the standard retry
53+
// logic, because the device auth flow is very strict about rate limits.
54+
refetchOnWindowFocus: false,
5455
});
5556

5657
useEffect(() => {

0 commit comments

Comments
 (0)