Skip to content

Commit

Permalink
chore: improve logging on retried upload failures
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryanthemanuel
ryanthemanuel committed Sep 19, 2023
1 parent dc282cd commit 2a17ecd
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 21 deletions.
12 changes: 7 additions & 5 deletions packages/server/lib/cloud/protocol.ts
View file
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
View file
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
104 changes: 93 additions & 11 deletions system-tests/__snapshots__/record_spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1277,8 +1277,8 @@ exports['e2e record passing passes 2'] = [
'testId': 'r3',
'testAttemptIndex': 0,
'takenAt': '2018-02-01T20:14:19.323Z',
'height': 720,
'width': 1280,
'height': 1440,
'width': 2560,
},
],
'reporterStats': {
Expand Down Expand Up @@ -1357,8 +1357,8 @@ exports['e2e record passing passes 2'] = [
'testId': 'r3',
'testAttemptIndex': 0,
'takenAt': '2018-02-01T20:14:19.323Z',
'height': 1022,
'width': 400,
'height': 2044,
'width': 800,
},
],
'reporterStats': {
Expand Down Expand Up @@ -1434,8 +1434,8 @@ exports['e2e record passing passes 2'] = [
'testId': 'r2',
'testAttemptIndex': 0,
'takenAt': '2018-02-01T20:14:19.323Z',
'height': 720,
'width': 1280,
'height': 1440,
'width': 2560,
},
],
'reporterStats': {
Expand Down 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
View file
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

0 comments on commit 2a17ecd

Please sign in to comment.