cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
47.02k stars 3.18k forks source link

Electron browser hangs/errors on apps with window.onbeforeunload alerts #2118

Open chheller opened 6 years ago

chheller commented 6 years ago

Current behavior:

When running a test using the Electron browser on a webpage that contains a window.onbeforeunload script and trying to navigate away, Cypress will fail to load the next page. The test will eventually fail after a PageLoadTimeout exception. This occurs when using any kind of navigation such as cy.reload(), cy.visit(), or interacting with an element that contains an href linking to somewhere else.

Test run with Electron 59:

Desired behavior:

Test run with Chrome 67:

Steps to reproduce:

describe('Electron 59 alert prompt bug', function() {
  it('Should navigate away from the page', function() {
    cy.on('window:confirm', function() {
      return true;
    });
    cy.visit('http://localhost:8080/');
    cy.visit('http://localhost:8080/form');
  });
  it('Should reload the page', function() {
    cy.visit('http://localhost:8080/');
    cy.reload();
  });
});

The page under test needs to contain a window.onbeforeunload function which alters the returnValue. To reproduce, I use http-server to serve a barebones HTML file

<!DOCTYPE html>

<script>
  window.onbeforeunload = function (e) {
    e.returnValue = 'Dialog';
    return;
  }
</script>

Versions

Cypress: 3.0.2 MacOS: High Sierra 10.13.5 Browsers: Chrome 67, Electron 59

marmite22 commented 6 years ago

I've just hit this exact same problem. For now I've wrapped my onBeforeUnload binding in a check for cypress which feels a bit dirty.

 if (!window.Cypress) {
    $window.onbeforeunload = function (e) { ... }
}
bimimicah commented 6 years ago

I was able to work around the issue with the following code in the test:

cy.window().then((win) =>  {
    win.onbeforeunload = null;
})
cy.visit(<the next page goes here>)
jennifer-shehane commented 6 years ago

Reproducible example: https://github.com/cypress-io/cypress-test-tiny/pull/27

brandonb927 commented 5 years ago

Just chiming in here to say we have an issue within our tests because Cypress doesn't properly handle the window.onbeforeunload event in Electron w/ Chromium 59, but works perfectly fine using Chrome 70 running the test runner. In our Jenkins env using docker, it also experiences the same issue with the cypress/base:10 image.

My guess is that the version of Chromium used by Electron (59) that ships with Cypress currently (3.1.0) has some major bugs and issues. I see #2559 is acknowledged, any ETA when that might get rolled out? I imagine this would fix a swath of issues being worked around due to an old version of Chromium.

Ya'll doin' great work though, I really appreciate Cypress :)

Cypress: 3.1.0 macOS: 10.14.1 Browsers: Electron 59, Chrome 70

Obi-Dann commented 5 years ago

Started encountering this problem in our CI build. For now going to workaround by monkey patching window.addEventListener:

Cypress.on('window:load', function(window) {
    const original = window.addEventListener;
    window.addEventListener = function() {
        if (arguments && arguments[0] === 'beforeunload') {
            return;
        }
        return original.apply(this, arguments);
    };
});
Fonger commented 5 years ago

If you're using window.onbeforeunload=function() {...} instead of window.addEventListener('beforeunload', function() {...}, you can use this workaround:

Cypress.on('window:load', function(window) {
  Object.defineProperty(window, 'onbeforeunload', {
    get: function() {},
    set: function() {}
  });
});
fritz-c commented 5 years ago

Regarding dannsam's approach, you may want to change 'window:load' to 'window:before:load' if you are adding the beforeunload listener in some code executed on load.

szymach commented 5 years ago

Thank you @dannsam and @fritz-c , your workaround did it for me. I have encountered this issue using the autosave plugin for CKEditor 5, since it tries to make sure the last call was completed or something.

bahmutov commented 5 years ago

I have pushed another example https://github.com/cypress-io/cypress-test-tiny/tree/test-unload that shows the problem in Electron 61 (Chrome passes, despite showing an error). The website in question is http://webdriverjsdemo.github.io/leave/

and has the following script

window.onbeforeunload = function(){
  return 'Are you sure you want to leave?';
};

The error displayed in the browser console:

Screen Shot 2019-07-06 at 6 05 20 PM

This error has docs in https://www.chromestatus.com/feature/5082396709879808 and probably is not disabled in Electron

Funnily, you cannot close the Electron browser in GUI mode - it just keeps showing the error

Screen Shot 2019-07-06 at 6 05 38 PM

Running cypress run --headed shows the error that happens on CI - the test keeps running

Screen Shot 2019-07-06 at 6 08 12 PM
jennifer-shehane commented 5 years ago

There is also a situation where, where after visiting the site above, this will exhibit as a Page Load timeout.

Failing test code below:

describe('page', () => {
  it('works', () => {
    cy.visit('http://webdriverjsdemo.github.io/leave/')
    cy.contains('Go Home')
      .click()
  })

  it('works 2', () => {
    cy.visit('http://webdriverjsdemo.github.io/leave/')
      .contains('Go Home')
      .click()
  })
})
Screen Shot 2019-11-04 at 5 03 12 PM Screen Shot 2019-11-04 at 5 00 17 PM

Electron will not display this error in the Console when this is happening also (if you have not manually interacted with the page)

[Intervention] Blocked attempt to show a 'beforeunload' confirmation panel for a frame that never had a user gesture since its load. https://www.chromestatus.com/feature/5082396709879808

Workaround

There are 2 possible workarounds.

  1. Run your tests in Chrome using the --chrome flag during cypress run or choosing Chrome during cypress open.

OR

  1. Add the code below to your support/index.js file. This will prevent the prompts from occurring - and not hang Electron.
Cypress.on('window:before:load', function (win) {
  const original = win.EventTarget.prototype.addEventListener

  win.EventTarget.prototype.addEventListener = function () {
    if (arguments && arguments[0] === 'beforeunload') {
      return
    }
    return original.apply(this, arguments)
  }

  Object.defineProperty(win, 'onbeforeunload', {
    get: function () { },
    set: function () { }
  })
})
cypress-bot[bot] commented 5 years ago

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

cypress-bot[bot] commented 5 years ago

Released in 3.6.1.

hvuong-sigma commented 4 years ago

Is this confirmed fix? Still experiencing this after upgrading to 3.6.1.

jennifer-shehane commented 4 years ago

@hvuong-sigma Yes, please open a new issue with a reproducible example.

jennifer-shehane commented 4 years ago

I am reopening this issue. While this was partially addressed in https://github.com/cypress-io/cypress/pull/5603 in 3.6.1 release by fixing the hanging while running in Electron (aka - the process would never exit), this did not actually fix the error portion of the issue.

Problem

Any application that has a defined onbeforeunload handler that triggers an alert causes Cypress page load to time out within the Electron browser (not Chrome). Typically this is only noticed in CI since Electron is the chosen browser by default. Resulting in the error below:

Screen Shot 2020-01-30 at 12 20 23 PM

To reproduce

Example 1

index.html

<html>
<body>
  hello world
</body>
<script>
  window.onbeforeunload = () => confirm("test");
</script>
</html>

spec.js

it('should accept the confirm and successfully reload', function () {
  cy.on('window:confirm', () => true) // this is unnecessary as it is the default behavior
  cy.visit(`index.html`)
  cy.reload()
})

Example 2

it('works', () => {
  cy.visit('http://webdriverjsdemo.github.io/leave/')
  cy.contains('Go Home')
    .click()
})

Workaround

There are 2 possible workarounds.

  1. Run your tests in Chrome using the --browser flag during cypress run or choosing Chrome during cypress open.

OR

  1. Add the code below to your support/index.js file. This will prevent the prompts from occurring - and not hang Electron.
Cypress.on('window:before:load', function (win) {
  const original = win.EventTarget.prototype.addEventListener

  win.EventTarget.prototype.addEventListener = function () {
    if (arguments && arguments[0] === 'beforeunload') {
      return
    }
    return original.apply(this, arguments)
  }

  Object.defineProperty(win, 'onbeforeunload', {
    get: function () { },
    set: function () { }
  })
})
juampynr commented 4 years ago

The window:before:load solution worked for me.

I had an aha moment when I realized that the project that I work on shows a popup to confirm that the user will release her lock on that piece of content.

jennifer-shehane commented 4 years ago

This is not fixed as part of upcoming Electron 9 upgrade.

Russ93 commented 4 years ago

Hey I is anyone else still seeing this issue when using a custom protocol? Like this zoommtg://us04web.zoom.us/join?action=join&confno=...

I am getting this error

image

valscion commented 4 years ago

Looks like a different issue to me, @Russ93 — consider opening a new issue ☺️

kbaala88 commented 3 years ago

Still the same issue occurs. Accessing an URL and throws one pop-up since one file couldn't be loaded. After that, even after closing the pop-up the tool shows 'Loading' progress and keeps waiting for the page to load and throws the below error. image

valscion commented 3 years ago

Yes, this hasn't been fixed yet. See https://github.com/cypress-io/cypress/issues/2118#issuecomment-580095512 where there exists a workaround.

kbaala88 commented 3 years ago

Thanks for the quick response. Tried the below workaround, however still not working.

Cypress.on('window:before:load', function (win) {
  const original = win.EventTarget.prototype.addEventListener

  win.EventTarget.prototype.addEventListener = function () {
    if (arguments && arguments[0] === 'beforeunload') {
      return
    }
    return original.apply(this, arguments)
  }

  Object.defineProperty(win, 'onbeforeunload', {
    get: function () { },
    set: function () { }
  })
})
kbaala88 commented 3 years ago

Please suggest for any other workarounds and I shall try it!! Thanks in advance!!!

kbaala88 commented 3 years ago

Team any idea if this will get fixed any time soon? This blocker happens in chrome browser too (Both headless & normal).

bahmutov commented 3 years ago

Confirmed the behavior is problematic IF there are some user interaction, see reproduction in https://github.com/cypress-io/cypress-test-tiny/tree/onbeforeunload

This is similar to what I have observed in https://github.com/cypress-io/cypress/issues/2118#issuecomment-508957157

Spec

/// <reference types="cypress" />
describe('Test window.confirm', () => {
  it('should accept the confirm and successfully reload', function () {
    cy.visit('/')
      .wait(1000)
    cy.reload()
      .wait(1000)
    cy.reload()
      .wait(1000)
    cy.wait(1000) // let the video finish
  });
});

App

window.onbeforeunload = function (e) {
  console.log('app window.onbeforeunload')
  e.returnValue = 'Dialog';
  return;
}

In Electron (note no confirm dialog, silently times out)

https://user-images.githubusercontent.com/2212006/107830600-347ca400-6d5a-11eb-8810-e4ea5fcf5fcf.mp4

In Chrome 90 the confirmation dialog pops up

https://user-images.githubusercontent.com/2212006/107830618-3d6d7580-6d5a-11eb-92b5-9c65468607e0.mp4

If I do not touch the confirm dialog, load times out

Screen Shot 2021-02-12 at 5 41 52 PM

Interestingly, from the DevTools console removing these listeners does not seem to remove the app's listener

getEventListeners(window).beforeunload
  .forEach(({ type, listener, useCapture }) => {
    console.log('removing', type, listener, useCapture)
    window.removeEventListener(type, listener, useCapture)
  })

Second interesting point is that I could not make it hang in cypress run mode running with Electron or Chrome

npx cypress run --browser chrome
npx cypress run --browser chrome --headless
bahmutov commented 3 years ago

Explanation and workarounds

I looked at this problem again, and it is due to the browser trying to show the confirmation popup. I describe the problem and my workarounds in the blog post https://glebbahmutov.com/blog/onbeforeunload/ with all code in https://github.com/bahmutov/onbeforeunload-example

kbaala88 commented 3 years ago

bahmutov - Thanks for the updates. Still, there is no luck and every time the single particular page loads(Chrome/Electron) , cypress couldn't proceed and fails consistently with the details shown in snapshot. Unfortunately this occurs in very second page , hence blocking the entire flow. Any help, Thanks!

Capture
bahmutov commented 3 years ago

Can you share the code and the tests please? Without a reproducible example there is not much we can do

kbaala88 commented 3 years ago

@bahmutov : Unfortunately couldn't share the test or app code. For this issue, got one cypress support call scheduled upcoming week. But the error behavior is very consistent and the application couldnt cross the "2nd Page" of the flow and is a blocker. Seeing this error in console, 'This event initially fires when your application fires its 'beforeunload' event and completes when your application fires its 'load' event after the next page loads.'

Is there a way to skip the test step and proceed to the next one or forcefully load the page and move on? Thanks in advance!!!

melibe23 commented 3 years ago

I am having the same issue with Circle CI and locally. Any progress?

melibe23 commented 3 years ago

Correct me if I am wrong, but since I have the following in a hook, I cannot use this workaround, right?:

