Skip to content

chore: additional context on protocol upload network errors #28986

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

Merged
merged 7 commits into from
Mar 6, 2024
Merged

chore: additional context on protocol upload network errors #28986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a81b0cd
chore: add additional context to messaging when network errors occur …
cacieprins Feb 20, 2024
bd2e429
Merge branch 'develop' into cacie/chore/additional-upload-error-info
cacieprins Feb 21, 2024
2e0f6da
reduce total attempts for uploading capture artifacts from 8 to 3
cacieprins Feb 22, 2024
42f1c74
Merge branch 'develop' into cacie/chore/additional-upload-error-info
cacieprins Feb 22, 2024
5341806
Merge branch 'develop' into cacie/chore/additional-upload-error-info
cacieprins Feb 26, 2024
4eae70e
Merge branch 'develop' into cacie/chore/additional-upload-error-info
cacieprins Mar 4, 2024
b2b3bfb
Merge branch 'develop' into cacie/chore/additional-upload-error-info
cacieprins Mar 6, 2024
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
14 changes: 12 additions & 2 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, 1000, 2000, 4000, 8000, 16000, 32000]
const RETRY_DELAYS = [500, 1000]
const DB_SIZE_LIMIT = 5000000000

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

const errorMessage = await res.json().catch(() => res.statusText)
const errorMessage = await res.json().catch(() => {
const url = new URL(uploadUrl)
Copy link
Member

Choose a reason for hiding this comment

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

Can't imagine why this wouldn't be a valid url, but should we gaurd against if this ends up raising an error as invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While investigating this, I don't think we need to guard at this point, because this code is unreachable if uploadUrl is not a valid URL: fetch on L#305 will throw in this case, which gets caught on L#345.


for (const [key, value] of url.searchParams) {
if (['x-amz-credential', 'x-amz-signature'].includes(key.toLowerCase())) {
url.searchParams.set(key, 'X'.repeat(value.length))
}
}

return `${res.status} ${res.statusText} (${url.href})`
})

debug(`error response: %O`, errorMessage)

