-
Notifications
You must be signed in to change notification settings - Fork 162
Automatically retry partners requests that fail with 401 #5646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Automatically retry partners requests that fail with 401 #5646
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success2199 tests passing in 959 suites. Report generated by 🧪jest coverage report action from 0b76517 |
packages/app/src/cli/utilities/developer-platform-client/partners-client.ts
Outdated
Show resolved
Hide resolved
cf1a625
to
441eee5
Compare
We detected some changes at packages/*/src and there are no updates in the .changeset. |
441eee5
to
0b76517
Compare
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/private/node/api.d.ts@@ -11,6 +11,13 @@ type RequestOptions<T> = {
request: () => Promise<T>;
url: string;
} & NetworkRetryBehaviour;
+export interface UnauthorizedHandlerThrow {
+ action: 'throw';
+}
+type UnauthorizedHandlerContinue = {
+ action: 'continue';
+} | undefined;
+export type UnauthorizedHandlerResult = Promise<UnauthorizedHandlerThrow | UnauthorizedHandlerContinue>;
export declare function simpleRequestWithDebugLog<T extends {
headers: Headers;
status: number;
@@ -35,7 +42,7 @@ export declare function simpleRequestWithDebugLog<T extends {
export declare function retryAwareRequest<T extends {
headers: Headers;
status: number;
-}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => Promise<void>, retryOptions?: {
+}>(requestOptions: RequestOptions<T>, errorHandler?: (error: unknown, requestId: string | undefined) => unknown, unauthorizedHandler?: () => UnauthorizedHandlerResult, retryOptions?: {
limitRetriesTo?: number;
defaultDelayMs?: number;
scheduleDelay: (fn: () => void, delay: number) => void;
packages/cli-kit/dist/public/node/api/graphql.d.ts@@ -1,3 +1,4 @@
+import { UnauthorizedHandlerResult, UnauthorizedHandlerThrow } from '../../../private/node/api.js';
import { ConfSchema, TimeInterval } from '../../../private/node/conf-store.js';
import { LocalStorage } from '../local-storage.js';
import { rawRequest, RequestDocument, Variables } from 'graphql-request';
@@ -16,6 +17,11 @@ export interface CacheOptions {
cacheExtraKey?: string;
cacheStore?: LocalStorage<ConfSchema>;
}
+interface RefreshTokenOnAuthorizedResponseRetryWithNewToken {
+ action: 'retry';
+ token: string;
+}
+export type RefreshTokenOnAuthorizedResponse = Promise<UnauthorizedHandlerThrow | RefreshTokenOnAuthorizedResponseRetryWithNewToken>;
interface GraphQLRequestBaseOptions<TResult> {
api: string;
url: string;
@@ -25,18 +31,19 @@ interface GraphQLRequestBaseOptions<TResult> {
};
responseOptions?: GraphQLResponseOptions<TResult>;
cacheOptions?: CacheOptions;
+ refreshTokenOnAuthorizedResponse?: () => RefreshTokenOnAuthorizedResponse;
}
export type GraphQLRequestOptions<T> = GraphQLRequestBaseOptions<T> & {
query: RequestDocument;
variables?: Variables;
- unauthorizedHandler?: () => Promise<void>;
+ unauthorizedHandler?: () => UnauthorizedHandlerResult;
};
export type GraphQLRequestDocOptions<TResult, TVariables> = GraphQLRequestBaseOptions<TResult> & {
query: TypedDocumentNode<TResult, TVariables> | TypedDocumentNode<TResult, Exact<{
[key: string]: never;
}>>;
variables?: TVariables;
- unauthorizedHandler?: () => Promise<void>;
+ unauthorizedHandler?: () => UnauthorizedHandlerResult;
};
export interface GraphQLResponseOptions<T> {
handleErrors?: boolean;
packages/cli-kit/dist/public/node/api/partners.d.ts@@ -1,4 +1,4 @@
-import { GraphQLVariables, GraphQLResponse, CacheOptions } from './graphql.js';
+import { GraphQLVariables, GraphQLResponse, CacheOptions, RefreshTokenOnAuthorizedResponse } from './graphql.js';
import { Variables } from 'graphql-request';
import { TypedDocumentNode } from '@graphql-typed-document-node/core';
/**
@@ -8,9 +8,10 @@ import { TypedDocumentNode } from '@graphql-typed-document-node/core';
* @param token - Partners token.
* @param variables - GraphQL variables to pass to the query.
* @param cacheOptions - Cache options.
+ * @param refreshTokenOnAuthorizedResponse - Optional handler for unauthorized requests.
* @returns The response of the query of generic type <T>.
*/
-export declare function partnersRequest<T>(query: string, token: string, variables?: GraphQLVariables, cacheOptions?: CacheOptions): Promise<T>;
+export declare function partnersRequest<T>(query: string, token: string, variables?: GraphQLVariables, cacheOptions?: CacheOptions, refreshTokenOnAuthorizedResponse?: () => RefreshTokenOnAuthorizedResponse): Promise<T>;
export declare const generateFetchAppLogUrl: (cursor?: string, filters?: {
status?: string;
source?: string;
@@ -21,9 +22,10 @@ export declare const generateFetchAppLogUrl: (cursor?: string, filters?: {
* @param query - GraphQL query to execute.
* @param token - Partners token.
* @param variables - GraphQL variables to pass to the query.
+ * @param refreshTokenOnAuthorizedResponse - Optional handler for unauthorized requests.
* @returns The response of the query of generic type <TResult>.
*/
-export declare function partnersRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables>, token: string, variables?: TVariables): Promise<TResult>;
+export declare function partnersRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables>, token: string, variables?: TVariables, refreshTokenOnAuthorizedResponse?: () => RefreshTokenOnAuthorizedResponse): Promise<TResult>;
/**
* Sets the next deprecation date from [GraphQL response extensions](https://www.apollographql.com/docs/resources/graphql-glossary/#extensions)
* if objects contain a (ISO 8601-formatted string).
|
WHY are these changes introduced?
To improve error handling in the Partners API client by adding support for token refresh when unauthorized responses are received.
WHAT is this pull request doing?
Adds an unauthorized handler to the Partners API client that automatically refreshes the token when an unauthorized response is received. This prevents API requests from failing due to expired tokens by:
unauthorizedHandler
parameter topartnersRequest
andpartnersRequestDoc
functionsPartnersClient
class that refreshes the token when neededMeasuring impact
We should see an improvement in our internal command success metrics.
Look out for
Do we need to increase the scope of this? Other services and such.
Checklist