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

Jump to bottom

Non-cross-origin uncaught error messages are left undefined #7590

Closed
juliogarciag opened this issue Jun 4, 2020 · 6 comments · Fixed by #7637
Closed

Non-cross-origin uncaught error messages are left undefined #7590

juliogarciag opened this issue Jun 4, 2020 · 6 comments · Fixed by #7637
Assignees
@chrisbreiding
Labels
type: error message v4.6.0 🐛 Issue present since 4.6.0

Comments

@juliogarciag
Copy link

juliogarciag commented Jun 4, 2020 •
edited

Current behavior:

In this PR, the way createUncaughtException creates an error changed, and the message variable was left undefined under these conditions: if crossOriginScriptRe.test(msg) fails and the err variable is not defined. The error that triggered the situation was a ResizeObserver loop limit exceeded error.

Because the error message ends up being undefined, checks like this no longer work:

Cypress.on("uncaught:exception", (error) => {
  if (error.message && error.message.includes("ResizeObserver loop limit")) {
    return false;
  }
});

This is a screenshot of the debugging session in which I found this:

Image 2020-06-04 at 2 26 24 PM

Desired behavior:

The final uncaught error message has the original information and can be used to catch expected errors.

Test code to reproduce

Any thrown string should work:

index.html

<html>
  <body>
    <button id="button">Click</button>
  </body>
  <script>
    document.getElementById("button").addEventListener(() => {
      throw "clicked";
    });
  </script>
</html>

test.js

Cypress.on('uncaught:exception', (err, runnable) => {
  if (err.message === "clicked") {
    return false;
  }
})

it("should not fail because the error should be caught", () => {
  cy.visit("index.html");
  cy.get("#button").click();
});

Versions

Cypress 4.7.0
MacOS 10.15.5
Electron 80
@jennifer-shehane jennifer-shehane added the v4.6.0 🐛 Issue present since 4.6.0 label Jun 8, 2020
@jennifer-shehane
Copy link
Member

jennifer-shehane commented Jun 8, 2020 •
edited

The code line being referred to is here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cy/errors.js#L42:L42

I can recreate the behavior from the code provided.

Screen Shot 2020-06-08 at 2 37 06 PM

This is a common workaround for people to ignore specific error messages coming from app/cross origin scripts and I believe should be continued to be supported in some way. I even advised people with this ResizeObserver error to ignore them in this way: quasarframework/quasar#2233 (comment)

@cypress-bot cypress-bot bot added the stage: needs investigating Someone from Cypress needs to look at this label Jun 8, 2020
@chrisbreiding
Copy link
Contributor

@jennifer-shehane I think the error you're seeing is different due to an omission in the code reproduction. In the html, it should be:

document.getElementById("button").addEventListener("click", () => {

When I fix that, I get the following error:

Cannot read property 'split' of undefined

@juliogarciag Is that the error you see when the ResizeObserver loop limit error is thrown? Just want to make sure we have the reproduction right.

@chrisbreiding
Copy link
Contributor

In looking into this further, I think the test code should be revised to:

Cypress.on('uncaught:exception', (err, runnable) => {
  if (err.message.includes("clicked")) {
    return false;
  }
})

The error will always be wrapped in the "originated from your application" part, so the message won't exactly equal the one thrown. It will include the message thrown though. This matches the code included in the issue description concerning the ResizeObserver error.

@juliogarciag
Copy link
Author

@jennifer-shehane I think the error you're seeing is different due to an omission in the code reproduction. In the html, it should be:

document.getElementById("button").addEventListener("click", () => {

When I fix that, I get the following error:

Cannot read property 'split' of undefined

@juliogarciag Is that the error you see when the ResizeObserver loop limit error is thrown? Just want to make sure we have the reproduction right.

Oh, it seems I made a mistake on that reproduction. It needs the click to work, and it only happen when an error meets these two criteria. I don't remember right now if that was the exact error I got, just that error.message was undefined.

@chrisbreiding chrisbreiding mentioned this issue Jun 9, 2020
Merged
2 tasks
@cypress-bot cypress-bot bot added stage: work in progress There is an open PR for this issue [WIP] stage: needs review The PR code is done & tested, needs review and removed stage: needs investigating Someone from Cypress needs to look at this stage: work in progress There is an open PR for this issue [WIP] stage: needs review The PR code is done & tested, needs review labels Jun 9, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 10, 2020

The code for this is done in cypress-io/cypress#7637, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot added stage: pending release There is a closed PR for this issue and removed stage: needs review The PR code is done & tested, needs review labels Jun 10, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 23, 2020

Released in 4.9.0.

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

@cypress-bot cypress-bot bot removed the stage: pending release There is a closed PR for this issue label Jun 23, 2020
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Jun 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@chrisbreiding chrisbreiding

Labels
type: error message v4.6.0 🐛 Issue present since 4.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle uncaught errors thrown as strings cypress-io/cypress
3 participants
@juliogarciag @chrisbreiding @jennifer-shehane