-
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 all 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 |
---|---|---|
|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,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_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL ? | ||
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. What is a user passes 'foo' to this env var, it'd evaluate to NaN, then what? |
||
parseInt(process.env.CYPRESS_TEST_REPLAY_UPLOAD_SAMPLING_INTERVAL, 10) : | ||
this._protocol.uploadStallSamplingInterval ? this._protocol.uploadStallSamplingInterval() : DEFAULT_STREAM_SAMPLING_INTERVAL | ||
|
||
await putProtocolArtifact(filePath, dbSizeLimit(), uploadUrl, samplingInterval) | ||
|
||
return { | ||
fileSize, | ||
|
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.
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.
@cacieprins This still needs addressing.