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.

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

feat: Improve UX when Test Replay upload fails due to slow upload #30235

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 commits
Commits
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
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

_Released 9/10/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).

**Dependency Updates:**

- Update `@cypress/request` from `3.0.1` to `3.0.4`. Addressed in [#30194](https://github.com/cypress-io/cypress/pull/30194).
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 28 additions & 2 deletions packages/errors/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.

Expand All @@ -598,14 +609,29 @@ 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`)}.

If this error occurs often, the sampling period may be configured by setting the ${fmt.highlightSecondary('CYPRESS_PROTOCOL_UPLOAD_SAMPLING_INTERVAL')} environment variable to a higher value than ${fmt.stringify(error.maxActivityDwellTime)}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like this might flow better if we rearranged the paragraphs.

Warning...

The upload...

To prevent...

The results for this...

If this error occurs...


To prevent long CI executing durations, this Test Replay recording will not be uploaded.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To prevent long CI executing durations, this Test Replay recording will not be uploaded.
To prevent long CI execution durations, this Test Replay recording will not be uploaded.

Copy link
Member

Choose a reason for hiding this comment

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

@cacieprins This still needs addressing.


The results for this spec will not display Test Replay recordings.
`
},
CLOUD_PROTOCOL_UPLOAD_AGGREGATE_ERROR: (error: {
errors: (Error & { kind?: 'SystemError', url: string } | Error & { kind: 'HttpError', url: string, status?: string, statusText?: string, responseBody?: string })[]
}) => {
if (error.errors.length === 1) {
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})
Expand Down
20 changes: 19 additions & 1 deletion packages/errors/test/unit/visualSnapshotErrors_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()
Expand All @@ -719,6 +730,13 @@ describe('visual error templates', () => {
withSystemError: [aggregateErrorWithSystemError],
}
},
CLOUD_PROTOCOL_UPLOAD_UNKNOWN_ERROR: () => {
const error = new Error('some message')
Copy link
Member

Choose a reason for hiding this comment

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

Will this be updated to be a more specific error message?


return {
default: [error],
}
},
CLOUD_RECORD_KEY_NOT_VALID: () => {
return {
default: ['record-key-123', 'project-id-123'],
Expand Down
4 changes: 3 additions & 1 deletion packages/graphql/schemas/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 2 additions & 7 deletions packages/server/lib/cloud/api/put_protocol_artifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,18 @@ 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)

if (size > maxFileSize) {
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()

Expand Down
24 changes: 24 additions & 0 deletions packages/server/lib/cloud/artifacts/print_protocol_upload_error.ts
Original file line number Diff line number Diff line change
@@ -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)
}
}
17 changes: 2 additions & 15 deletions packages/server/lib/cloud/artifacts/upload_artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

nice

}

// there is no upload results entry for protocol if we did not attempt to upload protocol due to a fatal error
Expand Down
8 changes: 7 additions & 1 deletion packages/server/lib/cloud/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -320,7 +322,11 @@ 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 samplingInterval = process.env.CYPRESS_PROTOCOL_UPLOAD_SAMPLING_INTERVAL ?
parseInt(process.env.CYPRESS_PROTOCOL_UPLOAD_SAMPLING_INTERVAL, 10) :
this._protocol.uploadStallSamplingInterval ? this._protocol.uploadStallSamplingInterval() : DEFAULT_STREAM_SAMPLING_INTERVAL

await putProtocolArtifact(filePath, dbSizeLimit(), uploadUrl, samplingInterval)

return {
fileSize,
Expand Down
Loading
Loading