Skip to content

Conversation

@shauns
Copy link
Contributor

@shauns shauns commented Feb 24, 2025

WHY are these changes introduced?

Improves network request handling in the CLI by introducing configurable request behaviours, timeouts, and retries. This enhances reliability and responsiveness of network operations across the codebase.

These are available for fetch & shopifyFetch, and used by the GraphQL stack.

These changes are backward-compatible for public interfaces in CLI kit, and test coverage for network APIs is greatly increased.

WHAT is this pull request doing?

  • Introduces requestMode to configure network request behaviors with presets:
    • default: Automatic retries and timeouts
    • non-blocking: No retries, with timeouts
    • slow-request: No retries or timeouts -- i.e. neutral
  • Adds configurable abort signals and timeout controls for requests
  • Enhances request retry logic with better error handling
  • Updates network calls across the codebase to use appropriate request modes
  • Adds comprehensive test coverage using MSW for network mocking

How to test your changes?

See the downstack PR for a test script to follow.

Measuring impact

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@shauns shauns mentioned this pull request Feb 24, 2025
3 tasks
Copy link
Contributor Author

shauns commented Feb 24, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shauns shauns marked this pull request as ready for review February 24, 2025 14:18
@shauns shauns requested a review from a team as a code owner February 24, 2025 14:18
@github-actions

This comment has been minimized.

@shauns shauns force-pushed the shauns/02-24-controllable_request_behaviour branch 2 times, most recently from 3114e10 to 033d7ff Compare February 24, 2025 14:31
@shauns shauns requested a review from a team as a code owner February 24, 2025 14:31
@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
76.45% (+0.38% 🔼)
9295/12158
🟡 Branches
71.64% (+0.41% 🔼)
4556/6360
🟡 Functions
76.05% (+0.44% 🔼)
2419/3181
🟡 Lines
77.04% (+0.39% 🔼)
8785/11403

Test suite run success

2109 tests passing in 930 suites.

Report generated by 🧪jest coverage report action from 36dccd6

@shauns shauns force-pushed the shauns/02-24-controllable_request_behaviour branch 3 times, most recently from 97e7c14 to b6e0cbd Compare February 25, 2025 15:09

const runningOnWindows = platformAndArch().platform === 'windows'

