Skip to content

Commit 68d8802

Browse files
committed
Automatic rate limit restoration w/ admin API calls
1 parent dec24f5 commit 68d8802

File tree

4 files changed

+141
-26
lines changed

4 files changed

+141
-26
lines changed

packages/app/src/cli/api/admin-as-app.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ interface AdminAsAppRequestOptions<TResult, TVariables extends Variables> {
1313
query: TypedDocumentNode<TResult, TVariables>
1414
session: AdminSession
1515
variables?: TVariables
16+
autoRateLimitRestore?: boolean
1617
}
1718

1819
/**
@@ -33,6 +34,10 @@ async function setupAdminAsAppRequest(session: AdminSession) {
3334
/**
3435
* Executes a GraphQL query against the Shopify Admin API, on behalf of the app. Uses typed documents.
3536
*
37+
* If `autoRateLimitRestore` is true, the function will wait for a period of time such that the rate limit consumed by
38+
* the query is restored back to its original value. This means this function is suitable for use in loops with
39+
* multiple queries performed.
40+
*
3641
* @param options - The options for the request.
3742
* @returns The response of the query of generic type <T>.
3843
*/
@@ -43,5 +48,6 @@ export async function adminAsAppRequestDoc<TResult, TVariables extends Variables
4348
query: options.query,
4449
...(await setupAdminAsAppRequest(options.session)),
4550
variables: options.variables,
51+
autoRateLimitRestore: options.autoRateLimitRestore,
4652
})
4753
}

packages/cli-kit/src/public/node/api/graphql.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {LocalStorage} from '../local-storage.js'
99
import {ConfSchema, GraphQLRequestKey} from '../../../private/node/conf-store.js'
1010
import {nonRandomUUID} from '../crypto.js'
1111
import {CLI_KIT_VERSION} from '../../common/version.js'
12+
import * as system from '../system.js'
1213
import {test, vi, describe, expect, beforeEach, beforeAll, afterAll, afterEach} from 'vitest'
1314
import {TypedDocumentNode} from '@graphql-typed-document-node/core'
1415
import {setupServer} from 'msw/node'
@@ -18,6 +19,8 @@ let mockedRequestId = 'request-id-123'
1819

1920
vi.spyOn(debugRequest, 'debugLogRequestInfo').mockResolvedValue(undefined)
2021

22+
vi.spyOn(system, 'sleep').mockImplementation(async () => {})
23+
2124
const mockedAddress = 'https://shopify.example/graphql'
2225
const mockVariables = {some: 'variables'}
2326
const mockToken = 'token'
@@ -36,6 +39,14 @@ const handlers = [
3639
data: {
3740
QueryName: {example: 'hello'},
3841
},
42+
extensions: {
43+
cost: {
44+
actualQueryCost: 10,
45+
throttleStatus: {
46+
restoreRate: 10000,
47+
},
48+
},
49+
},
3950
},
4051
{
4152
headers: {
@@ -365,6 +376,47 @@ describe('graphqlRequestDoc', () => {
365376
expect.anything(),
366377
)
367378
})
379+
380+
test('applies rate limit restoration', async () => {
381+
const document = {
382+
kind: 'Document',
383+
definitions: [
384+
{
385+
kind: 'OperationDefinition',
386+
operation: 'query',
387+
name: {kind: 'Name', value: 'QueryName'},
388+
selectionSet: {
389+
kind: 'SelectionSet',
390+
selections: [
391+
{
392+
kind: 'Field',
393+
name: {kind: 'Name', value: 'example'},
394+
},
395+
],
396+
},
397+
},
398+
],
399+
} as unknown as TypedDocumentNode<unknown, unknown>
400+
401+
// When
402+
const res = await graphqlRequestDoc({
403+
query: document,
404+
api: 'mockApi',
405+
url: mockedAddress,
406+
token: mockToken,
407+
addedHeaders: mockedAddedHeaders,
408+
variables: mockVariables,
409+
autoRateLimitRestore: true,
410+
})
411+
expect(res).toMatchInlineSnapshot(`
412+
{
413+
"QueryName": {
414+
"example": "hello",
415+
},
416+
}
417+
`)
418+
expect(system.sleep).toHaveBeenCalledWith(0.001)
419+
})
368420
})
369421

