Skip to content

Commit

Permalink
fix: do not retry uploads on 4xx errors (#6361)
Browse files Browse the repository at this point in the history
  • Loading branch information
kitop authored Jan 30, 2024
1 parent 86ce501 commit 0996232
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/utils/deploy/upload-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,15 @@ const retryUpload = (uploadFn, maxRetry) =>
} catch (error) {
lastError = error

// We don't need to retry for 400 or 422 errors
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
if (error.status === 400 || error.status === 422) {
return reject(error)
}

// observed errors: 408, 401 (4** swallowed), 502
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
if (error.status >= 400 || error.name === 'FetchError') {
if (error.status > 400 || error.name === 'FetchError') {
fibonacciBackoff.backoff()
return
}
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/utils/deploy/upload-files.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,36 @@ test('Adds a retry count to function upload requests', async () => {
expect(uploadDeployFunction).toHaveBeenNthCalledWith(2, expect.objectContaining({ xNfRetryCount: 1 }))
expect(uploadDeployFunction).toHaveBeenNthCalledWith(3, expect.objectContaining({ xNfRetryCount: 2 }))
})

test('Does not retry on 400 response from function upload requests', async () => {
const uploadDeployFunction = vi.fn()
const mockError = new Error('Uh-oh')

mockError.status = 400

uploadDeployFunction.mockRejectedValue(mockError)

const mockApi = {
uploadDeployFunction,
}
const deployId = generateUUID()
const files = [
{
assetType: 'function',
filepath: '/some/path/func1.zip',
normalizedPath: 'func1.zip',
runtime: 'js',
},
]
const options = {
concurrentUpload: 1,
maxRetry: 3,
statusCb: vi.fn(),
}

try {
await uploadFiles(mockApi, deployId, files, options)
} catch {}

expect(uploadDeployFunction).toHaveBeenCalledTimes(1)
})

2 comments on commit 0996232

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,282
  • Package size: 280 MB
  • Number of ts-expect-error directives: 1,172

@github-actions
Copy link

Choose a reason for hiding this comment

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

📊 Benchmark results

  • Dependency count: 1,282
  • Package size: 280 MB
  • Number of ts-expect-error directives: 1,172

Please sign in to comment.