Expand Down
36 changes: 18 additions & 18 deletions system-tests/__snapshots__/record_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3401,7 +3401,13 @@ exports['e2e record capture-protocol enabled protocol runtime errors non-fatal e

`

exports['capture-protocol api errors upload 500 - retries 8 times and fails continues 1'] = `
exports['capture-protocol api errors fetch script 500 continues 1'] = `
We encountered an unexpected error communicating with our servers.

StatusCodeError: 500 - "500 - Internal Server Error"

We will retry 1 more time in X second(s)...


====================================================================================================

Expand Down Expand Up @@ -3457,14 +3463,13 @@ exports['capture-protocol api errors upload 500 - retries 8 times and fails cont

- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay
- Test Replay - Failed Capturing - Error downloading capture code: 500 - "500 - Internal Server Error"

Uploading Cloud Artifacts: . . . . .

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Failed Uploading after Xm, Ys ZZ.ZZms 2/2 - Internal Server Error
- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/1 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png

====================================================================================================

Expand All @@ -3485,7 +3490,7 @@ exports['capture-protocol api errors upload 500 - retries 8 times and fails cont

`

exports['capture-protocol api errors upload 500 - retries 7 times and succeeds on the last call continues 1'] = `
exports['capture-protocol api errors error report 500 continues 1'] = `

====================================================================================================

Expand Down Expand Up @@ -3569,13 +3574,7 @@ exports['capture-protocol api errors upload 500 - retries 7 times and succeeds o

`

exports['capture-protocol api errors fetch script 500 continues 1'] = `
We encountered an unexpected error communicating with our servers.

StatusCodeError: 500 - "500 - Internal Server Error"

We will retry 1 more time in X second(s)...

exports['e2e record capture-protocol enabled passing retrieves the capture protocol, uploads the db, and updates the artifact upload report 1'] = `

====================================================================================================

Expand Down Expand Up @@ -3631,13 +3630,14 @@ We will retry 1 more time in X second(s)...

- 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 downloading capture code: 500 - "500 - Internal Server Error"
- Test Replay - 1 kB

Uploading Cloud Artifacts: . . . . .

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/1 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 2/2

====================================================================================================

Expand All @@ -3658,7 +3658,7 @@ We will retry 1 more time in X second(s)...

`

exports['capture-protocol api errors error report 500 continues 1'] = `
exports['capture-protocol api errors upload 500 - retries 3 times and fails continues 1'] = `

====================================================================================================

Expand Down Expand Up @@ -3714,14 +3714,14 @@ exports['capture-protocol api errors error report 500 continues 1'] = `

- Video - Nothing to upload
- Screenshot - 1 kB /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - 1 kB
- Test Replay

Uploading Cloud Artifacts: . . . . .

(Uploaded Cloud Artifacts)

- Screenshot - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 1/2 /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png
- Test Replay - Done Uploading 1 kB in Xm, Ys ZZ.ZZms 2/2
- Test Replay - Failed Uploading after Xm, Ys ZZ.ZZms 2/2 - 500 Internal Server Error (http://localhost:1234/capture-protocol/upload/?x-amz-credential=XXXXXXXX&x-amz-signature=XXXXXXXXXXXXX)

====================================================================================================

Expand All @@ -3742,7 +3742,7 @@ exports['capture-protocol api errors error report 500 continues 1'] = `

`

exports['e2e record capture-protocol enabled passing retrieves the capture protocol, uploads the db, and updates the artifact upload report 1'] = `
exports['capture-protocol api errors upload 500 - retries 2 times and succeeds on the last call continues 1'] = `

====================================================================================================

Expand Down
2 changes: 1 addition & 1 deletion system-tests/lib/serverStub.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ postInstanceTestsResponse.actions = []
export const postRunResponse = _.assign({}, postRunResponseWithWarnings, { warnings: [] })

// mocked here rather than attempting to intercept and mock an s3 req
export const CAPTURE_PROTOCOL_UPLOAD_URL = '/capture-protocol/upload/'
export const CAPTURE_PROTOCOL_UPLOAD_URL = '/capture-protocol/upload/?x-amz-credential=1234abcd&x-amz-signature=1a2b3c-4d5e6f'

let protocolStub: {
value: string
Expand Down
13 changes: 8 additions & 5 deletions system-tests/test/record_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2307,7 +2307,7 @@ describe('e2e record', () => {
snapshot: true,
}).then((ret) => {
const urls = getRequestUrls()
const artifactReport = getRequests().find(({ url }) => url === `PUT /instances/${instanceId}/artifacts`)?.body
const artifactReport = getRequests().find(({ url }) => url.includes(`/instances/${instanceId}/artifacts`))?.body

expect(urls).to.include.members([`PUT ${CAPTURE_PROTOCOL_UPLOAD_URL}`])

Expand Down Expand Up @@ -2541,7 +2541,7 @@ describe('capture-protocol api errors', () => {
}))
}

describe('upload 500 - retries 8 times and fails', () => {
describe('upload 500 - retries 3 times and fails', () => {
stubbedServerWithErrorOn('putCaptureProtocolUpload')
it('continues', function () {
process.env.API_RETRY_INTERVALS = '1000'
Expand All @@ -2560,14 +2560,17 @@ describe('capture-protocol api errors', () => {
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')
expect(artifactReport?.protocol?.error).to.equal(
'Failed to upload after 3 attempts. Errors: 500 Internal Server Error (http://localhost:1234/capture-protocol/upload/?x-amz-credential=XXXXXXXX&x-amz-signature=XXXXXXXXXXXXX), 500 Internal Server Error (http://localhost:1234/capture-protocol/upload/?x-amz-credential=XXXXXXXX&x-amz-signature=XXXXXXXXXXXXX), 500 Internal Server Error (http://localhost:1234/capture-protocol/upload/?x-amz-credential=XXXXXXXX&x-amz-signature=XXXXXXXXXXXXX)',
)

expect(artifactReport?.protocol?.errorStack).to.exist().and.not.to.be.empty()
})
})
})

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

let archiveFile = ''

Expand Down