370422
describe('sanitizeVariables', () => {

packages/cli-kit/src/public/node/api/graphql.ts

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ import {
1212
timeIntervalToMilliseconds,
1313
} from '../../../private/node/conf-store.js'
1414
import {LocalStorage} from '../local-storage.js'
15-
import {abortSignalFromRequestBehaviour, requestMode, RequestModeInput} from '../http.js'
15+
import {abortSignalFromRequestBehaviour, RequestBehaviour, requestMode, RequestModeInput} from '../http.js'
1616
import {CLI_KIT_VERSION} from '../../common/version.js'
17+
import {sleep} from '../system.js'
18+
import {outputContent, outputDebug} from '../output.js'
1719
import {
1820
GraphQLClient,
1921
rawRequest,
@@ -65,6 +67,7 @@ type PerformGraphQLRequestOptions<TResult> = GraphQLRequestBaseOptions<TResult>
6567
queryAsString: string
6668
variables?: Variables
6769
unauthorizedHandler?: UnauthorizedHandler
70+
autoRateLimitRestore?: boolean
6871
}
6972

7073
export type GraphQLRequestOptions<T> = GraphQLRequestBaseOptions<T> & {
@@ -77,13 +80,28 @@ export type GraphQLRequestDocOptions<TResult, TVariables> = GraphQLRequestBaseOp
7780
query: TypedDocumentNode<TResult, TVariables> | TypedDocumentNode<TResult, Exact<{[key: string]: never}>>
7881
variables?: TVariables
7982
unauthorizedHandler?: UnauthorizedHandler
83+
autoRateLimitRestore?: boolean
84+
}
85+
86+
interface RunRawGraphQLRequestOptions<TResult> {
87+
client: {
88+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
89+
setAbortSignal: (signal: any) => void
90+
rawRequest: (query: string, variables?: Variables) => Promise<GraphQLResponse<TResult>>
91+
}
92+
behaviour: RequestBehaviour
93+
queryAsString: string
94+
variables?: Variables
95+
autoRateLimitRestore: boolean
8096
}
8197

8298
export interface GraphQLResponseOptions<T> {
8399
handleErrors?: boolean
84100
onResponse?: (response: GraphQLResponse<T>) => void
85101
}
86102

103+
const MAX_RATE_LIMIT_RESTORE_DELAY_SECONDS = 0.3
104+
87105
async function createGraphQLClient({
88106
url,
89107
addedHeaders,
@@ -105,38 +123,77 @@ async function createGraphQLClient({
105123
}
106124
}
107125

108-
/**
109-
* Handles execution of a GraphQL query.
110-
*
111-
* @param options - GraphQL request options.
112-
*/
126+
async function waitForRateLimitRestore(fullResponse: GraphQLResponse<unknown>) {
127+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
128+
const cost = (fullResponse.extensions as any)?.cost
129+
const actualQueryCost = cost?.actualQueryCost
130+
const restoreRate = cost?.throttleStatus?.restoreRate
131+
if (actualQueryCost && typeof actualQueryCost === 'number' && restoreRate && typeof restoreRate === 'number') {
132+
const secondsToRestoreRate = actualQueryCost / restoreRate
133+
outputDebug(outputContent`Sleeping for ${secondsToRestoreRate.toString()} seconds to restore the rate limit.`)
134+
await sleep(Math.min(secondsToRestoreRate, MAX_RATE_LIMIT_RESTORE_DELAY_SECONDS))
135+
}
136+
}
137+
138+
async function runSingleRawGraphQLRequest<TResult>(
139+
options: RunRawGraphQLRequestOptions<TResult>,
140+
): Promise<GraphQLResponse<TResult>> {
141+
const {client, behaviour, queryAsString, variables, autoRateLimitRestore} = options
142+
let fullResponse: GraphQLResponse<TResult>
143+
// there is a errorPolicy option which returns rather than throwing on errors, but we _do_ ultimately want to
144+
// throw.
145+
try {
146+
client.setAbortSignal(abortSignalFromRequestBehaviour(behaviour))
147+
fullResponse = await client.rawRequest(queryAsString, variables)
148+
await logLastRequestIdFromResponse(fullResponse)
149+
150+
if (autoRateLimitRestore) {
151+
await waitForRateLimitRestore(fullResponse)
152+
}
153+
154+
return fullResponse
155+
} catch (error) {
156+
if (error instanceof ClientError) {
157+
// error.response does have a headers property like a normal response, but it's not typed as such.
158+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
159+
await logLastRequestIdFromResponse(error.response as any)
160+
}
161+
throw error
162+
}
163+
}
164+
113165
async function performGraphQLRequest<TResult>(options: PerformGraphQLRequestOptions<TResult>) {
114-
const {token, addedHeaders, queryAsString, variables, api, url, responseOptions, unauthorizedHandler, cacheOptions} =
115-
options
166+
const {
167+
token,
168+
addedHeaders,
169+
queryAsString,
170+
variables,
171+
api,
172+
url,
173+
responseOptions,
174+
unauthorizedHandler,
175+
cacheOptions,
176+
autoRateLimitRestore,
177+
} = options
116178
const behaviour = requestMode(options.preferredBehaviour ?? 'default')
117179

118180
let {headers, client} = await createGraphQLClient({url, addedHeaders, token})
119181
debugLogRequestInfo(api, queryAsString, url, variables, headers)
120182

121183
const rawGraphQLRequest = async () => {
122-
let fullResponse: GraphQLResponse<TResult>
123-
// there is a errorPolicy option which returns rather than throwing on errors, but we _do_ ultimately want to
124-
// throw.
125-
try {
126-
// mapping signal to any due to polyfill meaning types don't exactly match (but are functionally equivalent)
127-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
128-
client.requestConfig.signal = abortSignalFromRequestBehaviour(behaviour) as any
129-
fullResponse = await client.rawRequest<TResult>(queryAsString, variables)
130-
await logLastRequestIdFromResponse(fullResponse)
131-
return fullResponse
132-
} catch (error) {
133-
if (error instanceof ClientError) {
134-
// error.response does have a headers property like a normal response, but it's not typed as such.
184+
return runSingleRawGraphQLRequest({
185+
client: {
135186
// eslint-disable-next-line @typescript-eslint/no-explicit-any
136-
await logLastRequestIdFromResponse(error.response as any)
137-
}
138-
throw error
139-
}
187+
setAbortSignal: (signal: any) => {
188+
client.requestConfig.signal = signal
189+
},
190+
rawRequest: (query: string, variables?: Variables) => client.rawRequest<TResult>(query, variables),
191+
},
192+
behaviour,
193+
queryAsString,
194+
variables,
195+
autoRateLimitRestore: autoRateLimitRestore ?? false,
196+
})
140197
}
141198

142199
const tokenRefreshHandler = unauthorizedHandler?.handler

packages/cli-kit/src/public/node/http.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type AutomaticCancellationBehaviour =
3838
useAbortSignal: AbortSignal | (() => AbortSignal)
3939
}
4040

41-
type RequestBehaviour = NetworkRetryBehaviour & AutomaticCancellationBehaviour
41+
export type RequestBehaviour = NetworkRetryBehaviour & AutomaticCancellationBehaviour
4242

4343
export type RequestModeInput = PresetFetchBehaviour | RequestBehaviour
4444

0 commit comments

Comments
 (0)