Custom network settings for theme commands
Fixes https://github.com/Shopify/cli/issues/5617
WHAT is this pull request doing?
With the implementation of some new request behaviour, some theme commands were failing if they took longer than 15 seconds (the new default timeout). This can affect devs with larger themes and/or slower internet.
Enhancements to API Request Behavior:
- Added
RequestModeInputandRequestBehaviourtypes topackages/cli-kit/src/public/node/http.tsto define customizable network request behaviors. - Updated
adminRequestDocandperformGraphQLRequestfunctions to accept an optionalrequestBehaviourparameter for configuring request behavior. [1] [2] - Introduced a constant
THEME_API_NETWORK_BEHAVIOURinpackages/cli-kit/src/public/node/themes/api.tsto specify default network behavior for theme-related API calls.
These changes improve the robustness and configurability of API requests, particularly for theme-related operations.
How to test your changes?
Option 1:
- Pull down the branch
- Build the branch
- Download software that can manipulate your internet speed and slow it down to the point a theme command like
theme pushtakes over 15 seconds to complete. You shouldn't see a timeout error until 45 seconds (original 15 seconds)
Option 2:
- Pull down the branch
- in
/src/public/node/themes/api.ts, in the constantTHEME_API_NETWORK_BEHAVIOUR, changetimeoutMsandmaxRetryTimeMsto 1000 (1 second) - Build the branch
- Run
theme pushand it should fail almost instantly. Now we know the changes are affecting the right area.
Measuring impact
How do we know this change was effective? Please choose one:
- [X] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Checklist
- [X] I've considered possible cross-platform impacts (Mac, Linux, Windows)
Coverage report
St.:grey_question: |
Category | Percentage | Covered / Total |
|---|---|---|---|
| π‘ | Statements | 76.63% (+0.01% πΌ) |
9586/12509 |
| π‘ | Branches | 71.88% (+0.02% πΌ) |
4723/6571 |
| π‘ | Functions | 76.56% | 2485/3246 |
| π‘ | Lines | 77.14% (+0.01% πΌ) |
9059/11743 |
Test suite run success
2251 tests passing in 981 suites.
Report generated by π§ͺjest coverage report action from 842200406cd8338d601201f6796623d3ac2a956c
We detected some changes at packages/*/src and there are no updates in the .changeset. If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.
Can we loop in @shauns to this for visibility + an extra set of eyes?
Thanks for chiming in @shauns & @isaacroldan! I'll take a look at the other PR, do some edits and circle back.
Differences in type declarations
We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
- Some seemingly private modules might be re-exported through public modules.
- If the branch is behind
mainyou might see odd diffs, rebasemaininto this branch.
New type declarations
We found no new type declarations in this PR
Existing type declarations
packages/cli-kit/dist/public/node/api/admin.d.ts
@@ -1,5 +1,6 @@
import { GraphQLResponseOptions, GraphQLVariables } from './graphql.js';
import { AdminSession } from '../session.js';
+import { RequestModeInput } from '../http.js';
import { Variables } from 'graphql-request';
import { TypedDocumentNode } from '@graphql-typed-document-node/core';
/**
@@ -11,17 +12,27 @@ import { TypedDocumentNode } from '@graphql-typed-document-node/core';
* @returns The response of the query of generic type <T>.
*/
export declare function adminRequest<T>(query: string, session: AdminSession, variables?: GraphQLVariables): Promise<T>;
+export interface AdminRequestOptions<TResult, TVariables extends Variables> {
+ /** GraphQL query to execute. */
+ query: TypedDocumentNode<TResult, TVariables>;
+ /** Shopify admin session including token and Store FQDN. */
+ session: AdminSession;
+ /** GraphQL variables to pass to the query. */
+ variables?: TVariables;
+ /** API version. */
+ version?: string;
+ /** Control how API responses will be handled. */
+ responseOptions?: GraphQLResponseOptions<TResult>;
+ /** Custom request behaviour for retries and timeouts. */
+ requestBehaviour?: RequestModeInput;
+}
/**
* Executes a GraphQL query against the Admin API. Uses typed documents.
*
- * @param query - GraphQL query to execute.
- * @param session - Shopify admin session including token and Store FQDN.
- * @param variables - GraphQL variables to pass to the query.
- * @param version - API version.
- * @param responseOptions - Control how API responses will be handled.
+ * @param options - Admin request options.
* @returns The response of the query of generic type <TResult>.
*/
-export declare function adminRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables>, session: AdminSession, variables?: TVariables, version?: string, responseOptions?: GraphQLResponseOptions<TResult>): Promise<TResult>;
+export declare function adminRequestDoc<TResult, TVariables extends Variables>(options: AdminRequestOptions<TResult, TVariables>): Promise<TResult>;
/**
* GraphQL query to retrieve all supported API versions.
*
packages/cli-kit/dist/public/node/api/graphql.d.ts
@@ -32,6 +32,7 @@ export type GraphQLRequestOptions<T> = GraphQLRequestBaseOptions<T> & {
query: RequestDocument;
variables?: Variables;
unauthorizedHandler?: () => Promise<void>;
+ requestBehaviour?: RequestModeInput;
};
export type GraphQLRequestDocOptions<TResult, TVariables> = GraphQLRequestBaseOptions<TResult> & {
query: TypedDocumentNode<TResult, TVariables> | TypedDocumentNode<TResult, Exact<{
@@ -39,6 +40,7 @@ export type GraphQLRequestDocOptions<TResult, TVariables> = GraphQLRequestBaseOp
}>>;
variables?: TVariables;
unauthorizedHandler?: () => Promise<void>;
+ requestBehaviour?: RequestModeInput;
};
export interface GraphQLResponseOptions<T> {
handleErrors?: boolean;
@shauns or @isaacroldan, could you please take a look at this when you have a moment?