-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
base: develop
Are you sure you want to change the base?
Changes from 14 commits
fa8f03f
c494768
0e519cf
e27fe72
a6529e1
819ab5f
c41d9ed
23f1684
9807903
397af46
7c76042
331512f
334a035
e6632ee
e1fafcc
b93916f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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,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)}. | ||||||
|
||||||
To prevent long CI executing durations, this Test Replay recording will not be uploaded. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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}) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = new Error('some message') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'], | ||
|
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) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
There was a problem hiding this comment.
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.