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: more informative error messages when Test Replay fails to capture #29312

Merged
merged 46 commits into from
May 1, 2024

Conversation

cacieprins
Copy link
Contributor

@cacieprins cacieprins commented Apr 11, 2024

Additional details

Many of the errors that can crop up when initializing, capturing, or uploading Test Replay recordings were fairly cryptic. This PR attempts to improve that, with differentiated error messages for:

  • Test Replay recordings failing to initialize or capture
  • Test Replay recordings failing to upload due to system-level network errors
  • Test Replay recordings failing to upload due to HTTP errors returned from the archive upload endpoint

In addition, Test Replay will cease to display in the Artifact Upload manifest if it fails to initialize or if it encounters a fatal error during recording. We print an error message before the upload section of stdout in this case.

Several of these more verbose error messages include paths and URLs that will more readily identify issues and lead to faster and easier resolution. In certain situations, we can provide actionable advice, like: confirming network configuration, increasing available disk space, and ensuring that temporary directories are accessible.

Steps to test

  • Run record_spec system tests
  • Simulate network failures specific to s3 while running a spec that records to Test Replay
  • Simulate HTTP errors specific to s3 while running a spec that records to Test Replay

How has the user experience changed?

The existing upload report will still be displayed, however, the following error messages will also be displayed:

When a request to our API to confirm the status of artifact (video, screenshots, and Test Replay) fails

Warning: We encountered an error while confirming the upload of artifacts for this spec.

These results will not display artifacts.

This error will not affect or change the exit code.

StatusCodeError: 500 - "Internal Server Error"

When Test Replay fails to initialize

Warning: we encountered an error while initializing the Test Replay recording for this spec.
        
These results will not display Test Replay recordings.
        
This error will not affect or change the exit code.

(Error message and stack trace)

When Test Replay encounters a fatal error during recording

Warning: We encountered an error while recording Test Replay data for this spec.
        
These results will not display Test Replay recordings.

This can happen for many reasons. If this problem persists:

- Try increasing the available disk space.
- Ensure that /os/temporary-directory/cypress/protocol s both readable and writable.

This error will not affect or change the exit code.

(Error message and stack trace)

When S3 returns an HTTP status code that indicates an error

Warning: We encountered an HTTP error while uploading the Test Replay recording for this spec.

These results will not display Test Replay recordings.

This error will not affect or change the exit code.

[URL] responded with HTTP 500: Internal Server Error

When the Test Replay recording fails to upload due to a system-level network error

Warning: We encountered a network error while uploading the Test Replay recording for this spec.

Please verify your network configuration for accessing https://some/url

These results will not display Test Replay recordings.

This error will not affect or change the exit code.

Error: request to http://fake.test/url failed, reason: getaddrinfo ENOTFOUND fake.test

When the Test Replay recording fails to upload after several retries

An aggregate error is displayed with each error that was encountered while uploading. We display a recommendation to verify network configuration when we detect that at least one of these errors is a system-level network error.

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

In addition to the stack trace provided, 2 errors were encountered. Stack traces for these errors are omitted for brevity:
 - (error message)
 - (error message)

Error: http://some/url: ECONNRESET
    [STACK]

PR Tasks

@cacieprins cacieprins changed the base branch from develop to cacie/refactor/artifact-upload-to-ts April 11, 2024 17:21
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

I like the error message text that will be displayed. I wish they could be a little more actionable, but I don't quite see how they can be.

If we had any expectations of wanting to change the actions we recommend to people in this situations in the future, I would recommend putting an on.cypress.io link to our error-messages page so that we could more easily update the actions. But I'm not sure this is the case with these situations.

@cacieprins cacieprins changed the title feat: more verbose error messages when Test Replay fails to capture feat: more informative error messages when Test Replay fails to capture Apr 11, 2024
Base automatically changed from cacie/refactor/artifact-upload-to-ts to develop April 12, 2024 13:20
@cacieprins cacieprins force-pushed the cacie/feat/improved-upload-error-msgs branch 2 times, most recently from 7ce569e to 6771fdb Compare April 12, 2024 17:18
@cacieprins cacieprins force-pushed the cacie/feat/improved-upload-error-msgs branch from 6771fdb to e5f3cbd Compare April 12, 2024 18:03
@cacieprins cacieprins marked this pull request as ready for review April 15, 2024 14:39
@cacieprins cacieprins marked this pull request as draft April 15, 2024 14:56
@cacieprins cacieprins marked this pull request as ready for review April 15, 2024 16:32
Copy link

