Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve logging on retried upload failures #27847

Merged
merged 3 commits into from
Sep 20, 2023
Merged

chore: improve logging on retried upload failures #27847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a17ecd
chore: improve logging on retried upload failures
ryanthemanuel Sep 19, 2023
3dc44d4
Apply suggestions from code review
ryanthemanuel Sep 19, 2023
cd3528e
Merge branch 'develop' into ryanm/chore/improve-logging-on-upload-fai…
ryanthemanuel Sep 20, 2023
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
12 changes: 7 additions & 5 deletions packages/server/lib/cloud/protocol.ts
View file
Open in desktop
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
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading