Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
fix(server): scope HTTP 500 retry to Test Replay artifact uploads
Per review feedback, only retry on 500 for the Test Replay artifact
upload (which is idempotent). Other cloud requests using the shared
`isRetryableError` predicate may be non-idempotent — retrying after a
500 risks duplicating partially-applied work.

The 500 retry is now applied as a local override at the upload call
site in `put_protocol_artifact.ts`. The shared `isRetryableError` is
restored to its previous status list.
  • Loading branch information
mschile committed May 5, 2026
commit 9154fa003ec43f03acf0dfd80521e1f7a12c7a8d
2 changes: 1 addition & 1 deletion cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
**Bugfixes:**

- Fixed an issue where multi-origin tests using [`cy.origin`](https://docs.cypress.io/api/commands/origin) could fail to talk to a secondary origin after test isolation, when the spec-bridge iframe was already present, or when more than one secondary origin became ready around the same time. Cached spec-bridge window targets are now cleared at the correct lifecycle points, improving performance of specs with cy.origin calls. Addressed in [#33704](https://github.com/cypress-io/cypress/pull/33704).
- Fixed an issue where transient HTTP 500 responses from Cypress Cloud during Test Replay artifact uploads, Studio bundle/session requests, and `cy.prompt` bundle/session requests were not retried, causing avoidable failures. Fixed in [#33718](https://github.com/cypress-io/cypress/pull/33718).
- Fixed an issue where transient HTTP 500 responses during Test Replay artifact uploads were not retried, causing avoidable upload failures. Fixed in [#33718](https://github.com/cypress-io/cypress/pull/33718).

**Misc:**

Expand Down
3 changes: 2 additions & 1 deletion packages/server/lib/cloud/api/put_protocol_artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { StreamActivityMonitor } from '../upload/stream_activity_monitor'
import { asyncRetry, linearDelay } from '../../util/async_retry'
import { putFetch, ParseKinds } from '../network/fetch'
import { isRetryableError } from '../network/is_retryable_error'
import { HttpError } from '../network/http_error'
const debug = Debug('cypress:server:cloud:api:protocol-artifact')

const _delay = linearDelay(500)
Expand Down Expand Up @@ -38,6 +39,6 @@ export const putProtocolArtifact = asyncRetry(
}, {
maxAttempts: 3,
retryDelay: _delay,
shouldRetry: isRetryableError,
shouldRetry: (err) => isRetryableError(err) || (HttpError.isHttpError(err) && err.status === 500),
},
)
2 changes: 1 addition & 1 deletion packages/server/lib/cloud/network/is_retryable_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const isRetryableError = (error: any) => {
}

if (HttpError.isHttpError(error)) {
return [408, 429, 500, 502, 503, 504].includes(error.status)
return [408, 429, 502, 503, 504].includes(error.status)
}

return false
Expand Down
30 changes: 27 additions & 3 deletions packages/server/test/unit/cloud/api/put_protocol_artifact_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { StreamActivityMonitor } from '../../../../lib/cloud/upload/stream_activ
import { HttpError } from '../../../../lib/cloud/network/http_error'
import { putFetch, ParseKinds } from '../../../../lib/cloud/network/fetch'
import { linearDelay, asyncRetry } from '../../../../lib/util/async_retry'
import { isRetryableError } from '../../../../lib/cloud/network/is_retryable_error'
import type { putProtocolArtifact } from '../../../../lib/cloud/api/put_protocol_artifact'

chai.use(chaiAsPromised).use(sinonChai)
Expand Down Expand Up @@ -129,8 +128,33 @@ describe('putProtocolArtifact', () => {

expect(options.maxAttempts).to.eq(3)
expect(options.retryDelay).to.be.a('function')
// because of mockery, the isRetryableError ref here is different than the one imported into put_protocol_artifact_spec
expect(options.shouldRetry?.toString()).to.eq(isRetryableError.toString())
})

describe('shouldRetry', () => {
let shouldRetry: (err: any) => boolean

beforeEach(() => {
shouldRetry = asyncRetryStub.firstCall.args[1].shouldRetry!
})

// Test Replay uploads are idempotent, so retrying on 500 is safe — the
// shared `isRetryableError` predicate excludes 500 to avoid retrying
// potentially non-idempotent operations on other cloud endpoints.
it('retries on HTTP 500 in addition to the shared retryable statuses', () => {
const retryableStatuses = [408, 429, 500, 502, 503, 504]

retryableStatuses.forEach((status) => {
const err = new HttpError('some error', 'http://some/url', status, 'status text', '', sinon.createStubInstance(Response))

expect(shouldRetry(err), `status ${status}`).to.be.true
})
})

it('does not retry on non-retryable HTTP errors', () => {
const err = new HttpError('some error', 'http://some/url', 400, 'Bad Request', '', sinon.createStubInstance(Response))

expect(shouldRetry(err)).to.be.false
})
})

describe('when provided an artifact path that does not exist', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ describe('isRetryableError', () => {
})

it('returns true with retryable http errors', () => {
[408, 429, 500, 502, 503, 504].forEach((status) => {
[408, 429, 502, 503, 504].forEach((status) => {
const err = new HttpError('some error', url, status, 'status text', 'response_body', sinon.createStubInstance(Response))

expect(isRetryableError(err)).to.be.true
})
})

it('returns false with non-retryable http errors', () => {
[400, 401, 402, 403, 404, 405, 406, 407, 409, 410, 411, 412, 413, 414, 416, 417, 418, 421, 422, 423, 424, 425, 426, 428, 431, 451, 501, 505, 507, 508, 510, 511].forEach((status) => {
[400, 401, 402, 403, 404, 405, 406, 407, 409, 410, 411, 412, 413, 414, 416, 417, 418, 421, 422, 423, 424, 425, 426, 428, 431, 451, 500, 501, 505, 507, 508, 510, 511].forEach((status) => {
const err = new HttpError('some error', url, status, 'status text', 'response_body', sinon.createStubInstance(Response))

expect(isRetryableError(err)).to.be.false
Expand Down
Loading