Skip to content

Commit

Permalink
[Fix][DEVX-514] Fix v3 AbortController Retry Reinitialization (#884)
Browse files Browse the repository at this point in the history
* fix(sdk-v3-abortController): fix issue with abortController

- fix issue with abortController not reinitialized for each request

* Create grumpy-ways-drum.md

* chore(README.md): remove Beta from readme

- Remove Beta highlight from v3 README.md file

* chore(test): add manual test for multiple abort

* fix(DEVX-514): fix abortcontroller

---------

Co-authored-by: lojzatran <[email protected]>
  • Loading branch information
ajimae and lojzatran authored Dec 17, 2024
1 parent 67ef618 commit 3bfcd2e
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/grumpy-ways-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@commercetools/ts-client": patch
---

[Fix][DEVX-514] Fix v3 AbortController Retry Reinitialization
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
} from '../test-utils'
import { createApiBuilderFromCtpClient } from '../../../src'
import fetch from 'node-fetch'
import { TokenCache } from '@commercetools/ts-client'

describe('testing error cases', () => {
it('should throw error when a product type is not found', async () => {
Expand Down Expand Up @@ -63,4 +64,46 @@ describe('testing error cases', () => {

expect(response.statusCode).toBe(200)
})

// This test is skipped because currently I don‘t know how to verify its correctness, so this is only for manual testing
it.skip('should retry correctly multiple times when aborted', async () => {
const client = new ClientBuilderV3()
.withClientCredentialsFlow({
...authMiddlewareOptionsV3,
projectKey: 'a',
credentials: {
clientId: 'test',
clientSecret: 'test',
},
tokenCache: {
set: (cache) => {
console.log(cache)
},
get: (tokenCacheKey) => {
return {
token: 'test',
expirationTime: 9934431427363,
}
},
} as TokenCache,
})
.withHttpMiddleware({
...httpMiddlewareOptionsV3,
host: 'https://example.com:81',
enableRetry: true,
timeout: 10000,
retryConfig: {
retryOnAbort: true,
maxRetries: 4,
retryDelay: 400,
retryCodes: [401],
},
})
.build()

const apiRootV3 = createApiBuilderFromCtpClient(client).withProjectKey({
projectKey,
})
await apiRootV3.get().execute()
}, 999999999)
})
2 changes: 1 addition & 1 deletion packages/sdk-client-v3/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Commercetools Composable Commerce (Improved) TypeScript SDK client (beta)
# Commercetools Composable Commerce (Improved) TypeScript SDK client

This is the new and improved Typescript SDK client.

Expand Down
24 changes: 1 addition & 23 deletions packages/sdk-client-v3/src/middleware/create-http-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,14 @@ async function executeRequest({
httpClient,
clientOptions,
}: HttpOptions): Promise<ClientResult> {
let timer: ReturnType<typeof setTimeout>

const {
timeout,
request,
abortController,
maskSensitiveHeaderData,
includeRequestInErrorResponse,
includeResponseHeaders,
} = clientOptions

try {
if (timeout)
timer = setTimeout(() => {
abortController.abort()
}, timeout)

const response: TResponse = await executor({
url,
...clientOptions,
Expand Down Expand Up @@ -134,8 +125,6 @@ async function executeRequest({
body: null,
error,
}
} finally {
clearTimeout(timer)
}
}

Expand All @@ -162,13 +151,6 @@ export default function createHttpMiddleware(

return (next: Next) => {
return async (request: MiddlewareRequest): Promise<MiddlewareResponse> => {
let abortController: AbortController

if (timeout || getAbortController)
abortController =
(getAbortController ? getAbortController() : null) ||
new AbortController()

const url = host.replace(/\/$/, '') + request.uri
const requestHeader: JsonObject<QueryParam> = { ...request.headers }

Expand Down Expand Up @@ -217,13 +199,9 @@ export default function createHttpMiddleware(
clientOptions.credentialsMode = credentialsMode
}

if (abortController) {
clientOptions.signal = abortController.signal
}

if (timeout) {
clientOptions.timeout = timeout
clientOptions.abortController = abortController
clientOptions.getAbortController = getAbortController
}

if (body) {
Expand Down
4 changes: 3 additions & 1 deletion packages/sdk-client-v3/src/types/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import AbortController from 'abort-controller'
import AbortController, {AbortSignal} from 'abort-controller'

export type Nullable<T> = T | null
export type Keys = string | number | symbol
Expand Down Expand Up @@ -295,6 +295,8 @@ export type IClientOptions = {
body?: Record<string, any> | string | Uint8Array;
timeout?: number
abortController?: AbortController
signal?: AbortSignal,
getAbortController?: () => AbortController
includeOriginalRequest?: boolean
enableRetry?: boolean
retryConfig?: RetryOptions
Expand Down
36 changes: 27 additions & 9 deletions packages/sdk-client-v3/src/utils/executor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { HttpClientConfig, IResponse, TResponse } from '../types/types'
import { calculateRetryDelay, sleep, validateRetryCodes } from '../utils'
import AbortController from 'abort-controller'

function hasResponseRetryCode(
retryCodes: Array<string | number>,
Expand Down Expand Up @@ -36,7 +37,7 @@ export default async function executor(request: HttpClientConfig) {

const data: TResponse = await executeHttpClientRequest(
async (options: HttpClientConfig): Promise<TResponse> => {
const { enableRetry, retryConfig, abortController } = rest
const { enableRetry, retryConfig, timeout, getAbortController } = rest
const {
retryCodes = [],
maxDelay = Infinity,
Expand All @@ -49,7 +50,8 @@ export default async function executor(request: HttpClientConfig) {

let result: string,
data: any,
retryCount: number = 0
retryCount: number = 0,
timer: ReturnType<typeof setTimeout>

// validate the `retryCodes` option
validateRetryCodes(retryCodes)
Expand All @@ -71,12 +73,30 @@ export default async function executor(request: HttpClientConfig) {
})
}

function initializeAbortController() {
const abortController =
(getAbortController ? getAbortController() : null) ||
new AbortController()
rest.abortController = abortController
rest.signal = abortController.signal
return abortController
}

async function executeWithRetry<T = any>(): Promise<T> {
const executeWithTryCatch = async (
retryCodes: (string | number)[],
retryWhenAborted: boolean
) => {
let _response = {} as any
if (timeout) {
let abortController = initializeAbortController()

timer = setTimeout(() => {
abortController.abort()
abortController = initializeAbortController()
}, timeout)
}

try {
_response = await execute()
if (
Expand All @@ -91,16 +111,17 @@ export default async function executor(request: HttpClientConfig) {
} else {
throw e
}
} finally {
clearTimeout(timer)
}

return { _response, shouldRetry: false }
}

const retryWhenAborted =
retryOnAbort || !abortController || !abortController.signal
// first attempt
let { _response, shouldRetry } = await executeWithTryCatch(
retryCodes,
retryWhenAborted
retryOnAbort
)
// retry attempts
while (enableRetry && shouldRetry && retryCount < maxRetries) {
Expand All @@ -117,10 +138,7 @@ export default async function executor(request: HttpClientConfig) {
})
)

const execution = await executeWithTryCatch(
retryCodes,
retryWhenAborted
)
const execution = await executeWithTryCatch(retryCodes, retryOnAbort)
_response = execution._response
shouldRetry = execution.shouldRetry
}
Expand Down

0 comments on commit 3bfcd2e

Please sign in to comment.