From 1d845323d4f716b027c17d074b16b9c7ae2a3b57 Mon Sep 17 00:00:00 2001 From: Cacie Prins Date: Wed, 25 Sep 2024 12:39:28 -0400 Subject: [PATCH] feat: Improve UX when Test Replay upload fails due to slow upload (#30235) * new error message for stream stall upload failures * new unknown error msg, extract printProtocolUploadError for testing, consolidatd args for stream stall msg * rename env more appropriately, add sampling interval param to putProtocolArtifact * default to 5000, override with app capture protocol value, override that with env var * update snapshots * fix math in upload stall error msg * changelog * update gql schema w/ new error msg, fix protocol tscheck * update test to use fn for app capture supplied interval * increase default stream stall interval to 10 seconds * typos * snapshots * rename env var * do not use user-supplied interval if it parses to NaN * use the more standard makeErr for unknown/uncategorized upload error in visual snapshots * rearrange upload stall error message * fix typo * changelog --------- Co-authored-by: Jennifer Shehane --- cli/CHANGELOG.md | 6 +- ...OAD_AGGREGATE_ERROR - withSystemError.html | 46 ++++++++ ...LOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE.html} | 0 ..._PROTOCOL_UPLOAD_STREAM_STALL_FAILURE.html | 47 ++++++++ .../CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR.html | 45 ++++++++ packages/errors/src/errors.ts | 30 ++++- .../test/unit/visualSnapshotErrors_spec.ts | 20 +++- packages/graphql/schemas/schema.graphql | 4 +- .../lib/cloud/api/put_protocol_artifact.ts | 9 +- .../artifacts/print_protocol_upload_error.ts | 24 ++++ .../lib/cloud/artifacts/upload_artifacts.ts | 17 +-- packages/server/lib/cloud/protocol.ts | 9 +- .../cloud/upload/stream_activity_monitor.ts | 13 +-- .../lib/cloud/upload/stream_stalled_error.ts | 16 +++ .../cloud/api/put_protocol_artifact_spec.ts | 37 ++++-- .../print_protocol_upload_error_spec.ts | 70 ++++++++++++ .../server/test/unit/cloud/protocol_spec.ts | 106 +++++++++++++++--- .../upload/stream_activity_monitor_spec.ts | 3 +- packages/types/src/protocol.ts | 1 + system-tests/__snapshots__/record_spec.js | 18 +++ 20 files changed, 455 insertions(+), 66 deletions(-) create mode 100644 packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR - withSystemError.html rename packages/errors/__snapshot-html__/{CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE.html => CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE.html} (100%) create mode 100644 packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE.html create mode 100644 packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR.html create mode 100644 packages/server/lib/cloud/artifacts/print_protocol_upload_error.ts create mode 100644 packages/server/lib/cloud/upload/stream_stalled_error.ts create mode 100644 packages/server/test/unit/cloud/artifacts/print_protocol_upload_error_spec.ts diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 3ad3ea313fe6..f5983c34059e 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -1,8 +1,12 @@ -## 13.14.3 +## 13.15.0 _Released 9/24/2024 (PENDING)_ +**Features:** + +- Cypress now displays more actionable errors when a Test Replay upload takes too long, and more verbose messages when uncategorized errors occur during the upload process. Addressed in [#30235](https://github.com/cypress-io/cypress/pull/30235). + **Bugfixes:** - Fixed an issue where Firefox was incorrectly mutating the state of click events on checkboxes after Firefox version `129` and up. Addressed in [#30245](https://github.com/cypress-io/cypress/pull/30245). diff --git a/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR - withSystemError.html b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR - withSystemError.html new file mode 100644 index 000000000000..56e65f16b079 --- /dev/null +++ b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR - withSystemError.html @@ -0,0 +1,46 @@ + + + + + + + + + + + + 
Warning: We encountered multiple errors while uploading the Test Replay recording for this spec.
+
+We attempted to upload the Test Replay recording 3 times.
+
+Some or all of the errors encountered are system-level network errors. Please verify your network configuration for connecting to http://some/url
+
+http://some/url: ECONNRESET
+fail whale
+fail whale
+
\ No newline at end of file diff --git a/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE.html b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE.html similarity index 100% rename from packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE.html rename to packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE.html diff --git a/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE.html b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE.html new file mode 100644 index 000000000000..12d1c1da3408 --- /dev/null +++ b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE.html @@ -0,0 +1,47 @@ + + + + + + + + + + + + 
Warning: We encountered slow network conditions while uploading the Test Replay recording for this spec.
+
+The upload transfer rate fell below 102.4kbps over a sampling period of 5000ms.
+
+To prevent long CI execution durations, this Test Replay recording will not be uploaded.
+
+The results for this spec will not display Test Replay recordings.
+
+If this error occurs often, the sampling period may be configured by setting the CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL environment variable to a higher value than 5000.
+
+
\ No newline at end of file diff --git a/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR.html b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR.html new file mode 100644 index 000000000000..6620d0f83892 --- /dev/null +++ b/packages/errors/__snapshot-html__/CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR.html @@ -0,0 +1,45 @@ + + + + + + + + + + + + 
Warning: We encountered an error while uploading the Test Replay recording of this spec.
+
+These results will not display Test Replay recordings.
+
+This error will not affect or change the exit code.
+
+Error: fail whale
+
+
\ No newline at end of file diff --git a/packages/errors/src/errors.ts b/packages/errors/src/errors.ts index 0311a177fe12..fb5a6121dd9f 100644 --- a/packages/errors/src/errors.ts +++ b/packages/errors/src/errors.ts @@ -574,6 +574,17 @@ export const AllCypressErrors = { This error will not affect or change the exit code. ` }, + CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR: (error: Error) => { + return errTemplate`\ + Warning: We encountered an error while uploading the Test Replay recording of this spec. + + These results will not display Test Replay recordings. + + This error will not affect or change the exit code. + + ${fmt.highlightSecondary(error)} + ` + }, CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE: (error: Error & { url: string, status: number, statusText: string, responseBody: string }) => { return errTemplate`\ Warning: We encountered an HTTP error while uploading the Test Replay recording for this spec. @@ -586,7 +597,7 @@ export const AllCypressErrors = { ${fmt.highlightTertiary(error.responseBody)}` }, - CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE: (error: Error & { url: string }) => { + CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE: (error: Error & { url: string }) => { return errTemplate`\ Warning: We encountered a network error while uploading the Test Replay recording for this spec. @@ -598,6 +609,21 @@ export const AllCypressErrors = { ${fmt.highlightSecondary(error)}` }, + CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE: (error: Error & { chunkSizeKB: number, maxActivityDwellTime: number }) => { + const kbpsThreshold = (error.chunkSizeKB * 8) / (error.maxActivityDwellTime / 1000) + + return errTemplate`\ + Warning: We encountered slow network conditions while uploading the Test Replay recording for this spec. + + The upload transfer rate fell below ${fmt.highlightSecondary(`${kbpsThreshold}kbps`)} over a sampling period of ${fmt.highlightSecondary(`${error.maxActivityDwellTime}ms`)}. + + To prevent long CI execution durations, this Test Replay recording will not be uploaded. + + The results for this spec will not display Test Replay recordings. + + If this error occurs often, the sampling period may be configured by setting the ${fmt.highlightSecondary('CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL')} environment variable to a higher value than ${fmt.stringify(error.maxActivityDwellTime)}. + ` + }, CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR: (error: { errors: (Error & { kind?: 'SystemError', url: string } | Error & { kind: 'HttpError', url: string, status?: string, statusText?: string, responseBody?: string })[] }) => { @@ -605,7 +631,7 @@ export const AllCypressErrors = { const firstError = error.errors[0] if (firstError?.kind === 'SystemError') { - return AllCypressErrors.CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE(firstError as Error & { url: string }) + return AllCypressErrors.CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE(firstError as Error & { url: string }) } return AllCypressErrors.CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE(error.errors[0] as Error & { url: string, status: number, statusText: string, responseBody: string}) diff --git a/packages/errors/test/unit/visualSnapshotErrors_spec.ts b/packages/errors/test/unit/visualSnapshotErrors_spec.ts index f64db6c98397..1ba6621130a5 100644 --- a/packages/errors/test/unit/visualSnapshotErrors_spec.ts +++ b/packages/errors/test/unit/visualSnapshotErrors_spec.ts @@ -685,7 +685,7 @@ describe('visual error templates', () => { default: [err], } }, - CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE: () => { + CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE: () => { // @ts-expect-error const err: Error & { url: string } = makeErr() @@ -695,6 +695,17 @@ describe('visual error templates', () => { default: [err], } }, + CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE: () => { + // @ts-expect-error + const err: Error & { chunkSizeKB: number, maxActivityDwellTime: number } = new Error('stream stall') + + err.chunkSizeKB = 64 + err.maxActivityDwellTime = 5000 + + return { + default: [err], + } + }, CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR: () => { // @ts-expect-error const aggregateError: Error & { errors: any[] } = makeErr() @@ -719,6 +730,13 @@ describe('visual error templates', () => { withSystemError: [aggregateErrorWithSystemError], } }, + CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR: () => { + const error = makeErr() + + return { + default: [error], + } + }, CLOUD_RECORD_KEY_NOT_VALID: () => { return { default: ['record-key-123', 'project-id-123'], diff --git a/packages/graphql/schemas/schema.graphql b/packages/graphql/schemas/schema.graphql index 903804eef0fb..2d31e0e19df1 100644 --- a/packages/graphql/schemas/schema.graphql +++ b/packages/graphql/schemas/schema.graphql @@ -1153,7 +1153,9 @@ enum ErrorTypeEnum { CLOUD_PROTOCOL_INITIALIZATION_FAILURE CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE - CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE + CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE + CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE + CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR CLOUD_RECORD_KEY_NOT_VALID CLOUD_RUN_GROUP_NAME_NOT_UNIQUE CLOUD_STALE_RUN diff --git a/packages/server/lib/cloud/api/put_protocol_artifact.ts b/packages/server/lib/cloud/api/put_protocol_artifact.ts index b252c93005a7..3a596bc12c38 100644 --- a/packages/server/lib/cloud/api/put_protocol_artifact.ts +++ b/packages/server/lib/cloud/api/put_protocol_artifact.ts @@ -7,15 +7,10 @@ import { putFetch, ParseKinds } from '../network/put_fetch' import { isRetryableError } from '../network/is_retryable_error' const debug = Debug('cypress:server:cloud:api:protocol-artifact') -// the upload will get canceled if the stream pipeline -// stalls (does not push data to the `fetch` sink) for more -// than 5 seconds -const MAX_ACTIVITY_DWELL_TIME = 5000 - export const _delay = linearDelay(500) export const putProtocolArtifact = asyncRetry( - async (artifactPath: string, maxFileSize: number, destinationUrl: string) => { + async (artifactPath: string, maxFileSize: number, destinationUrl: string, uploadMonitorSamplingRate: number) => { debug(`Atttempting to upload Test Replay archive from ${artifactPath} to ${destinationUrl})`) const { size } = await fsAsync.stat(artifactPath) @@ -23,7 +18,7 @@ export const putProtocolArtifact = asyncRetry( throw new Error(`Spec recording too large: artifact is ${size} bytes, limit is ${maxFileSize} bytes`) } - const activityMonitor = new StreamActivityMonitor(MAX_ACTIVITY_DWELL_TIME) + const activityMonitor = new StreamActivityMonitor(uploadMonitorSamplingRate) const fileStream = fs.createReadStream(artifactPath) const controller = activityMonitor.getController() diff --git a/packages/server/lib/cloud/artifacts/print_protocol_upload_error.ts b/packages/server/lib/cloud/artifacts/print_protocol_upload_error.ts new file mode 100644 index 000000000000..7a94a038429a --- /dev/null +++ b/packages/server/lib/cloud/artifacts/print_protocol_upload_error.ts @@ -0,0 +1,24 @@ +import { HttpError } from '../network/http_error' +import { SystemError } from '../network/system_error' +import { StreamStalledError } from '../upload/stream_stalled_error' +import Debug from 'debug' +import * as errors from '../../errors' + +const debug = Debug('cypress:server:cloud:artifacts') + +export const printProtocolUploadError = (error: Error) => { + debug('protocol error: %O', error) + // eslint-disable-next-line no-console + console.log('') + if ((error as AggregateError).errors) { + errors.warning('CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR', error as AggregateError) + } else if (HttpError.isHttpError(error)) { + errors.warning('CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE', error) + } else if (SystemError.isSystemError(error)) { + errors.warning('CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE', error) + } else if (StreamStalledError.isStreamStalledError(error)) { + errors.warning('CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE', error) + } else { + errors.warning('CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR', error) + } +} diff --git a/packages/server/lib/cloud/artifacts/upload_artifacts.ts b/packages/server/lib/cloud/artifacts/upload_artifacts.ts index 800a60b68de1..d41ef303225b 100644 --- a/packages/server/lib/cloud/artifacts/upload_artifacts.ts +++ b/packages/server/lib/cloud/artifacts/upload_artifacts.ts @@ -12,7 +12,7 @@ import { createScreenshotArtifactBatch } from './screenshot_artifact' import { createVideoArtifact } from './video_artifact' import { createProtocolArtifact, composeProtocolErrorReportFromOptions } from './protocol_artifact' import { HttpError } from '../network/http_error' -import { SystemError } from '../network/system_error' +import { printProtocolUploadError } from './print_protocol_upload_error' const debug = Debug('cypress:server:cloud:artifacts') @@ -230,20 +230,7 @@ export const uploadArtifacts = async (options: UploadArtifactOptions) => { if (postUploadProtocolFatalError && postUploadProtocolFatalError.captureMethod === 'uploadCaptureArtifact') { const error = postUploadProtocolFatalError.error - debug('protocol error: %O', error) - if ((error as AggregateError).errors) { - // eslint-disable-next-line no-console - console.log('') - errors.warning('CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR', postUploadProtocolFatalError.error as AggregateError) - } else if (HttpError.isHttpError(error)) { - // eslint-disable-next-line no-console - console.log('') - errors.warning('CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE', error) - } else if (SystemError.isSystemError(error)) { - // eslint-disable-next-line no-console - console.log('') - errors.warning('CLOUD_PROTOCOL_UPLOAD_NEWORK_FAILURE', error) - } + printProtocolUploadError(error) } // there is no upload results entry for protocol if we did not attempt to upload protocol due to a fatal error diff --git a/packages/server/lib/cloud/protocol.ts b/packages/server/lib/cloud/protocol.ts index 16ae3966e3a8..d017f483e7c1 100644 --- a/packages/server/lib/cloud/protocol.ts +++ b/packages/server/lib/cloud/protocol.ts @@ -26,6 +26,8 @@ const DELETE_DB = !process.env.CYPRESS_LOCAL_PROTOCOL_PATH export const DB_SIZE_LIMIT = 5000000000 +export const DEFAULT_STREAM_SAMPLING_INTERVAL = 10000 + const dbSizeLimit = () => { return env.get('CYPRESS_INTERNAL_SYSTEM_TESTS') === '1' ? 200 : DB_SIZE_LIMIT @@ -320,7 +322,12 @@ export class ProtocolManager implements ProtocolManagerShape { debug(`uploading %s to %s with a file size of %s`, filePath, uploadUrl, fileSize) try { - await putProtocolArtifact(filePath, dbSizeLimit(), uploadUrl) + const environmentSuppliedInterval = parseInt(process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL || '', 10) + const samplingInterval = !Number.isNaN(environmentSuppliedInterval) ? + environmentSuppliedInterval : + this._protocol.uploadStallSamplingInterval ? this._protocol.uploadStallSamplingInterval() : DEFAULT_STREAM_SAMPLING_INTERVAL + + await putProtocolArtifact(filePath, dbSizeLimit(), uploadUrl, samplingInterval) return { fileSize, diff --git a/packages/server/lib/cloud/upload/stream_activity_monitor.ts b/packages/server/lib/cloud/upload/stream_activity_monitor.ts index ba7686e34ee3..e1d6a6b2e869 100644 --- a/packages/server/lib/cloud/upload/stream_activity_monitor.ts +++ b/packages/server/lib/cloud/upload/stream_activity_monitor.ts @@ -1,15 +1,9 @@ import Debug from 'debug' import { Transform, Readable } from 'stream' - +import { StreamStalledError } from './stream_stalled_error' const debug = Debug('cypress:server:cloud:stream-activity-monitor') const debugVerbose = Debug('cypress-verbose:server:cloud:stream-activity-monitor') -export class StreamStalledError extends Error { - constructor (maxActivityDwellTime: number) { - super(`Stream stalled: no activity detected in the previous ${maxActivityDwellTime}ms`) - } -} - /** * `StreamActivityMonitor` encapsulates state with regard to monitoring a stream for flow * failure states. Given a maxActivityDwellTime, this class can `monitor` a Node Readable @@ -36,6 +30,9 @@ export class StreamStalledError extends Error { * } * */ + +const DEFAULT_FS_READSTREAM_CHUNK_SIZE = 64 * 1024 // Kilobytes + export class StreamActivityMonitor { private streamMonitor: Transform | undefined private activityTimeout: NodeJS.Timeout | undefined @@ -81,7 +78,7 @@ export class StreamActivityMonitor { debug('marking activity interval') clearTimeout(this.activityTimeout) this.activityTimeout = setTimeout(() => { - this.controller?.abort(new StreamStalledError(this.maxActivityDwellTime)) + this.controller?.abort(new StreamStalledError(this.maxActivityDwellTime, DEFAULT_FS_READSTREAM_CHUNK_SIZE)) }, this.maxActivityDwellTime) } } diff --git a/packages/server/lib/cloud/upload/stream_stalled_error.ts b/packages/server/lib/cloud/upload/stream_stalled_error.ts new file mode 100644 index 000000000000..04b8c76558a0 --- /dev/null +++ b/packages/server/lib/cloud/upload/stream_stalled_error.ts @@ -0,0 +1,16 @@ +export const StreamStalledErrorKind = 'StreamStalled' + +export class StreamStalledError extends Error { + public readonly kind = StreamStalledErrorKind + + constructor ( + public readonly maxActivityDwellTime: number, + public readonly chunkSizeKB: number, + ) { + super(`Stream stalled: failed to transfer ${chunkSizeKB} kilobytes over the previous ${maxActivityDwellTime}ms`) + } + + public static isStreamStalledError (error: Error & {kind?: any}): error is StreamStalledError { + return error.kind === StreamStalledErrorKind + } +} diff --git a/packages/server/test/unit/cloud/api/put_protocol_artifact_spec.ts b/packages/server/test/unit/cloud/api/put_protocol_artifact_spec.ts index 5948cc74f3a9..97e250082002 100644 --- a/packages/server/test/unit/cloud/api/put_protocol_artifact_spec.ts +++ b/packages/server/test/unit/cloud/api/put_protocol_artifact_spec.ts @@ -11,6 +11,7 @@ import { HttpError } from '../../../../lib/cloud/network/http_error' import { putFetch, ParseKinds } from '../../../../lib/cloud/network/put_fetch' import { linearDelay, asyncRetry } from '../../../../lib/util/async_retry' import { isRetryableError } from '../../../../lib/cloud/network/is_retryable_error' +import type { putProtocolArtifact } from '../../../../lib/cloud/api/put_protocol_artifact' chai.use(chaiAsPromised).use(sinonChai) @@ -18,13 +19,17 @@ describe('putProtocolArtifact', () => { let filePath: string let maxFileSize: number let fileSize: number + let uploadMonitorSamplingRate: number let mockStreamMonitor: sinon.SinonStubbedInstance let mockReadStream let destinationUrl let statStub: sinon.SinonStub let asyncRetryStub: sinon.SinonStub, ReturnType> let putFetchStub: sinon.SinonStub, ReturnType> - let putProtocolArtifact: (artifactPath: string, maxFileSize: number, destinationUrl: string) => Promise + let putArtifact: typeof putProtocolArtifact + let streamMonitorImport: { + StreamActivityMonitor: sinon.SinonStub<[maxActivityDwellTime: number], StreamActivityMonitor> + } /** * global.mockery is defined the first time `test/spec_helper.js` is required by any spec. @@ -63,6 +68,7 @@ describe('putProtocolArtifact', () => { filePath = '/some/file/path' fileSize = 20 destinationUrl = 'https://some/destination/url' + uploadMonitorSamplingRate = 10000 mockReadStream = sinon.createStubInstance(ReadStream) mockery.registerMock('fs', { @@ -91,13 +97,16 @@ describe('putProtocolArtifact', () => { sinon.stub(mockAbortController, 'signal').get(() => mockSignal) - mockStreamMonitor = sinon.createStubInstance(StreamActivityMonitor) - mockStreamMonitor.getController.returns(mockAbortController) - mockery.registerMock('../upload/stream_activity_monitor', { - StreamActivityMonitor: sinon.stub().callsFake(() => { + streamMonitorImport = { + StreamActivityMonitor: sinon.stub<[maxActivityDwellTime: number], StreamActivityMonitor>().callsFake(() => { return mockStreamMonitor }), - }) + } + + mockStreamMonitor = sinon.createStubInstance(StreamActivityMonitor) + mockStreamMonitor.getController.returns(mockAbortController) + + mockery.registerMock('../upload/stream_activity_monitor', streamMonitorImport) statStub = sinon.stub() @@ -107,7 +116,7 @@ describe('putProtocolArtifact', () => { const req = require('../../../../lib/cloud/api/put_protocol_artifact') - putProtocolArtifact = req.putProtocolArtifact + putArtifact = req.putProtocolArtifact }) afterEach(() => { @@ -149,7 +158,7 @@ describe('putProtocolArtifact', () => { }) it('rejects with a file does not exist error', () => { - return expect(putProtocolArtifact(invalidPath, maxFileSize, destinationUrl)).to.be.rejectedWith(`ENOENT: no such file or directory, stat '/some/invalid/path'`) + return expect(putArtifact(invalidPath, maxFileSize, destinationUrl, uploadMonitorSamplingRate)).to.be.rejectedWith(`ENOENT: no such file or directory, stat '/some/invalid/path'`) }) }) @@ -164,16 +173,20 @@ describe('putProtocolArtifact', () => { }) it('rejects with a file too large error', () => { - return expect(putProtocolArtifact(filePath, maxFileSize, destinationUrl)) + return expect(putArtifact(filePath, maxFileSize, destinationUrl, uploadMonitorSamplingRate)) .to.be.rejectedWith('Spec recording too large: artifact is 20 bytes, limit is 19 bytes') }) }) describe('and fetch completes successfully', () => { - it('resolves', async () => { + beforeEach(() => { putFetchStub.resolves() + }) + + it('creates the stream activity monitor with the provided sampling interval and resolves', async () => { + await expect(putArtifact(filePath, maxFileSize, destinationUrl, uploadMonitorSamplingRate)).to.be.fulfilled - expect(putProtocolArtifact(filePath, maxFileSize, destinationUrl)).to.be.fulfilled + expect(streamMonitorImport.StreamActivityMonitor).to.have.been.calledWith(uploadMonitorSamplingRate) }) }) @@ -200,7 +213,7 @@ describe('putProtocolArtifact', () => { let error: Error | undefined try { - await putProtocolArtifact(filePath, maxFileSize, destinationUrl) + await putArtifact(filePath, maxFileSize, destinationUrl) } catch (e) { error = e } diff --git a/packages/server/test/unit/cloud/artifacts/print_protocol_upload_error_spec.ts b/packages/server/test/unit/cloud/artifacts/print_protocol_upload_error_spec.ts new file mode 100644 index 000000000000..3747060ee29f --- /dev/null +++ b/packages/server/test/unit/cloud/artifacts/print_protocol_upload_error_spec.ts @@ -0,0 +1,70 @@ +import { proxyquire, expect } from '../../../spec_helper' +import sinon from 'sinon' +import { HttpError } from '../../../../lib/cloud/network/http_error' +import { SystemError } from '../../../../lib/cloud/network/system_error' +import { StreamStalledError } from '../../../../lib/cloud/upload/stream_stalled_error' +import type { warning } from '../../../../lib/errors' +import type { printProtocolUploadError } from '../../../../lib/cloud/artifacts/print_protocol_upload_error' + +describe('printProtocolUploadError', () => { + let stubbedErrorWarning: sinon.SinonStub, ReturnType> + + let print: typeof printProtocolUploadError + + beforeEach(() => { + stubbedErrorWarning = sinon.stub, ReturnType>() + const importPrintProtocolUploadError = proxyquire('../lib/cloud/artifacts/print_protocol_upload_error', { + '../../errors': { + warning: stubbedErrorWarning, + }, + }) + + print = importPrintProtocolUploadError.printProtocolUploadError + }) + + describe('when passed an aggregate error', () => { + it('prints a CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR message', () => { + const error: Error & { errors?: Error[] } = new Error('message') + + error.errors = [] + print(error) + expect(stubbedErrorWarning).to.have.been.calledWith('CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR', error) + }) + }) + + describe('when passed an http error', () => { + it('prints a CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE', () => { + const error = new HttpError('Service Unavailable', 'http://some.url', 503, 'Service Unavailable', '', {} as Response) + + print(error) + expect(stubbedErrorWarning).to.have.been.calledWith('CLOUD_PROTOCOL_UPLOAD_HTTP_FAILURE', error) + }) + }) + + describe('when passed a system error', () => { + it('prints a CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE warning', () => { + const err = new SystemError(new Error('msg'), 'http://some.url') + + print(err) + expect(stubbedErrorWarning).to.have.been.calledWith('CLOUD_PROTOCOL_UPLOAD_NETWORK_FAILURE', err) + }) + }) + + describe('when passed a stream stalled error', () => { + it('prints a CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE warning', () => { + const err = new StreamStalledError(5000, 64 * 1024) + + print(err) + expect(stubbedErrorWarning).to.have.been.calledWith('CLOUD_PROTOCOL_UPLOAD_STREAM_STALL_FAILURE', err) + }) + }) + + describe('when passed some other kind of error', () => { + it('prints a CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR warning', () => { + const err = new Error('message') + + print(err) + expect(stubbedErrorWarning).to.have.been.calledWith('CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR', err) + }) + }) +}) diff --git a/packages/server/test/unit/cloud/protocol_spec.ts b/packages/server/test/unit/cloud/protocol_spec.ts index cdc52155f8da..559cf067d7fb 100644 --- a/packages/server/test/unit/cloud/protocol_spec.ts +++ b/packages/server/test/unit/cloud/protocol_spec.ts @@ -16,7 +16,7 @@ const mockDb = sinon.stub() const mockDatabase = sinon.stub().returns(mockDb) const mockPutProtocolArtifact = sinon.stub() -const { ProtocolManager, DB_SIZE_LIMIT } = proxyquire('../lib/cloud/protocol', { +const { ProtocolManager, DB_SIZE_LIMIT, DEFAULT_STREAM_SAMPLING_INTERVAL } = proxyquire('../lib/cloud/protocol', { 'better-sqlite3': mockDatabase, './api/put_protocol_artifact': { putProtocolArtifact: mockPutProtocolArtifact }, }) as typeof import('@packages/server/lib/cloud/protocol') @@ -347,30 +347,102 @@ describe('lib/cloud/protocol', () => { }) describe('when upload succeeds', () => { + let defaultInterval + beforeEach(() => { - mockPutProtocolArtifact.withArgs(filePath, DB_SIZE_LIMIT, uploadUrl).resolves() + defaultInterval = DEFAULT_STREAM_SAMPLING_INTERVAL + }) + + describe('with default sampling rate', () => { + beforeEach(() => { + mockPutProtocolArtifact.withArgs(filePath, DB_SIZE_LIMIT, uploadUrl, defaultInterval).resolves() + }) + + it('uses 5000ms as the default stream monitoring sample rate', async () => { + await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + + expect(mockPutProtocolArtifact).to.have.been.calledWith(filePath, DB_SIZE_LIMIT, uploadUrl, defaultInterval) + }) + + it('unlinks the db & returns fileSize, afterSpec durations, success=true, and the db metadata', async () => { + const res = await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + + expect(res).not.to.be.undefined + expect(res).to.include({ + fileSize, + success: true, + }) + + expect(res?.afterSpecDurations).to.include({ + afterSpecTotal: expectedAfterSpecTotal, + ...expectedAfterSpecDurations.durations, + }) + + // @ts-ignore + expect(res?.specAccess.offset).to.eq(offset) + // @ts-ignore + expect(res?.specAccess.size).to.eq(size) + + expect(fs.unlink).to.have.been.called + }) }) - it('unlinks the db & returns fileSize, afterSpec durations, success=true, and the db metadata', async () => { - const res = await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + describe('when protocol exports a sampling rate', () => { + let appCaptureProtocolInterval - expect(res).not.to.be.undefined - expect(res).to.include({ - fileSize, - success: true, + beforeEach(() => { + appCaptureProtocolInterval = 7500 + + protocol.uploadStallSamplingInterval = sinon.stub().callsFake(() => { + return appCaptureProtocolInterval + }) + }) + + afterEach(() => { + // @ts-ignore + protocol.uploadStallSamplingInterval = undefined + }) + + it('uses the sampling rate defined by protocol', async () => { + mockPutProtocolArtifact.withArgs(filePath, DB_SIZE_LIMIT, uploadUrl, appCaptureProtocolInterval).resolves() + await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + expect(mockPutProtocolArtifact).to.have.been.calledWith(filePath, DB_SIZE_LIMIT, uploadUrl, appCaptureProtocolInterval) }) - expect(res?.afterSpecDurations).to.include({ - afterSpecTotal: expectedAfterSpecTotal, - ...expectedAfterSpecDurations.durations, + describe('and the user specifies a sampling rate env var', () => { + let userDefinedInterval + + beforeEach(() => { + userDefinedInterval = 10000 + process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL = '10000' + }) + + afterEach(() => { + process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL = undefined + }) + + it('uses the override value from the env var', async () => { + mockPutProtocolArtifact.withArgs(filePath, DB_SIZE_LIMIT, uploadUrl, userDefinedInterval).resolves() + await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + expect(mockPutProtocolArtifact).to.have.been.calledWith(filePath, DB_SIZE_LIMIT, uploadUrl, userDefinedInterval) + }) }) - // @ts-ignore - expect(res?.specAccess.offset).to.eq(offset) - // @ts-ignore - expect(res?.specAccess.size).to.eq(size) + describe('and the user specifies a sampling rate env var that parses to NaN', () => { + beforeEach(() => { + process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL = 'not-a-number' + }) - expect(fs.unlink).to.have.been.called + afterEach(() => { + process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL = undefined + }) + + it('uses the value from app capture protocol', async () => { + mockPutProtocolArtifact.withArgs(filePath, DB_SIZE_LIMIT, uploadUrl, appCaptureProtocolInterval).resolves() + await protocolManager.uploadCaptureArtifact({ uploadUrl, filePath, fileSize }) + expect(mockPutProtocolArtifact).to.have.been.calledWith(filePath, DB_SIZE_LIMIT, uploadUrl, appCaptureProtocolInterval) + }) + }) }) }) @@ -380,7 +452,7 @@ describe('lib/cloud/protocol', () => { beforeEach(() => { err = new Error() - ;(mockPutProtocolArtifact as SinonStub).withArgs(filePath, DB_SIZE_LIMIT, uploadUrl).rejects(err) + ;(mockPutProtocolArtifact as SinonStub).withArgs(filePath, DB_SIZE_LIMIT, uploadUrl, DEFAULT_STREAM_SAMPLING_INTERVAL).rejects(err) }) describe('and there is no local protocol path in env', () => { diff --git a/packages/server/test/unit/cloud/upload/stream_activity_monitor_spec.ts b/packages/server/test/unit/cloud/upload/stream_activity_monitor_spec.ts index 42b16a1b6c83..46eeffd983ff 100644 --- a/packages/server/test/unit/cloud/upload/stream_activity_monitor_spec.ts +++ b/packages/server/test/unit/cloud/upload/stream_activity_monitor_spec.ts @@ -1,6 +1,7 @@ const { sinon, expect } = require('../../../spec_helper') -import { StreamActivityMonitor, StreamStalledError } from '../../../../lib/cloud/upload/stream_activity_monitor' +import { StreamActivityMonitor } from '../../../../lib/cloud/upload/stream_activity_monitor' +import { StreamStalledError } from '../../../../lib/cloud/upload/stream_stalled_error' import { Readable, Writable } from 'stream' describe('StreamTimeoutController', () => { diff --git a/packages/types/src/protocol.ts b/packages/types/src/protocol.ts index e55c55154812..ac81cb2e35e6 100644 --- a/packages/types/src/protocol.ts +++ b/packages/types/src/protocol.ts @@ -39,6 +39,7 @@ export interface AppCaptureProtocolCommon { export interface AppCaptureProtocolInterface extends AppCaptureProtocolCommon { getDbMetadata (): { offset: number, size: number } | undefined beforeSpec ({ workingDirectory, archivePath, dbPath, db }: { workingDirectory: string, archivePath: string, dbPath: string, db: Database }): void + uploadStallSamplingInterval: () => number } export type ProtocolCaptureMethod = keyof AppCaptureProtocolInterface | 'setupProtocol' | 'uploadCaptureArtifact' | 'getCaptureProtocolScript' | 'cdpClient.on' | 'getZippedDb' | 'UNKNOWN' | 'createProtocolArtifact' | 'protocolUploadUrl' diff --git a/system-tests/__snapshots__/record_spec.js b/system-tests/__snapshots__/record_spec.js index a2faf17f89c8..30e8f40fdc3c 100644 --- a/system-tests/__snapshots__/record_spec.js +++ b/system-tests/__snapshots__/record_spec.js @@ -3049,6 +3049,15 @@ exports['e2e record capture-protocol enabled protocol runtime errors db size too - 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 - Spec recording too large: artifact is 1024 bytes, limit is 200 bytes +Warning: We encountered an error while uploading the Test Replay recording of this spec. + +These results will not display Test Replay recordings. + +This error will not affect or change the exit code. + +Error: Spec recording too large: artifact is 1024 bytes, limit is 200 bytes + + ==================================================================================================== (Run Finished) @@ -3985,6 +3994,15 @@ Error: File not found: /os/tmpdir/cypress/protocol/e9e81b5e-cc58-4026-b2ff-8ae31 - 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 +Warning: We encountered an error while uploading the Test Replay recording of this spec. + +These results will not display Test Replay recordings. + +This error will not affect or change the exit code. + +Error: File not found: /os/tmpdir/cypress/protocol/e9e81b5e-cc58-4026-b2ff-8ae3161435a6.tar + + ==================================================================================================== (Run Finished)