test.skipIf(runningOnWindows)('Cleans up if download fails', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not get this to pass. Something, somewhere, is doing an lstat on a file it's not allowed to and this is leaking out. But fileExists and the unlink inside the download function both wrap their FS usage in a catch-all.

I would like to resolve this -- if only to be sure its nothing in the CLI's own code -- but for the moment, I've left it skipped to get a green CI run.

@gonzaloriestra
Copy link
Contributor

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g @shopify/cli@0.0.0-snapshot-20250226103056

Tip

If you get an ETARGET error, install it with NPM and the flag --@shopify:registry=https://registry.npmjs.org

Caution

After installing, validate the version by running just shopify in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@shauns shauns force-pushed the shauns/02-18-network_error_request_retries branch from 4306e67 to d4cd281 Compare March 3, 2025 11:17
@shauns shauns force-pushed the shauns/02-24-controllable_request_behaviour branch from b6e0cbd to 976b5d9 Compare March 3, 2025 11:17
@shauns shauns requested a review from isaacroldan March 3, 2025 11:23
@shauns shauns force-pushed the shauns/02-18-network_error_request_retries branch from d4cd281 to 73c78ac Compare March 3, 2025 13:09
@shauns shauns force-pushed the shauns/02-24-controllable_request_behaviour branch from 976b5d9 to 36dccd6 Compare March 3, 2025 13:09
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2025

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 main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/private/node/sleep-with-backoff.d.ts
export declare const DEFAULT_MAX_TIME_MS = 10000;
interface BackoffResult {
    remainingMs: number;
    iterations: number;
}
/**
 * Generator that sleeps with exponential backoff between yields, stopping before exceeding a time limit
 *
 * Yields the amount of time slept in milliseconds.
 *
 * @param maxTimeMs - Maximum total time in milliseconds before stopping
 * @param firstDelayMs - First delay in milliseconds
 * @returns Information about the backoff sequence: remaining time and iteration count
 */
export declare function sleepWithBackoffUntil(maxTimeMs?: number, firstDelayMs?: number): AsyncGenerator<number, BackoffResult, unknown>;
export {};

Existing type declarations

packages/cli-kit/dist/private/node/api.d.ts
@@ -1,18 +1,41 @@
 import { Headers } from 'form-data';
 export type API = 'admin' | 'storefront-renderer' | 'partners' | 'business-platform' | 'app-management';
 export declare const allAPIs: API[];
-interface RequestOptions<T> {
+export type NetworkRetryBehaviour = {
+    useNetworkLevelRetry: true;
+    maxRetryTimeMs: number;
+} | {
+    useNetworkLevelRetry: false;
+};
+type RequestOptions<T> = {
     request: () => Promise<T>;
     url: string;
-}
+} & NetworkRetryBehaviour;
 export declare function simpleRequestWithDebugLog<T extends {
     headers: Headers;
     status: number;
-}>({ request, url }: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown): Promise<T>;
+}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown): Promise<T>;
+/**
+ * Makes a HTTP request to some API, retrying if response headers indicate a retryable error.
+ *
+ * If a request fails with a 429, the retry-after header determines a delay before an automatic retry is performed.
+ *
+ * If unauthorizedHandler is provided, then it will be called in the case of a 401 and a retry performed. This allows
+ * for a token refresh for instance.
+ *
+ * If there's a network error, e.g. DNS fails to resolve, then API calls are automatically retried.
+ *
+ * @param request - A function that returns a promise of the response
+ * @param url - The URL to request
+ * @param errorHandler - A function that handles errors
+ * @param unauthorizedHandler - A function that handles unauthorized errors
+ * @param retryOptions - Options for the retry
+ * @returns The response from the request
+ */
 export declare function retryAwareRequest<T extends {
     headers: Headers;
     status: number;
-}>({ request, url }: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => Promise<void>, retryOptions?: {
+}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => Promise<void>, retryOptions?: {
     limitRetriesTo?: number;
     defaultDelayMs?: number;
     scheduleDelay: (fn: () => void, delay: number) => void;
packages/cli-kit/dist/private/node/constants.d.ts
@@ -32,6 +32,8 @@ export declare const environmentVariables: {
     themeKitAccessDomain: string;
     json: string;
     neverUsePartnersApi: string;
+    skipNetworkLevelRetry: string;
+    maxRequestTimeForNetworkCalls: string;
 };
 export declare const defaultThemeKitAccessDomain = "theme-kit-access.shopifyapps.com";
 export declare const systemEnvironmentVariables: {
packages/cli-kit/dist/public/node/environment.d.ts
@@ -55,4 +55,24 @@ export declare function jsonOutputEnabled(environment?: NodeJS.ProcessEnv): bool
  *
  * @returns True if the SHOPIFY_CLI_NEVER_USE_PARTNERS_API environment variable is set.
  */
-export declare function blockPartnersAccess(): boolean;
\ No newline at end of file
+export declare function blockPartnersAccess(): boolean;
+/**
+ * If true, the CLI should not use the network level retry.
+ *
+ * If there is an error when calling a network API that looks like a DNS or connectivity issue, the CLI will by default
+ * automatically retry the request.
+ *
+ * @param environment - Process environment variables.
+ * @returns True if the SHOPIFY_CLI_SKIP_NETWORK_LEVEL_RETRY environment variable is set.
+ */
+export declare function skipNetworkLevelRetry(environment?: NodeJS.ProcessEnv): boolean;
+/**
+ * Returns the default maximum request time for network calls in milliseconds.
+ *
+ * After this long, API requests may be cancelled by an AbortSignal. The limit can be overridden by setting the
+ * SHOPIFY_CLI_MAX_REQUEST_TIME_FOR_NETWORK_CALLS environment variable.
+ *
+ * @param environment - Process environment variables.
+ * @returns The maximum request time in milliseconds.
+ */
+export declare function maxRequestTimeForNetworkCallsMs(environment?: NodeJS.ProcessEnv): number;
\ No newline at end of file
packages/cli-kit/dist/public/node/http.d.ts
@@ -1,3 +1,4 @@
+import { NetworkRetryBehaviour } from '../../private/node/api.js';
 import FormData from 'form-data';
 import { RequestInfo, RequestInit, Response } from 'node-fetch';
 export { FetchError, Request, Response } from 'node-fetch';
@@ -7,6 +8,43 @@ export { FetchError, Request, Response } from 'node-fetch';
  * @returns A FormData object.
  */
 export declare function formData(): FormData;
+type AbortSignal = RequestInit['signal'];
+type PresetFetchBehaviour = 'default' | 'non-blocking' | 'slow-request';
+type AutomaticCancellationBehaviour = {
+    useAbortSignal: true;
+    timeoutMs: number;
+} | {
+    useAbortSignal: false;
+} | {
+    useAbortSignal: AbortSignal | (() => AbortSignal);
+};
+type RequestBehaviour = NetworkRetryBehaviour & AutomaticCancellationBehaviour;
+type RequestModeInput = PresetFetchBehaviour | RequestBehaviour;
+/**
+ * Specify the behaviour of a network request.
+ *
+ * - default: Requests are automatically retried, and are subject to automatic cancellation if they're taking too long.
+ * This is generally desirable.
+ * - non-blocking: Requests are not retried if they fail with a network error, and are automatically cancelled if
+ * they're taking too long. This is good for throwaway requests, like polling or tracking.
+ * - slow-request: Requests are not retried if they fail with a network error, and are not automatically cancelled.
+ * This is good for slow requests that should be give the chance to complete, and are unlikely to be safe to retry.
+ *
+ * Some request behaviours may be de-activated by the environment, and this function takes care of that concern. You
+ * can also provide a customised request behaviour.
+ *
+ * @param preset - The preset to use.
+ * @param env - Process environment variables.
+ * @returns A request behaviour object.
+ */
+export declare function requestMode(preset?: RequestModeInput, env?: NodeJS.ProcessEnv): RequestBehaviour;
+/**
+ * Create an AbortSignal for automatic request cancellation, from a request behaviour.
+ *
+ * @param behaviour - The request behaviour.
+ * @returns An AbortSignal.
+ */
+export declare function abortSignalFromRequestBehaviour(behaviour: RequestBehaviour): AbortSignal;
 /**
  * An interface that abstracts way node-fetch. When Node has built-in
  * support for "fetch" in the standard library, we can drop the node-fetch
@@ -15,21 +53,28 @@ export declare function formData(): FormData;
  * they are consistent with the Web API so if we drop node-fetch in the future
  * it won't require changes from the callers.
  *
+ * The CLI's fetch function supports special behaviours, like automatic retries. These are disabled by default through
+ * this function.
+ *
  * @param url - This defines the resource that you wish to fetch.
  * @param init - An object containing any custom settings that you want to apply to the request.
+ * @param preferredBehaviour - A request behaviour object that overrides the default behaviour.
  * @returns A promise that resolves with the response.
  */
-export declare function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
+export declare function fetch(url: RequestInfo, init?: RequestInit, preferredBehaviour?: RequestModeInput): Promise<Response>;
 /**
  * A fetch function to use with Shopify services. The function ensures the right
  * TLS configuragion is used based on the environment in which the service is running
- * (e.g. Spin).
+ * (e.g. Spin). NB: headers/auth are the responsibility of the caller.
+ *
+ * By default, the CLI's fetch function's special behaviours, like automatic retries, are enabled.
  *
  * @param url - This defines the resource that you wish to fetch.
  * @param init - An object containing any custom settings that you want to apply to the request.
+ * @param preferredBehaviour - A request behaviour object that overrides the default behaviour.
  * @returns A promise that resolves with the response.
  */
-export declare function shopifyFetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
+export declare function shopifyFetch(url: RequestInfo, init?: RequestInit, preferredBehaviour?: RequestModeInput): Promise<Response>;
 /**
  * Download a file from a URL to a local path.
  *
packages/cli-kit/dist/private/node/session/schema.d.ts
@@ -9,15 +9,15 @@ declare const IdentityTokenSchema: zod.ZodObject<{
     scopes: zod.ZodArray<zod.ZodString, "many">;
     userId: zod.ZodString;
 }, "strip", zod.ZodTypeAny, {
-    scopes: string[];
     accessToken: string;
     refreshToken: string;
+    scopes: string[];
     expiresAt: Date;
     userId: string;
 }, {
-    scopes: string[];
     accessToken: string;
     refreshToken: string;
+    scopes: string[];
     userId: string;
     expiresAt?: unknown;
 }>;
@@ -29,12 +29,12 @@ declare const ApplicationTokenSchema: zod.ZodObject<{
     expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
     scopes: zod.ZodArray<zod.ZodString, "many">;
 }, "strip", zod.ZodTypeAny, {
-    scopes: string[];
     accessToken: string;
+    scopes: string[];
     expiresAt: Date;
 }, {
-    scopes: string[];
     accessToken: string;
+    scopes: string[];
     expiresAt?: unknown;
 }>;
 /**
@@ -58,15 +58,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -79,65 +79,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };
@@ -154,15 +154,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -175,65 +175,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };
@@ -250,15 +250,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -271,65 +271,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2025

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 main you might see odd diffs, rebase main into this branch.

New type declarations

packages/cli-kit/dist/private/node/sleep-with-backoff.d.ts
export declare const DEFAULT_MAX_TIME_MS = 10000;
interface BackoffResult {
    remainingMs: number;
    iterations: number;
}
/**
 * Generator that sleeps with exponential backoff between yields, stopping before exceeding a time limit
 *
 * Yields the amount of time slept in milliseconds.
 *
 * @param maxTimeMs - Maximum total time in milliseconds before stopping
 * @param firstDelayMs - First delay in milliseconds
 * @returns Information about the backoff sequence: remaining time and iteration count
 */
export declare function sleepWithBackoffUntil(maxTimeMs?: number, firstDelayMs?: number): AsyncGenerator<number, BackoffResult, unknown>;
export {};

Existing type declarations

packages/cli-kit/dist/private/node/api.d.ts
@@ -1,18 +1,41 @@
 import { Headers } from 'form-data';
 export type API = 'admin' | 'storefront-renderer' | 'partners' | 'business-platform' | 'app-management';
 export declare const allAPIs: API[];
-interface RequestOptions<T> {
+export type NetworkRetryBehaviour = {
+    useNetworkLevelRetry: true;
+    maxRetryTimeMs: number;
+} | {
+    useNetworkLevelRetry: false;
+};
+type RequestOptions<T> = {
     request: () => Promise<T>;
     url: string;
-}
+} & NetworkRetryBehaviour;
 export declare function simpleRequestWithDebugLog<T extends {
     headers: Headers;
     status: number;
-}>({ request, url }: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown): Promise<T>;
+}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown): Promise<T>;
+/**
+ * Makes a HTTP request to some API, retrying if response headers indicate a retryable error.
+ *
+ * If a request fails with a 429, the retry-after header determines a delay before an automatic retry is performed.
+ *
+ * If unauthorizedHandler is provided, then it will be called in the case of a 401 and a retry performed. This allows
+ * for a token refresh for instance.
+ *
+ * If there's a network error, e.g. DNS fails to resolve, then API calls are automatically retried.
+ *
+ * @param request - A function that returns a promise of the response
+ * @param url - The URL to request
+ * @param errorHandler - A function that handles errors
+ * @param unauthorizedHandler - A function that handles unauthorized errors
+ * @param retryOptions - Options for the retry
+ * @returns The response from the request
+ */
 export declare function retryAwareRequest<T extends {
     headers: Headers;
     status: number;
-}>({ request, url }: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => Promise<void>, retryOptions?: {
+}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => Promise<void>, retryOptions?: {
     limitRetriesTo?: number;
     defaultDelayMs?: number;
     scheduleDelay: (fn: () => void, delay: number) => void;
packages/cli-kit/dist/private/node/constants.d.ts
@@ -32,6 +32,8 @@ export declare const environmentVariables: {
     themeKitAccessDomain: string;
     json: string;
     neverUsePartnersApi: string;
+    skipNetworkLevelRetry: string;
+    maxRequestTimeForNetworkCalls: string;
 };
 export declare const defaultThemeKitAccessDomain = "theme-kit-access.shopifyapps.com";
 export declare const systemEnvironmentVariables: {
packages/cli-kit/dist/public/node/environment.d.ts
@@ -55,4 +55,24 @@ export declare function jsonOutputEnabled(environment?: NodeJS.ProcessEnv): bool
  *
  * @returns True if the SHOPIFY_CLI_NEVER_USE_PARTNERS_API environment variable is set.
  */
-export declare function blockPartnersAccess(): boolean;
\ No newline at end of file
+export declare function blockPartnersAccess(): boolean;
+/**
+ * If true, the CLI should not use the network level retry.
+ *
+ * If there is an error when calling a network API that looks like a DNS or connectivity issue, the CLI will by default
+ * automatically retry the request.
+ *
+ * @param environment - Process environment variables.
+ * @returns True if the SHOPIFY_CLI_SKIP_NETWORK_LEVEL_RETRY environment variable is set.
+ */
+export declare function skipNetworkLevelRetry(environment?: NodeJS.ProcessEnv): boolean;
+/**
+ * Returns the default maximum request time for network calls in milliseconds.
+ *
+ * After this long, API requests may be cancelled by an AbortSignal. The limit can be overridden by setting the
+ * SHOPIFY_CLI_MAX_REQUEST_TIME_FOR_NETWORK_CALLS environment variable.
+ *
+ * @param environment - Process environment variables.
+ * @returns The maximum request time in milliseconds.
+ */
+export declare function maxRequestTimeForNetworkCallsMs(environment?: NodeJS.ProcessEnv): number;
\ No newline at end of file
packages/cli-kit/dist/public/node/http.d.ts
@@ -1,3 +1,4 @@
+import { NetworkRetryBehaviour } from '../../private/node/api.js';
 import FormData from 'form-data';
 import { RequestInfo, RequestInit, Response } from 'node-fetch';
 export { FetchError, Request, Response } from 'node-fetch';
@@ -7,6 +8,43 @@ export { FetchError, Request, Response } from 'node-fetch';
  * @returns A FormData object.
  */
 export declare function formData(): FormData;
+type AbortSignal = RequestInit['signal'];
+type PresetFetchBehaviour = 'default' | 'non-blocking' | 'slow-request';
+type AutomaticCancellationBehaviour = {
+    useAbortSignal: true;
+    timeoutMs: number;
+} | {
+    useAbortSignal: false;
+} | {
+    useAbortSignal: AbortSignal | (() => AbortSignal);
+};
+type RequestBehaviour = NetworkRetryBehaviour & AutomaticCancellationBehaviour;
+type RequestModeInput = PresetFetchBehaviour | RequestBehaviour;
+/**
+ * Specify the behaviour of a network request.
+ *
+ * - default: Requests are automatically retried, and are subject to automatic cancellation if they're taking too long.
+ * This is generally desirable.
+ * - non-blocking: Requests are not retried if they fail with a network error, and are automatically cancelled if
+ * they're taking too long. This is good for throwaway requests, like polling or tracking.
+ * - slow-request: Requests are not retried if they fail with a network error, and are not automatically cancelled.
+ * This is good for slow requests that should be give the chance to complete, and are unlikely to be safe to retry.
+ *
+ * Some request behaviours may be de-activated by the environment, and this function takes care of that concern. You
+ * can also provide a customised request behaviour.
+ *
+ * @param preset - The preset to use.
+ * @param env - Process environment variables.
+ * @returns A request behaviour object.
+ */
+export declare function requestMode(preset?: RequestModeInput, env?: NodeJS.ProcessEnv): RequestBehaviour;
+/**
+ * Create an AbortSignal for automatic request cancellation, from a request behaviour.
+ *
+ * @param behaviour - The request behaviour.
+ * @returns An AbortSignal.
+ */
+export declare function abortSignalFromRequestBehaviour(behaviour: RequestBehaviour): AbortSignal;
 /**
  * An interface that abstracts way node-fetch. When Node has built-in
  * support for "fetch" in the standard library, we can drop the node-fetch
@@ -15,21 +53,28 @@ export declare function formData(): FormData;
  * they are consistent with the Web API so if we drop node-fetch in the future
  * it won't require changes from the callers.
  *
+ * The CLI's fetch function supports special behaviours, like automatic retries. These are disabled by default through
+ * this function.
+ *
  * @param url - This defines the resource that you wish to fetch.
  * @param init - An object containing any custom settings that you want to apply to the request.
+ * @param preferredBehaviour - A request behaviour object that overrides the default behaviour.
  * @returns A promise that resolves with the response.
  */
-export declare function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
+export declare function fetch(url: RequestInfo, init?: RequestInit, preferredBehaviour?: RequestModeInput): Promise<Response>;
 /**
  * A fetch function to use with Shopify services. The function ensures the right
  * TLS configuragion is used based on the environment in which the service is running
- * (e.g. Spin).
+ * (e.g. Spin). NB: headers/auth are the responsibility of the caller.
+ *
+ * By default, the CLI's fetch function's special behaviours, like automatic retries, are enabled.
  *
  * @param url - This defines the resource that you wish to fetch.
  * @param init - An object containing any custom settings that you want to apply to the request.
+ * @param preferredBehaviour - A request behaviour object that overrides the default behaviour.
  * @returns A promise that resolves with the response.
  */
-export declare function shopifyFetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
+export declare function shopifyFetch(url: RequestInfo, init?: RequestInit, preferredBehaviour?: RequestModeInput): Promise<Response>;
 /**
  * Download a file from a URL to a local path.
  *
packages/cli-kit/dist/private/node/session/schema.d.ts
@@ -9,15 +9,15 @@ declare const IdentityTokenSchema: zod.ZodObject<{
     scopes: zod.ZodArray<zod.ZodString, "many">;
     userId: zod.ZodString;
 }, "strip", zod.ZodTypeAny, {
-    scopes: string[];
     accessToken: string;
     refreshToken: string;
+    scopes: string[];
     expiresAt: Date;
     userId: string;
 }, {
-    scopes: string[];
     accessToken: string;
     refreshToken: string;
+    scopes: string[];
     userId: string;
     expiresAt?: unknown;
 }>;
@@ -29,12 +29,12 @@ declare const ApplicationTokenSchema: zod.ZodObject<{
     expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
     scopes: zod.ZodArray<zod.ZodString, "many">;
 }, "strip", zod.ZodTypeAny, {
-    scopes: string[];
     accessToken: string;
+    scopes: string[];
     expiresAt: Date;
 }, {
-    scopes: string[];
     accessToken: string;
+    scopes: string[];
     expiresAt?: unknown;
 }>;
 /**
@@ -58,15 +58,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -79,65 +79,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };
@@ -154,15 +154,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -175,65 +175,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };
@@ -250,15 +250,15 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         scopes: zod.ZodArray<zod.ZodString, "many">;
         userId: zod.ZodString;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     }, {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     }>;
@@ -271,65 +271,65 @@ export declare const SessionSchema: zod.ZodObject<{}, "strip", zod.ZodObject<{
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, zod.objectOutputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">, zod.objectInputType<{}, zod.ZodObject<{
         accessToken: zod.ZodString;
         expiresAt: zod.ZodEffects<zod.ZodDate, Date, unknown>;
         scopes: zod.ZodArray<zod.ZodString, "many">;
     }, "strip", zod.ZodTypeAny, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt: Date;
     }, {
-        scopes: string[];
         accessToken: string;
+        scopes: string[];
         expiresAt?: unknown;
     }>, "strip">>;
 }, "strip", zod.ZodTypeAny, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         expiresAt: Date;
         userId: string;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt: Date;
         };
     };
 }, {
     identity: {
-        scopes: string[];
         accessToken: string;
         refreshToken: string;
+        scopes: string[];
         userId: string;
         expiresAt?: unknown;
     };
     applications: {} & {
         [k: string]: {
-            scopes: string[];
             accessToken: string;
+            scopes: string[];
             expiresAt?: unknown;
         };
     };

Base automatically changed from shauns/02-18-network_error_request_retries to main March 4, 2025 15:48
@shauns shauns added this pull request to the merge queue Mar 4, 2025
Merged via the queue into main with commit 620dc4d Mar 4, 2025
1 check passed
@shauns shauns deleted the shauns/02-24-controllable_request_behaviour branch March 4, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants