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

fix MaxListenersExceededWarning #910

Merged

Conversation

@mizdra
Copy link
Contributor

@mizdra mizdra commented Sep 29, 2017

A EventListener is not removed when Protocol error (Page.navigate) is catched. This causes MaxListenersExceededWarning.

Copy link
Collaborator

@aslushnikov aslushnikov left a comment

Awesome, thank you for the PR.

lgtm % test comment

const closedPage = await browser.newPage();
await closedPage.close();

process.once('warning', warning => expect(warning).toBe(null));

This comment has been minimized.

@aslushnikov

aslushnikov Sep 29, 2017
Collaborator

this event listener should be removed after the test.

note: You can just copy the previous test and use 'asdf' instead of EMPTY_PAGE as url:

    it('should not leak listeners during bad navigation', SX(async function() {
      let warning = null;
      const warningHandler = w => warning = w;
      process.on('warning', warningHandler);
      for (let i = 0; i < 20; ++i)
        await page.goto('asdf').catch(e => {/* swallow navigation error */});
      process.removeListener('warning', warningHandler);
      expect(warning).toBe(null);
    }));

This comment has been minimized.

@mizdra

mizdra Sep 30, 2017
Author Contributor

Oh, I fixed it.

Thank you 😃

- add removeListener
- use 'asdf' instead of EMPTY_PAGE
@aslushnikov aslushnikov merged commit 215b349 into puppeteer:master Sep 30, 2017
2 checks passed
2 checks passed
cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants