Skip to content
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

feat(rest-client): retry requests on network errors #449

Merged
merged 2 commits into from
Nov 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions packages/rest-client/src/makeApiRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { fetch, Headers, Request } from './lib/fetch/fetch.node'
import { applyDefaultSettings, UserSettings } from './settings'
import { getUserAgent } from './tools/getUserAgent'
import { RestClientError } from './tools/RestClientError'
import { retryIfThrottled } from './tools/retryIfThrottled'
import { retryIfFailed } from './tools/retryIfFailed'

export type ApiRequestQuery = Record<
string,
Expand Down Expand Up @@ -85,10 +85,10 @@ export async function makeApiRequest(
body: requestBody
})

const response = await retryIfThrottled(
() => fetch(signedRequest),
settings.retryThrottledRequestMaxTimes
)
const response = await retryIfFailed(() => fetch(signedRequest), {
retryThrottledRequestMaxTimes: settings.retryThrottledRequestMaxTimes,
retryNetworkErrorMaxTimes: settings.retryNetworkErrorMaxTimes
})

return {
request: signedRequest,
Expand Down
9 changes: 7 additions & 2 deletions packages/rest-client/src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,22 @@ export type UserSettings = {
authSchema: AuthSchema
apiBaseURL?: string
retryThrottledRequestMaxTimes?: number
retryNetworkErrorMaxTimes?: number
integration?: string
userAgent?: CustomUserAgent
}

export type DefaultSettings = Required<
Pick<UserSettings, 'apiBaseURL' | 'retryThrottledRequestMaxTimes'>
Pick<
UserSettings,
'apiBaseURL' | 'retryThrottledRequestMaxTimes' | 'retryNetworkErrorMaxTimes'
>
>

export const defaultSettings: DefaultSettings = {
apiBaseURL: 'https://api.uploadcare.com/',
retryThrottledRequestMaxTimes: 5
retryThrottledRequestMaxTimes: 5,
retryNetworkErrorMaxTimes: 3
}

export const applyDefaultSettings = (
Expand Down
114 changes: 114 additions & 0 deletions packages/rest-client/src/tools/retryIfFailed.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import { expect, jest } from '@jest/globals'
import { Headers, Response } from '../lib/fetch/fetch.node'
import { RestClientError } from './RestClientError'
import { retryIfFailed } from './retryIfFailed'

const THROTTLED_RESPONSE = new Response(
JSON.stringify({
detail: 'Request was throttled. Expected available in 1 second.'
}),
{
status: 429,
statusText: 'Too Many Requests',
headers: new Headers({
'retry-after': '1'
})
}
)

const createRunner = ({
error,
attempts,
response = new Response()
}: {
attempts: number
error?: Error
response?: Response
}) => {
let runs = 0
const mock = jest.fn(async () => {
++runs

if (runs <= attempts) {
if (error) {
throw error
}
return THROTTLED_RESPONSE
}

return response
})

return mock
}

describe('retryIfFailed', () => {
it("should be resolved with task's resolvee if was nor throttled nor failed", async () => {
const response = new Response()
const mock = createRunner({ attempts: 0, response })

await expect(
retryIfFailed(mock, {
retryThrottledRequestMaxTimes: 5,
retryNetworkErrorMaxTimes: 5
})
).resolves.toEqual(response)
expect(mock).toHaveBeenCalledTimes(1)
})

describe('throttling', () => {
it('should be rejected with RestClientError if was throttled and limit was left', async () => {
const mock = createRunner({ attempts: 10 })

await expect(
retryIfFailed(mock, {
retryThrottledRequestMaxTimes: 5,
retryNetworkErrorMaxTimes: 0
})
).rejects.toThrowError(RestClientError)
expect(mock).toHaveBeenCalledTimes(6)
})

it("should be resolved with task's resolvee if was throttled and limit was not left", async () => {
const response = new Response()
const mock = createRunner({ attempts: 3, response })

await expect(
retryIfFailed(mock, {
retryThrottledRequestMaxTimes: 10,
retryNetworkErrorMaxTimes: 0
})
).resolves.toEqual(response)
expect(mock).toHaveBeenCalledTimes(4)
})
})

describe('network errors', () => {
it("should be rejected with tasks's error if limit was left", async () => {
const error = new Error()
const mock = createRunner({ attempts: 5, error })

await expect(
retryIfFailed(mock, {
retryThrottledRequestMaxTimes: 0,
retryNetworkErrorMaxTimes: 2
})
).rejects.toThrowError(error)
expect(mock).toHaveBeenCalledTimes(3)
})

it("should be resolved with tasks's resolvee if limit was not left", async () => {
const error = new Error()
const response = new Response()
const mock = createRunner({ attempts: 2, error, response })

await expect(
retryIfFailed(mock, {
retryThrottledRequestMaxTimes: 0,
retryNetworkErrorMaxTimes: 5
})
).resolves.toEqual(response)
expect(mock).toHaveBeenCalledTimes(3)
})
})
})
48 changes: 48 additions & 0 deletions packages/rest-client/src/tools/retryIfFailed.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { retrier } from '@uploadcare/api-client-utils'
import { RestClientError } from './RestClientError'

const THROTTLED_STATUS = 429
const DEFAULT_RETRY_AFTER_TIMEOUT = 15000
const DEFAULT_NETWORK_ERROR_TIMEOUT = 1000

function getTimeoutFromThrottledRequest(response: Response): number {
const { headers } = response
if (!headers || !headers.get('retry-after')) {
return DEFAULT_RETRY_AFTER_TIMEOUT
}
const seconds = parseInt(headers.get('retry-after') as string, 10)
if (!Number.isFinite(seconds)) {
return DEFAULT_RETRY_AFTER_TIMEOUT
}
return seconds * 1000
}

export function retryIfFailed(
fn: () => Promise<Response>,
options: {
retryThrottledRequestMaxTimes: number
retryNetworkErrorMaxTimes: number
}
): Promise<Response> {
return retrier(({ attempt, retry }) =>
fn()
.then(async (response) => {
if (response.status !== THROTTLED_STATUS) {
return response
}
if (attempt < options.retryThrottledRequestMaxTimes) {
return retry(getTimeoutFromThrottledRequest(response))
}
const json = await response.json()
const { detail } = json
throw new RestClientError(detail, { response })
})
.catch((error) => {
if (attempt < options.retryNetworkErrorMaxTimes) {
return retry((attempt + 1) * DEFAULT_NETWORK_ERROR_TIMEOUT)
}

throw error
})
)
}
79 changes: 0 additions & 79 deletions packages/rest-client/src/tools/retryIfThrottled.test.ts

This file was deleted.

35 changes: 0 additions & 35 deletions packages/rest-client/src/tools/retryIfThrottled.ts

This file was deleted.