const ga = cy.stub().as("ga");
      cy.on("window:before:load", win => {
        Object.defineProperty(win, "ga", {
          configurable: false,
          // always return the stub
          get: () => (...props) => {
            ["send", "create"].includes(props[0]) && ga(...props);
          },
          set: () => {} // don't allow actual google analytics to overwrite this property
        });
melibe23 commented 3 years ago

Is there any news on this?

jennifer-shehane commented 3 years ago

@melibe23 No updates on this. It is still an issue.

leilafi commented 2 years ago

Are there still no updates on this issue?

Raph-Capitale commented 2 years ago

Still waiting for an update on this. Thanks for your work !

k4mr4n commented 1 year ago

This worked for me

Cypress.on('window:before:unload', e => {
    e.stopImmediatePropagation()
})
Joelasaur commented 1 year ago

This issue is keeping me from using cy.origin(), since when I click the href to navigate to an external site, I can see the browser is loading the page, but cypress is still failing because of the page load timeout error. Unable to post a reproducer since it's proprietary code but here's an excerpt of what I'm doing:

    cy.get('[data-cy="my-external-site"]').click();
    cy.origin('https://my-external-site.com', () => {
      cy.url().should('include', 'my-external-site');
    });

According to the docs, the above code should work, but this page load timeout error is keeping me from using it.

hemaybedead commented 1 year ago

Putting the code from workaround number 2 into my beforeEach() block solved this for me.

alecmestroni commented 1 year ago

Putting the code from workaround number 2 into my beforeEach() block solved this for me.

Whats the workaround number 2? @hemaybedead

alecmestroni commented 1 year ago

The only workaround that worked for me is to extend the 'pageLoadTimeout' to 600000 in cypress.config.js

adnanerlansyah403 commented 11 months ago

Any updates on this issue guys ?, I have got the same issue too and I've already made the thread for this and provide a reproducable code. here's the link, I hope you'll can fix this as fast because this making me frustrating. I can't run the test on chrome's and edge browser and only working on firefox

https://github.com/cypress-io/cypress/issues/28449

jennifer-shehane commented 11 months ago

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

kbaala88 commented 11 months ago

This is a major blocker for our app automation and reported like 3 years ago. still we hadn’t got the fix. Thanks team !!

On Wed, Dec 13, 2023 at 7:55 AM Jennifer Shehane @.***> wrote:

@adnanerlansyah403 https://github.com/adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

— Reply to this email directly, view it on GitHub https://github.com/cypress-io/cypress/issues/2118#issuecomment-1854194578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AONEAZPQ7BVWMDKLXL46AVTYJHFYNAVCNFSM4FJGT5WKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBVGQYTSNBVG44A . You are receiving this because you commented.Message ID: @.***>

adnanerlansyah403 commented 11 months ago

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

Oke, Thank you so much for your assitance. Please notice me or let me know if it's already fixed 🙏

adnanerlansyah403 commented 10 months ago

@adnanerlansyah403 No, this hasn't been fixed yet. It's on a list of bugs that we'd like to prioritize fixing though, if we find the time.

Hey @jennifer-shehane sorry, how about now for the issue? Is there any update on this? How to progress in solving the problem?

adnanerlansyah403 commented 9 months ago

Please keep it up for this issue, there are bunches of people occurring this.

valscion commented 9 months ago

Posting repeatedly on the issue thread will not help you, @adnanerlansyah403. It will only work to annoy people like me who are subscribed to this thread to see if it gets resolved at some point.

Cypress team will prioritize the issues as they see fit. You can add a 👍 reaction to the original issue message to help Cypress team prioritize this issue and see how many people it affects.

If anything, adding more "please fix this" comments serves to make Cypress team less interested in fixing this issue.

[!NOTE] I am not associated with Cypress in any way — so these opinions are my own.

jennifer-shehane commented 9 months ago

@valscion Commenting certainly doesn't make Cypress less interested in fixing an issue, but for longstanding issues, we do prioritize them based on impact and effort to fix and this is certainly a known issue in our backlog to be prioritized against other work.

There's this other issue with onbeforeunload alerts that's popped up and made this prevelant within Chrome browsers which has put this somewhat on our radar. https://github.com/cypress-io/cypress/issues/28553 But I'm not sure they can both be addressed together or not.

adnanerlansyah403 commented 9 months ago

@valscion Commenting certainly doesn't make Cypress less interested in fixing an issue, but for longstanding issues, we do prioritize them based on impact and effort to fix and this is certainly a known issue in our backlog to be prioritized against other work.

There's this other issue with onbeforeunload alerts that's popped up and made this prevelant within Chrome browsers which has put this somewhat on our radar. #28553 But I'm not sure they can both be addressed together or not.

I'm glad this issue is already on the backlog, I hope this can be fixed as soon as possible. And if it's already fixed on the next update of cypress, please notice it too in the description. I appreciate so much for developers cypress for what they're doing to fix the issue like this.

posti85 commented 1 month ago

I was facing the same issue. But I think the cause is different from the rest. My app is configured to be PWA, and the service worker was causing issues in the navigations (the cy.visit() call always hang).

More info about the problem and one of its workarounds here: https://www.youtube.com/watch?v=hmbx_f5Nlfs, people are talking about the same issue in this thread: https://github.com/cypress-io/cypress/issues/27501.

The workaround that worked for my was:

cy.intercept('**/registerSW.js', '');