Skip to content

Commit

Permalink
chore: improve logging on retried upload failures (#27847)
Browse files Browse the repository at this point in the history
  • Loading branch information
ryanthemanuel authored Sep 20, 2023
1 parent 0ba2a48 commit 311001b
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 15 deletions.
12 changes: 7 additions & 5 deletions packages/server/lib/cloud/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const DELETE_DB = !process.env.CYPRESS_LOCAL_PROTOCOL_PATH

// Timeout for upload
const TWO_MINUTES = 120000
const RETRY_DELAYS = [500, 100, 2000, 4000, 8000]
const RETRY_DELAYS = [500, 1000, 2000, 4000, 8000, 16000, 32000]
const DB_SIZE_LIMIT = 5000000000

const dbSizeLimit = () => {
Expand Down Expand Up @@ -278,7 +278,7 @@ export class ProtocolManager implements ProtocolManagerShape {

debug(`uploading %s to %s with a file size of %s`, archivePath, uploadUrl, fileSize)

const retryRequest = async (retryCount: number) => {
const retryRequest = async (retryCount: number, errors: Error[]) => {
try {
if (fileSize > dbSizeLimit()) {
throw new Error(`Spec recording too large: db is ${fileSize} bytes, limit is ${dbSizeLimit()} bytes`)
Expand Down Expand Up @@ -327,16 +327,18 @@ export class ProtocolManager implements ProtocolManagerShape {
debug(`retrying upload %o`, { retryCount })
await new Promise((resolve) => setTimeout(resolve, RETRY_DELAYS[retryCount]))

return await retryRequest(retryCount + 1)
return await retryRequest(retryCount + 1, [...errors, e])
}
}

throw e
const totalErrors = [...errors, e]

throw new AggregateError(totalErrors, e.message)
}
}

try {
return await retryRequest(0)
return await retryRequest(0, [])
} catch (e) {
if (CAPTURE_ERRORS) {
this._errors.push({
Expand Down
20 changes: 19 additions & 1 deletion packages/server/lib/modes/record.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,19 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {
stack: err.stack,
})

if (err.errors) {
const lastError = _.last(err.errors)

return {
key: artifact.reportKey,
success: false,
error: lastError.message,
allErrors: err.errors,
url: artifact.uploadUrl,
pathToFile: artifact.filePath,
}
}

return {
key: artifact.reportKey,
success: false,
Expand Down Expand Up @@ -320,9 +333,14 @@ const uploadArtifactBatch = async (artifacts, protocolManager, quiet) => {

return uploadResults.reduce((acc, { key, skipped, ...report }) => {
if (key === 'protocol') {
const error = report.allErrors ? `Failed to upload after ${report.allErrors.length} attempts. Errors: ${report.allErrors.map((error) => error.message).join(', ')}` : report.error

return skipped && !report.error ? acc : {
...acc,
[key]: report,
[key]: {
...report,
error,
},
}
}

Expand Down
92 changes: 87 additions & 5 deletions system-tests/__snapshots__/record_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3387,7 +3387,88 @@ exports['e2e record capture-protocol enabled protocol runtime errors error in pr
`

exports['capture-protocol api errors upload 500 - retries 6 times continues 1'] = `
exports['e2e record capture-protocol enabled protocol runtime errors error in protocol beforeTest displays the error and reports the fatal error to the cloud via artifacts 1'] = `
====================================================================================================
(Run Starting)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Cypress: 1.2.3 │
│ Browser: FooBrowser 88 │
│ Specs: 1 found (record_pass.cy.js) │
│ Searched: cypress/e2e/record_pass* │
│ Params: Tag: false, Group: false, Parallel: false │
│ Run URL: https://dashboard.cypress.io/projects/cjvoj7/runs/12 │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────────────────────────
Running: record_pass.cy.js (1 of 1)
Estimated: X second(s)
record pass
✓ passes
- is pending
1 passing
1 pending
(Results)
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 2 │
│ Passing: 1 │
│ Failing: 0 │
│ Pending: 1 │
│ Skipped: 0 │
│ Screenshots: 1 │
│ Video: false │
│ Duration: X seconds │
│ Estimated: X second(s) │
│ Spec Ran: record_pass.cy.js │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
(Screenshots)
- /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png (400x1022)
(Uploading Cloud Artifacts)
- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Capturing - error in beforeTest
(Uploaded Cloud Artifacts)
- Screenshot - Done Uploading 1 kB 1/1 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
====================================================================================================
(Run Finished)
Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ record_pass.cy.js XX:XX 2 1 - 1 - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 2 1 - 1 -
───────────────────────────────────────────────────────────────────────────────────────────────────────
Recorded Run: https://dashboard.cypress.io/projects/cjvoj7/runs/12
`

exports['capture-protocol api errors upload 500 - retries 8 times continues 1'] = `
====================================================================================================
Expand Down Expand Up @@ -3469,7 +3550,7 @@ exports['capture-protocol api errors upload 500 - retries 6 times continues 1']
`

exports['capture-protocol api errors upload 500 - retries 5 times and succeeds on the last call continues 1'] = `
exports['capture-protocol api errors upload 500 - retries 7 times and succeeds on the last call continues 1'] = `
====================================================================================================
Expand Down Expand Up @@ -3551,7 +3632,7 @@ exports['capture-protocol api errors upload 500 - retries 5 times and succeeds o
`

exports['e2e record capture-protocol enabled protocol runtime errors error in protocol beforeTest displays the error and reports the fatal error to the cloud via artifacts 1'] = `
exports['capture-protocol api errors upload 500 - retries 8 times and fails continues 1'] = `
====================================================================================================
Expand Down Expand Up @@ -3607,11 +3688,12 @@ exports['e2e record capture-protocol enabled protocol runtime errors error in pr
- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Capturing - error in beforeTest
- Test Replay
(Uploaded Cloud Artifacts)
- Screenshot - Done Uploading 1 kB 1/1 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Screenshot - Done Uploading 1 kB 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Uploading 2/2 - Internal Server Error
====================================================================================================
Expand Down
16 changes: 12 additions & 4 deletions system-tests/test/record_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2322,7 +2322,6 @@ describe('e2e record', () => {
})

it('displays error and does not upload if db size is too large', function () {
// have to write the db to fs here instead of in the t
return systemTests.exec(this, {
key: 'f858a2bc-b469-4e48-be67-0876339ee7e1',
configFile: 'cypress-with-project-id.config.js',
Expand Down Expand Up @@ -2477,7 +2476,7 @@ describe('capture-protocol api errors', () => {
}))
}

describe('upload 500 - retries 6 times', () => {
describe('upload 500 - retries 8 times and fails', () => {
stubbedServerWithErrorOn('putCaptureProtocolUpload')
it('continues', function () {
process.env.API_RETRY_INTERVALS = '1000'
Expand All @@ -2488,12 +2487,21 @@ describe('capture-protocol api errors', () => {
spec: 'record_pass*',
record: true,
snapshot: true,
}).then(() => {
const urls = getRequestUrls()

expect(urls).to.include.members([`PUT /instances/${instanceId}/artifacts`])

const artifactReport = getRequests().find(({ url }) => url === `PUT /instances/${instanceId}/artifacts`)?.body

expect(artifactReport?.protocol).to.exist()
expect(artifactReport?.protocol?.error).to.equal('Failed to upload after 8 attempts. Errors: Internal Server Error, Internal Server Error, Internal Server Error, Internal Server Error, Internal Server Error, Internal Server Error, Internal Server Error, Internal Server Error')
})
})
})

describe('upload 500 - retries 5 times and succeeds on the last call', () => {
stubbedServerWithErrorOn('putCaptureProtocolUpload', 5)
describe('upload 500 - retries 7 times and succeeds on the last call', () => {
stubbedServerWithErrorOn('putCaptureProtocolUpload', 7)

let archiveFile = ''

Expand Down

4 comments on commit 311001b

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 311001b Sep 20, 2023

Choose a reason for hiding this comment

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

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.3.0/linux-x64/develop-311001b7d1c0a6389e46bbefb9b7084392ffd6c9/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 311001b Sep 20, 2023

Choose a reason for hiding this comment

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

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.3.0/linux-arm64/develop-311001b7d1c0a6389e46bbefb9b7084392ffd6c9/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 311001b Sep 20, 2023

Choose a reason for hiding this comment

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

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.3.0/darwin-x64/develop-311001b7d1c0a6389e46bbefb9b7084392ffd6c9/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 311001b Sep 20, 2023

Choose a reason for hiding this comment

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

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/13.3.0/win32-x64/develop-311001b7d1c0a6389e46bbefb9b7084392ffd6c9/cypress.tgz

Please sign in to comment.