Skip to content

Commit befa091

Browse files
Merge pull request #8573 from Shopify/gonzalo/device-authorization-gateway-retries
Retry gateway failures during device authorization
2 parents 37d2b06 + 8053cf7 commit befa091

5 files changed

Lines changed: 202 additions & 24 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/cli-kit': patch
3+
---
4+
5+
Retry gateway failures during device authorization before reporting an expected service error.
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
const GATEWAY_ERROR_STATUSES = new Set([502, 503, 504])
2+
3+
/**
4+
* Checks whether an HTTP status indicates a gateway-level failure.
5+
*
6+
* @param status - The HTTP status to check.
7+
* @returns Whether the status is 502, 503, or 504.
8+
*/
9+
export function isGatewayErrorStatus(status: number | undefined): boolean {
10+
return status !== undefined && GATEWAY_ERROR_STATUSES.has(status)
11+
}

‎packages/cli-kit/src/private/node/session/device-authorization.test.ts‎

Lines changed: 120 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@ import {clientId} from './identity.js'
77
import {IdentityToken} from './schema.js'
88
import {exchangeDeviceCodeForAccessToken} from './exchange.js'
99
import {identityFqdn} from '../../../public/node/context/fqdn.js'
10+
import {recordRetry} from '../../../public/node/analytics.js'
1011
import {shopifyFetch} from '../../../public/node/http.js'
1112
import {isTTY} from '../../../public/node/ui.js'
1213
import {err, ok} from '../../../public/node/result.js'
13-
import {AbortError} from '../../../public/node/error.js'
14-
import {isCI, openURL} from '../../../public/node/system.js'
14+
import {AbortError, BugError} from '../../../public/node/error.js'
15+
import {isCI, openURL, sleep} from '../../../public/node/system.js'
1516
import * as output from '../../../public/node/output.js'
1617

1718
import {beforeEach, describe, expect, test, vi} from 'vitest'
1819
import {Response} from 'node-fetch'
1920

2021
vi.mock('../../../public/node/context/fqdn.js')
2122
vi.mock('./identity')
23+
vi.mock('../../../public/node/analytics.js')
2224
vi.mock('../../../public/node/http.js')
2325
vi.mock('../../../public/node/ui.js')
2426
vi.mock('./exchange.js')
@@ -160,7 +162,7 @@ describe('requestDeviceAuthorization', () => {
160162
expect(outputInfo).not.toHaveBeenCalledWith('👉 Press any key to open the login page on your browser')
161163
})
162164

