From 2a17ecd92c417d26a01cf42a7b5c784bcd23386f Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Tue, 19 Sep 2023 15:40:13 -0500 Subject: [PATCH 1/2] chore: improve logging on retried upload failures --- packages/server/lib/cloud/protocol.ts | 12 +-- packages/server/lib/modes/record.js | 20 ++++- system-tests/__snapshots__/record_spec.js | 104 +++++++++++++++++++--- system-tests/test/record_spec.js | 16 +++- 4 files changed, 131 insertions(+), 21 deletions(-) diff --git a/packages/server/lib/cloud/protocol.ts b/packages/server/lib/cloud/protocol.ts index 3631d173d711..e7bf72d3b0e5 100644 --- a/packages/server/lib/cloud/protocol.ts +++ b/packages/server/lib/cloud/protocol.ts @@ -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 = () => { @@ -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`) @@ -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({ diff --git a/packages/server/lib/modes/record.js b/packages/server/lib/modes/record.js index 01f90aa46105..22997cbc4a75 100644 --- a/packages/server/lib/modes/record.js +++ b/packages/server/lib/modes/record.js @@ -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, @@ -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, + }, } } diff --git a/system-tests/__snapshots__/record_spec.js b/system-tests/__snapshots__/record_spec.js index a592cf617c90..c106da1f99e1 100644 --- a/system-tests/__snapshots__/record_spec.js +++ b/system-tests/__snapshots__/record_spec.js @@ -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': { @@ -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': { @@ -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': { @@ -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'] = ` ==================================================================================================== @@ -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'] = ` ==================================================================================================== @@ -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'] = ` ==================================================================================================== @@ -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 ==================================================================================================== diff --git a/system-tests/test/record_spec.js b/system-tests/test/record_spec.js index 6bbdf75c937f..97a4348d0788 100644 --- a/system-tests/test/record_spec.js +++ b/system-tests/test/record_spec.js @@ -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', @@ -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' @@ -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 = '' From 3dc44d42e98e1b88b1e81335cb2b7cc33aac66fe Mon Sep 17 00:00:00 2001 From: Ryan Manuel Date: Tue, 19 Sep 2023 15:42:23 -0500 Subject: [PATCH 2/2] Apply suggestions from code review --- system-tests/__snapshots__/record_spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/system-tests/__snapshots__/record_spec.js b/system-tests/__snapshots__/record_spec.js index c106da1f99e1..25e60b8a49b2 100644 --- a/system-tests/__snapshots__/record_spec.js +++ b/system-tests/__snapshots__/record_spec.js @@ -1277,8 +1277,8 @@ exports['e2e record passing passes 2'] = [ 'testId': 'r3', 'testAttemptIndex': 0, 'takenAt': '2018-02-01T20:14:19.323Z', - 'height': 1440, - 'width': 2560, + 'height': 720, + 'width': 1280, }, ], 'reporterStats': { @@ -1357,8 +1357,8 @@ exports['e2e record passing passes 2'] = [ 'testId': 'r3', 'testAttemptIndex': 0, 'takenAt': '2018-02-01T20:14:19.323Z', - 'height': 2044, - 'width': 800, + 'height': 1022, + 'width': 400, }, ], 'reporterStats': { @@ -1434,8 +1434,8 @@ exports['e2e record passing passes 2'] = [ 'testId': 'r2', 'testAttemptIndex': 0, 'takenAt': '2018-02-01T20:14:19.323Z', - 'height': 1440, - 'width': 2560, + 'height': 720, + 'width': 1280, }, ], 'reporterStats': {