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

Merged
merged 23 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fa8f03f
new error message for stream stall upload failures
cacieprins Sep 11, 2024
c494768
new unknown error msg, extract printProtocolUploadError for testing, …
cacieprins Sep 11, 2024
0e519cf
rename env more appropriately, add sampling interval param to putProt…
cacieprins Sep 12, 2024
e27fe72
default to 5000, override with app capture protocol value, override t…
cacieprins Sep 12, 2024
a6529e1
update snapshots
cacieprins Sep 12, 2024
819ab5f
fix math in upload stall error msg
cacieprins Sep 12, 2024
c41d9ed
changelog
cacieprins Sep 12, 2024
23f1684
Merge branch 'develop' into cacie/9143/stream-stall-enhancements
cacieprins Sep 12, 2024
9807903
update gql schema w/ new error msg, fix protocol tscheck
cacieprins Sep 12, 2024
397af46
update test to use fn for app capture supplied interval
cacieprins Sep 12, 2024
7c76042
increase default stream stall interval to 10 seconds
cacieprins Sep 12, 2024
331512f
typos
cacieprins Sep 13, 2024
334a035
snapshots
cacieprins Sep 13, 2024
e6632ee
Merge branch 'develop' into cacie/9143/stream-stall-enhancements
cacieprins Sep 13, 2024
e1fafcc
rename env var
cacieprins Sep 17, 2024
b93916f
Merge branch 'develop' into cacie/9143/stream-stall-enhancements
jennifer-shehane Sep 18, 2024
dda7577
do not use user-supplied interval if it parses to NaN
cacieprins Sep 23, 2024
d2cfbb3
use the more standard makeErr for unknown/uncategorized upload error …
cacieprins Sep 23, 2024
8d6dccf
rearrange upload stall error message
cacieprins Sep 23, 2024
397448d
fix typo
cacieprins Sep 23, 2024
366495e
Merge branch 'develop' into cacie/9143/stream-stall-enhancements
cacieprins Sep 23, 2024
f3047c9
Merge branch 'develop' into cacie/9143/stream-stall-enhancements
cacieprins Sep 25, 2024
cf342cf
changelog
cacieprins Sep 25, 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
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ _Released 9/24/2024 (PENDING)_

- Pass along the related log to the `createSnapshot` function for protocol usage. Addressed in [#30244](https://github.com/cypress-io/cypress/pull/30244).

**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`)}.

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 })[]
}) => {
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 = makeErr()

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)
jennifer-shehane marked this conversation as resolved.
Show resolved Hide resolved
}

// there is no upload results entry for protocol if we did not attempt to upload protocol due to a fatal error
Expand Down
9 changes: 8 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,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,
Expand Down
Loading
Loading