Skip to content

Commit b62fc3f

Browse files
cacieprinscacieprins
authored
fix: no longer use capture script url as default upload url for test replay recordings (#29512)
* fix: no longer attempt to upload to capture script location * changelog
1 parent 3706062 commit b62fc3f

File tree

9 files changed

+190
-7
lines changed

9 files changed

+190
-7
lines changed

‎cli/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ _Released 5/21/2024 (PENDING)_
66
**Bugfixes:**
77

88
- Fixed an issue where Cypress was unable to search in the Specs list for files or folders containing numbers. Fixes [#29034](https://github.com/cypress-io/cypress/issues/29034).
9+
- Fixed an issue where Cypress would use the wrong URL to upload Test Replay recordings when it wasn't able to determine the upload URL. It now displays an error when the upload URL cannot be determined, rather than a "Request Entity Too Large" error. Addressed in [#29512](https://github.com/cypress-io/cypress/pull/29512).
910

1011
**Dependency Updates:**
1112

‎packages/errors/__snapshot-html__/CLOUD_PROTOCOL_CANNOT_UPLOAD_ARTIFACT.html

+43
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/errors/src/errors.ts

+9
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,15 @@ export const AllCypressErrors = {
565565
566566
${fmt.highlightSecondary(error)}`
567567
},
568+
CLOUD_PROTOCOL_CANNOT_UPLOAD_ARTIFACT: (error: Error) => {
569+
return errTemplate`\
570+
Warning: We are unable to upload the Test Replay recording of this spec due to a missing or invalid upload URL.
571+
572+
These results will not display Test Replay recordings.
573+
574+
This error will not affect or change the exit code.
575+
`
576+
},
568577
CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE: (error: Error & { url: string, status: number, statusText: string }) => {
569578
return errTemplate`\
570579
Warning: We encountered an HTTP error while uploading the Test Replay recording for this spec.

‎packages/errors/test/unit/visualSnapshotErrors_spec.ts

+7
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,13 @@ describe('visual error templates', () => {
651651
default: [err],
652652
}
653653
},
654+
CLOUD_PROTOCOL_CANNOT_UPLOAD_ARTIFACT: () => {
655+
const err = makeErr()
656+
657+
return {
658+
default: [err],
659+
}
660+
},
654661
CLOUD_PROTOCOL_INITIALIZATION_FAILURE: () => {
655662
const err = makeErr()
656663

‎packages/server/lib/cloud/artifacts/upload_artifacts.ts

+10-6
Original file line numberDiff line numberDiff line change
@@ -106,25 +106,27 @@ const extractArtifactsFromOptions = async ({
106106
try {
107107
const protocolFilePath = protocolManager?.getArchivePath()
108108

109-
const protocolUploadUrl = captureUploadUrl || protocolCaptureMeta.url
110-
111-
const shouldAddProtocolArtifact = protocolManager && protocolFilePath && protocolUploadUrl && !protocolManager.hasFatalError()
109+
const shouldAddProtocolArtifact = protocolManager && protocolFilePath && captureUploadUrl && !protocolManager.hasFatalError()
112110

113111
debug('should add protocol artifact? %o', {
114112
protocolFilePath,
115-
protocolUploadUrl,
113+
captureUploadUrl,
116114
protocolManager: !!protocolManager,
117115
fatalError: protocolManager?.hasFatalError(),
118116
shouldAddProtocolArtifact,
119117
})
120118

121119
if (shouldAddProtocolArtifact) {
122-
const protocolArtifact = await createProtocolArtifact(protocolFilePath, protocolUploadUrl, protocolManager)
120+
const protocolArtifact = await createProtocolArtifact(protocolFilePath, captureUploadUrl, protocolManager)
123121

124122
debug(protocolArtifact)
125123
if (protocolArtifact) {
126124
artifacts.push(protocolArtifact)
127125
}
126+
} else if (protocolCaptureMeta.url && !captureUploadUrl) {
127+
const err = new Error('Invalid or missing Test Replay upload URL')
128+
129+
protocolManager?.addFatalError('protocolUploadUrl', err)
128130
}
129131
} catch (e) {
130132
debug('Error creating protocol artifact: %O', e)
@@ -171,7 +173,9 @@ export const uploadArtifacts = async (options: UploadArtifactOptions) => {
171173
const postArtifactExtractionFatalError = protocolManager?.getFatalError()
172174

173175
if (postArtifactExtractionFatalError) {
174-
if (isProtocolInitializationError(postArtifactExtractionFatalError)) {
176+
if (postArtifactExtractionFatalError.captureMethod === 'protocolUploadUrl') {
177+
errors.warning('CLOUD_PROTOCOL_CANNOT_UPLOAD_ARTIFACT', postArtifactExtractionFatalError.error)
178+
} else if (isProtocolInitializationError(postArtifactExtractionFatalError)) {
175179
errors.warning('CLOUD_PROTOCOL_INITIALIZATION_FAILURE', postArtifactExtractionFatalError.error)
176180
} else {
177181
errors.warning('CLOUD_PROTOCOL_CAPTURE_FAILURE', postArtifactExtractionFatalError.error)

‎packages/types/src/protocol.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export interface AppCaptureProtocolInterface extends AppCaptureProtocolCommon {
4141
beforeSpec ({ workingDirectory, archivePath, dbPath, db }: { workingDirectory: string, archivePath: string, dbPath: string, db: Database }): void
4242
}
4343

44-
export type ProtocolCaptureMethod = keyof AppCaptureProtocolInterface | 'setupProtocol' | 'uploadCaptureArtifact' | 'getCaptureProtocolScript' | 'cdpClient.on' | 'getZippedDb' | 'UNKNOWN' | 'createProtocolArtifact'
44+
export type ProtocolCaptureMethod = keyof AppCaptureProtocolInterface | 'setupProtocol' | 'uploadCaptureArtifact' | 'getCaptureProtocolScript' | 'cdpClient.on' | 'getZippedDb' | 'UNKNOWN' | 'createProtocolArtifact' | 'protocolUploadUrl'
4545

4646
export interface ProtocolError {
4747
args?: any

‎system-tests/__snapshots__/record_spec.js

+83
Original file line numberDiff line numberDiff line change
@@ -4248,3 +4248,86 @@ StatusCodeError: 500 - "Internal Server Error"
42484248
42494249
42504250
`
4251+
4252+
exports['e2e record capture-protocol enabled but missing upload url Does not try to upload the protocol artifact to the capture protocol script url 1'] = `
4253+
4254+
====================================================================================================
4255+
4256+
(Run Starting)
4257+
4258+
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
4259+
│ Cypress: 1.2.3 │
4260+
│ Browser: FooBrowser 88 │
4261+
│ Specs: 1 found (record_pass.cy.js) │
4262+
│ Searched: cypress/e2e/record_pass* │
4263+
│ Params: Tag: false, Group: false, Parallel: false │
4264+
│ Run URL: https://dashboard.cypress.io/projects/cjvoj7/runs/12 │
4265+
└────────────────────────────────────────────────────────────────────────────────────────────────┘
4266+
4267+
4268+
────────────────────────────────────────────────────────────────────────────────────────────────────
4269+
4270+
Running: record_pass.cy.js (1 of 1)
4271+
Estimated: X second(s)
4272+
4273+
4274+
record pass
4275+
✓ passes
4276+
- is pending
4277+
4278+
4279+
1 passing
4280+
1 pending
4281+
4282+
4283+
(Results)
4284+
4285+
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
4286+
│ Tests: 2 │
4287+
│ Passing: 1 │
4288+
│ Failing: 0 │
4289+
│ Pending: 1 │
4290+
│ Skipped: 0 │
4291+
│ Screenshots: 1 │
4292+
│ Video: false │
4293+
│ Duration: X seconds │
4294+
│ Estimated: X second(s) │
4295+
│ Spec Ran: record_pass.cy.js │
4296+
└────────────────────────────────────────────────────────────────────────────────────────────────┘
4297+
4298+
4299+
(Screenshots)
4300+
4301+
- /XXX/XXX/XXX/cypress/screenshots/record_pass.cy.js/yay it passes.png (400x1022)
4302+
4303+
Warning: We are unable to upload the Test Replay recording of this spec due to a missing or invalid upload URL.
4304+
4305+
These results will not display Test Replay recordings.
4306+
4307+
This error will not affect or change the exit code.
4308+
4309+
4310+
(Uploading Cloud Artifacts)
4311+
4312+
- Video - Nothing to upload
4313+
- Screenshot - Nothing to upload
4314+
- Test Replay - Failed Capturing - Invalid or missing Test Replay upload URL
4315+
4316+
====================================================================================================
4317+
4318+
(Run Finished)
4319+
4320+
4321+
Spec Tests Passing Failing Pending Skipped
4322+
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
4323+
│ ✔ record_pass.cy.js XX:XX 2 1 - 1 - │
4324+
└────────────────────────────────────────────────────────────────────────────────────────────────┘
4325+
✔ All specs passed! XX:XX 2 1 - 1 -
4326+
4327+
4328+
───────────────────────────────────────────────────────────────────────────────────────────────────────
4329+
4330+
Recorded Run: https://dashboard.cypress.io/projects/cjvoj7/runs/12
4331+
4332+
4333+
`

‎system-tests/lib/serverStub.ts

+7
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,13 @@ export const routeHandlers: Record<string, RouteHandler> = {
243243
}
244244
},
245245
},
246+
putCaptureScript: {
247+
method: 'put',
248+
url: '/capture-protocol/script/*',
249+
res: async (_, res) => {
250+
res.status(413).send('')
251+
},
252+
},
246253
putCaptureProtocolUpload: {
247254
method: 'put',
248255
url: '/capture-protocol/upload',

‎system-tests/test/record_spec.js

+29
Original file line numberDiff line numberDiff line change
@@ -2584,6 +2584,35 @@ describe('e2e record', () => {
25842584
})
25852585
})
25862586
})
2587+
2588+
describe('capture-protocol enabled but missing upload url', () => {
2589+
enableCaptureProtocol()
2590+
setupStubbedServer(createRoutes({
2591+
postInstanceResults: {
2592+
res: (req, res) => {
2593+
res.status(200).json({
2594+
screenshotUploadUrls: [],
2595+
videoUploadUrl: undefined,
2596+
captureUloadUrl: undefined,
2597+
})
2598+
},
2599+
},
2600+
}))
2601+
2602+
it('Does not try to upload the protocol artifact to the capture protocol script url', function () {
2603+
return systemTests.exec(this, {
2604+
key: 'f858a2bc-b469-4e48-be67-0876339ee7e1',
2605+
configFile: 'cypress-with-project-id.config.js',
2606+
spec: 'record_pass*',
2607+
record: true,
2608+
snapshot: true,
2609+
}).then(() => {
2610+
const requestUrls = getRequestUrls()
2611+
2612+
expect(requestUrls.find((url) => url.includes('PUT /capture-protocol/script/'))).to.be.undefined
2613+
})
2614+
})
2615+
})
25872616
})
25882617

25892618
describe('capture-protocol api errors', () => {

0 commit comments

Comments
 (0)