163-
test('when the response is not valid JSON, throw an error with context', async () => {
165+
test('when the response is not valid JSON, throw an error with context without retrying', async () => {
164166
// Given
165167
const response = new Response('not valid JSON')
166168
Object.defineProperty(response, 'status', {value: 200})
@@ -169,10 +171,121 @@ describe('requestDeviceAuthorization', () => {
169171
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
170172
vi.mocked(clientId).mockReturnValue('clientId')
171173

172-
// When/Then
173-
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
174+
// When
175+
const request = requestDeviceAuthorization(['scope1', 'scope2'])
176+
177+
// Then
178+
await expect(request).rejects.toBeInstanceOf(BugError)
179+
await expect(request).rejects.toThrowError(
174180
'Received invalid response from authorization service (HTTP 200). Response could not be parsed as valid JSON. If this issue persists, please contact support at https://help.shopify.com',
175181
)
182+
expect(shopifyFetch).toHaveBeenCalledTimes(1)
183+
})
184+
185+
test('retries a gateway response and returns once the service recovers', async () => {
186+
// Given
187+
vi.mocked(shopifyFetch)
188+
.mockResolvedValueOnce(new Response('Service unavailable', {status: 503}))
189+
.mockResolvedValueOnce(new Response(JSON.stringify(data), {status: 200}))
190+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
191+
vi.mocked(clientId).mockReturnValue('clientId')
192+
193+
// When
194+
const got = await requestDeviceAuthorization(['scope1', 'scope2'])
195+
196+
// Then
197+
expect(got).toEqual(dataExpected)
198+
expect(shopifyFetch).toHaveBeenCalledTimes(2)
199+
expect(sleep).toHaveBeenCalledOnce()
200+
expect(sleep).toHaveBeenCalledWith(0.2)
201+
expect(recordRetry).toHaveBeenCalledWith(
202+
'https://fqdn.com/oauth/device_authorization',
203+
'device-authorization-gateway-error',
204+
)
205+
})
206+
207+
test.each([
208+
{retryAfter: '3', expectedDelay: 3},
209+
{retryAfter: '0', expectedDelay: 0},
210+
{retryAfter: 'Fri, 18 Sep 2026 12:00:10 GMT', expectedDelay: 10},
211+
{retryAfter: 'Fri, 18 Sep 2026 11:59:59 GMT', expectedDelay: 0},
212+
{retryAfter: 'invalid', expectedDelay: 0.2},
213+
{retryAfter: '-1', expectedDelay: 0.2},
214+
{retryAfter: '', expectedDelay: 0.2},
215+
{retryAfter: ' ', expectedDelay: 0.2},
216+
])('uses $expectedDelay seconds for Retry-After "$retryAfter"', async ({retryAfter, expectedDelay}) => {
217+
const now = vi.spyOn(Date, 'now').mockReturnValue(new Date('2026-09-18T12:00:00Z').getTime())
218+
vi.mocked(shopifyFetch)
219+
.mockResolvedValueOnce(new Response('Service unavailable', {status: 503, headers: {'Retry-After': retryAfter}}))
220+
.mockResolvedValueOnce(new Response(JSON.stringify(data), {status: 200}))
221+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
222+
vi.mocked(clientId).mockReturnValue('clientId')
223+
224+
try {
225+
const result = await requestDeviceAuthorization(['scope1', 'scope2'])
226+
227+
expect(result).toEqual(dataExpected)
228+
expect(shopifyFetch).toHaveBeenCalledTimes(2)
229+
expect(sleep).toHaveBeenCalledExactlyOnceWith(expectedDelay)
230+
} finally {
231+
now.mockRestore()
232+
}
233+
})
234+
235+
test('uses the Retry-After from each response and stops after two retries', async () => {
236+
vi.mocked(shopifyFetch)
237+
.mockResolvedValueOnce(new Response('Service unavailable', {status: 503, headers: {'Retry-After': '2'}}))
238+
.mockResolvedValueOnce(new Response('Service unavailable', {status: 503, headers: {'Retry-After': '4'}}))
239+
.mockResolvedValueOnce(new Response('Service unavailable', {status: 503, headers: {'Retry-After': '6'}}))
240+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
241+
vi.mocked(clientId).mockReturnValue('clientId')
242+
243+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toBeInstanceOf(AbortError)
244+
245+
expect(shopifyFetch).toHaveBeenCalledTimes(3)
246+
expect(sleep).toHaveBeenCalledTimes(2)
247+
expect(sleep).toHaveBeenNthCalledWith(1, 2)
248+
expect(sleep).toHaveBeenNthCalledWith(2, 4)
249+
})
250+
251+
test.each([502, 503, 504])(
252+
'when HTTP %i gateway responses persist, throw an expected error after two retries',
253+
async (status) => {
254+
// Given
255+
vi.mocked(shopifyFetch).mockImplementation(async () => new Response('Service unavailable', {status}))
256+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
257+
vi.mocked(clientId).mockReturnValue('clientId')
258+
259+
// When
260+
const request = requestDeviceAuthorization(['scope1', 'scope2'])
261+
262+
// Then
263+
await expect(request).rejects.toBeInstanceOf(AbortError)
264+
await expect(request).rejects.toThrowError(
265+
`Received invalid response from authorization service (HTTP ${status}). The service may be experiencing issues. Response could not be parsed as valid JSON. If this issue persists, please contact support at https://help.shopify.com`,
266+
)
267+
expect(shopifyFetch).toHaveBeenCalledTimes(3)
268+
expect(sleep).toHaveBeenNthCalledWith(1, 0.2)
269+
expect(sleep).toHaveBeenNthCalledWith(2, 0.4)
270+
expect(recordRetry).toHaveBeenCalledTimes(2)
271+
},
272+
)
273+
274+
test('preserves the existing error for a persistent JSON gateway response', async () => {
275+
// Given
276+
vi.mocked(shopifyFetch).mockImplementation(
277+
async () => new Response(JSON.stringify({error: 'service_unavailable'}), {status: 503}),
278+
)
279+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
280+
vi.mocked(clientId).mockReturnValue('clientId')
281+
282+
// When
283+
const request = requestDeviceAuthorization(['scope1', 'scope2'])
284+
285+
// Then
286+
await expect(request).rejects.toBeInstanceOf(BugError)
287+
await expect(request).rejects.toThrowError('Failed to start authorization process')
288+
expect(shopifyFetch).toHaveBeenCalledTimes(3)
176289
})
177290

178291
test('when the response is empty, throw an error with empty body message', async () => {
@@ -206,7 +319,7 @@ describe('requestDeviceAuthorization', () => {
206319
)
207320
})
208321

209-
test('when the server returns a 500 error with non-JSON response, throw an error with server issue message', async () => {
322+
test('when the server returns a 500 error with non-JSON response, throw an error without retrying', async () => {
210323
// Given
211324
const response = new Response('Internal Server Error')
212325
Object.defineProperty(response, 'status', {value: 500})
@@ -219,6 +332,7 @@ describe('requestDeviceAuthorization', () => {
219332
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
220333
'Received invalid response from authorization service (HTTP 500). The service may be experiencing issues. Response could not be parsed as valid JSON. If this issue persists, please contact support at https://help.shopify.com',
221334
)
335+
expect(shopifyFetch).toHaveBeenCalledTimes(1)
222336
})
223337

224338
test('when response.text() fails, throw an error about network/streaming issue', async () => {

‎packages/cli-kit/src/private/node/session/device-authorization.ts‎

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import {clientId} from './identity.js'
22
import {exchangeDeviceCodeForAccessToken} from './exchange.js'
33
import {IdentityToken} from './schema.js'
4+
import {isGatewayErrorStatus} from '../api/status-checks.js'
45
import {identityFqdn} from '../../../public/node/context/fqdn.js'
6+
import {recordRetry} from '../../../public/node/analytics.js'
57
import {shopifyFetch} from '../../../public/node/http.js'
68
import {outputContent, outputDebug, outputInfo, outputToken} from '../../../public/node/output.js'
79
import {AbortError, BugError} from '../../../public/node/error.js'
8-
import {isCI, openURL} from '../../../public/node/system.js'
10+
import {isCI, openURL, sleep} from '../../../public/node/system.js'
911

1012
import {Response} from 'node-fetch'
1113

@@ -18,6 +20,9 @@ export interface DeviceAuthorizationResponse {
1820
interval?: number
1921
}
2022

23+
const GATEWAY_ERROR_RETRY_LIMIT = 2
24+
const GATEWAY_ERROR_INITIAL_RETRY_DELAY_SECONDS = 0.2
25+
2126
/**
2227
* Initiate a device authorization flow.
2328
* This will return a DeviceAuthorizationResponse containing the URL where user
@@ -34,22 +39,7 @@ export async function requestDeviceAuthorization(scopes: string[]): Promise<Devi
3439
const queryParams = {client_id: identityClientId, scope: scopes.join(' ')}
3540
const url = `https://${fqdn}/oauth/device_authorization`
3641

37-
const response = await shopifyFetch(url, {
38-
method: 'POST',
39-
headers: {'Content-type': 'application/x-www-form-urlencoded'},
40-
body: convertRequestToParams(queryParams),
41-
})
42-
43-
// First read the response body as text so we have it for debugging
44-
let responseText: string
45-
try {
46-
responseText = await response.text()
47-
} catch (error) {
48-
throw new BugError(
49-
`Failed to read response from authorization service (HTTP ${response.status}). Network or streaming error occurred.`,
50-
'Check your network connection and try again.',
51-
)
52-
}
42+
const {response, responseText} = await requestDeviceAuthorizationResponse(url, convertRequestToParams(queryParams))
5343

5444
// Now try to parse the text as JSON
5545
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -59,6 +49,9 @@ export async function requestDeviceAuthorization(scopes: string[]): Promise<Devi
5949
} catch {
6050
// JSON.parse failed, handle the parsing error
6151
const errorMessage = buildAuthorizationParseErrorMessage(response, responseText)
52+
if (isGatewayErrorStatus(response.status)) {
53+
throw new AbortError(errorMessage)
54+
}
6255
throw new BugError(errorMessage)
6356
}
6457

@@ -153,6 +146,60 @@ export async function pollForDeviceAuthorization(code: string, interval = 5): Pr
153146
})
154147
}
155148

149+
async function requestDeviceAuthorizationResponse(
150+
url: string,
151+
body: string,
152+
gatewayRetriesUsed = 0,
153+
): Promise<{response: Response; responseText: string}> {
154+
const response = await shopifyFetch(url, {
155+
method: 'POST',
156+
headers: {'Content-type': 'application/x-www-form-urlencoded'},
157+
body,
158+
})
159+
160+
let responseText: string
161+
try {
162+
responseText = await response.text()
163+
} catch {
164+
throw new BugError(
165+
`Failed to read response from authorization service (HTTP ${response.status}). Network or streaming error occurred.`,
166+
'Check your network connection and try again.',
167+
)
168+
}
169+
170+
if (!isGatewayErrorStatus(response.status) || gatewayRetriesUsed >= GATEWAY_ERROR_RETRY_LIMIT) {
171+
return {response, responseText}
172+
}
173+
174+
const retryNumber = gatewayRetriesUsed + 1
175+
const retryDelaySeconds =
176+
retryAfterDelaySeconds(response.headers.get('retry-after')) ??
177+
GATEWAY_ERROR_INITIAL_RETRY_DELAY_SECONDS * 2 ** gatewayRetriesUsed
178+
recordRetry(url, 'device-authorization-gateway-error')
179+
outputDebug(
180+
`Scheduling device authorization retry #${retryNumber} after HTTP ${response.status} in ${retryDelaySeconds} seconds`,
181+
)
182+
await sleep(retryDelaySeconds)
183+
184+
return requestDeviceAuthorizationResponse(url, body, retryNumber)
185+
}
186+
187+
function retryAfterDelaySeconds(retryAfter: string | null): number | undefined {
188+
const value = retryAfter?.trim()
189+
if (!value) return undefined
190+
191+
// Retry-After can specify either a delay in seconds or an HTTP date.
192+
const delaySeconds = Number(value)
193+
if (!Number.isNaN(delaySeconds)) {
194+
return Number.isFinite(delaySeconds) && delaySeconds >= 0 ? delaySeconds : undefined
195+
}
196+
197+
const retryAt = Date.parse(value)
198+
if (Number.isNaN(retryAt)) return undefined
199+
200+
return Math.max(0, (retryAt - Date.now()) / 1000)
201+
}
202+
156203
function convertRequestToParams(queryParams: {client_id: string; scope: string}): string {
157204
return new URLSearchParams(Object.entries(queryParams).filter(([, value]) => Boolean(value))).toString()
158205
}

‎packages/cli-kit/src/public/node/error/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {normalizePath} from '../path.js'
22
import {flushStdout, OutputMessage, stringifyMessage, TokenizedString} from '../output.js'
33
import {tokenItemToString, type InlineToken, type TokenItem} from '../../../private/node/ui/components/token-item.js'
44
import {hasRateLimitCode} from '../../../private/node/analytics/graphql-error-codes.js'
5+
import {isGatewayErrorStatus} from '../../../private/node/api/status-checks.js'
56

67
import {Errors} from '@oclif/core'
78

@@ -249,7 +250,7 @@ function isExpectedApiError(error: Error): boolean {
249250
return false
250251
}
251252
const status = candidate.response.status
252-
if (status === 401 || status === 429 || status === 502 || status === 503 || status === 504) {
253+
if (status === 401 || status === 429 || isGatewayErrorStatus(status)) {
253254
return true
254255
}
255256
return hasRateLimitCode(candidate.response.errors)

0 commit comments

Comments
 (0)