cypress bot commented Apr 18, 2024

4 flaky tests on run #55272 ↗︎

0 29185 1326 0 Flakiness 4

Details:

rm stack traces from error messages
Project: cypress Commit: 3e610748c9
Status: Passed Duration: 19:29 💡
Started: May 1, 2024 5:04 PM Ended: May 1, 2024 5:24 PM
Flakiness  scaffold-component-testing.cy.ts • 1 flaky test • launchpad-e2e

View Output

Test Artifacts
scaffolding component testing > create-react-app > scaffolds component testing for Create React App v5 project Test Replay Screenshots
Flakiness  e2e/origin/config_env.cy.ts • 1 flaky test • 5x-driver-chrome:beta

View Output

Test Artifacts
cy.origin- Cypress.config() > serializable > overwrites different values in secondary if one exists in the primary Test Replay
Flakiness  commands/net_stubbing.cy.ts • 2 flaky tests • 5x-driver-webkit

View Output

Test Artifacts
... > with `resourceType` > can match a proxied image request by resourceType
    </td>
  </tr>
  <tr>
    <td colspan="2">
      <a href="https://cloud.cypress.io/projects/ypt4pf/runs/55272/overview/c5f5624e-1679-4869-827c-e2af79059ee3?reviewViewBy=FLAKY&utm_source=github&utm_medium=flaky&utm_campaign=view%20test">
        ... > stops waiting when an xhr request is canceled
      </a>
    </td>
    <td>
      
    </td>
  </tr></table>

Review all test suite changes for PR #29312 ↗︎


for (const [key, value] of parsedUrl.searchParams) {
if (SENSITIVE_KEYS.includes(key)) {
parsedUrl.searchParams.set(key, 'X'.repeat(value.length))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to match the value length? Not sure if just the length of the string could give something away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these keys are fairly temporary, so I'm not sure if it's even necessary to be honest.

Copy link
Collaborator

@ryanthemanuel ryanthemanuel left a comment

Choose a reason for hiding this comment

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

Looks really good. A couple of smaller things.

@cacieprins cacieprins requested a review from mschile April 26, 2024 15:36
@@ -1,12 +1,16 @@
<!-- See the ../guides/writing-the-cypress-changelog.md for details on writing the changelog. -->
## 13.8.1
## 13.9.0
Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a new section for 13.9.0 since 13.8.1 has already been released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 122cd50

Copy link
Contributor

@mschile mschile left a comment

Choose a reason for hiding this comment

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

Looks good! One small comment on the changelog.

cli/CHANGELOG.md Outdated Show resolved Hide resolved
@cacieprins cacieprins merged commit fc88764 into develop May 1, 2024
80 of 82 checks passed
@cacieprins cacieprins deleted the cacie/feat/improved-upload-error-msgs branch May 1, 2024 18:32
jj497 pushed a commit to jj497/cypress that referenced this pull request May 5, 2024
…re (cypress-io#29312)

* new error messages

* errors for initialization and capturing test replay

* messaging the case where no protocol instance but also no fatal error

* adds suggested remedies to initialization errors

* differentiation between network and http errors, initial work on db missing error

* improve db not found error

* add new error type to schema

* rm commented dead code

* clean up network error creation in uploadStream

* make artifact confirmation error msg more general

* test display of put instance artifacts failure

* changelog

* ensure we are displaying errors even in quiet mode

* fix pending changelog

* new snapshots for protocol errors

* improve aggregate error

* resolved html error snapshots

* fix check-ts

* fix test

* sanitize temp dir in CLOUD_PROTOCOL_CAPTURE_FAILURE error msg

* changelog

* fix double entry of certain network errors, error message prop for network errors

* fix url arg for network error

* Update packages/server/lib/cloud/artifacts/upload_artifacts.ts

Co-authored-by: Ryan Manuel <[email protected]>

* rm extra formatting from debug

* snapshot whitespace

* changelog update

* +pending

* Update cli/CHANGELOG.md

* resolve snapshots?

* does moving the stack trace fix whitespace in snapshots in ci?

* maybe fixes whitespace on electron now?

* fully normalize stack traces

* remove full normalization

* maybe debug stack normalization

* rm stack traces from error messages

---------

Co-authored-by: Jennifer Shehane <[email protected]>
Co-authored-by: Ryan Manuel <[email protected]>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 8, 2024

Released in 13.9.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.9.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test Replay: [Upload] Display more informative errors
4 participants