facebook / memlab

A framework for finding JavaScript memory leaks and analyzing heap snapshots
https://facebook.github.io/memlab/
MIT License
4.43k stars 121 forks source link

Error occurs when executing scenarios involving dialog display #98

Closed YuyaItagaki closed 1 year ago

YuyaItagaki commented 1 year ago

I tried pressing a button on a web page that shows an alert dialog in the scenario, but it didn't work and showed an error.

Version

memlab version: 1.1.41 Node.js version: 18.18.2

The current behavior

The scenario is interrupted and an error is displayed.

Error
[rocky@localhost dialog-issue]$ memlab run --scenario ./scenario.js
page-load[498.7KB](baseline)[s1] > action-on-page(target)[s2] > revert(final)[s3]  
/home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/puppeteer-core/lib/cjs/puppeteer/util/assert.js:28
        throw new Error(message);
              ^

Error: Cannot accept dialog which is already handled!
    at assert (/home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/puppeteer-core/lib/cjs/puppeteer/util/assert.js:28:15)
    at CdpDialog.accept (/home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Dialog.js:84:32)
    at BrowserInfo.<anonymous> (/home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/@memlab/core/dist/lib/BrowserInfo.js:113:26)
    at Generator.next (<anonymous>)
    at /home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/@memlab/core/dist/lib/BrowserInfo.js:17:71
    at new Promise (<anonymous>)
    at __awaiter (/home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/@memlab/core/dist/lib/BrowserInfo.js:13:12)
    at /home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/@memlab/core/dist/lib/BrowserInfo.js:106:39
    at /home/rocky/.nvm/versions/node/v18.18.2/lib/node_modules/memlab/node_modules/puppeteer-core/lib/cjs/third_party/mitt/mitt.js:1:732
    at Array.map (<anonymous>)

Node.js v18.18.2

The expected behavior

The error is not displayed and the scenario continues.

Steps To Reproduce

  1. Install memlab with the command npm install -g memlab

  2. Create a page with an alert button (index.html).

<!DOCTYPE html>
  <html lang="en">
    <head>
      <meta charset="utf-8">
      <title>simple-alert</title>
    </head>
    <body>
      <button>Button</button>
    </body>
</html>
  1. Create a scenario to display alerts (scenario.js).
function url() {
  return "http://localhost:8080/";
}

async function action(page) {
  await page.click("button"); // Press the button to display the alert
}

async function back(page) {
  // No process
}

module.exports = { action, back, url };
  1. Start the HTTP server with the command npx http-server.

  2. Run the scenario with the command memlab run --scenario scenario.js.

Additional information

Workarounds

The error is worked around by resetting the definition of the dialog process and redefining dialog.accept() as follows.

However, it should be noted that the info display will also be reset. Because all the contents defined at the three locations described in "Details of Assumed Problem Areas" below is reset.

function url() {
  return "http://localhost:8080/";
}

async function action(page) {
  page.off('dialog');
  page.on('dialog', async dialog => {
    await dialog.accept();
  });
  await page.click("button"); // Press the button to display the alert
}

async function back(page) {
  // No process
}

module.exports = { action, back, url };
Assumed Problem Areas

In the testInBrowser function ( https://github.com/facebook/memlab/blob/main/packages/api/src/API.ts#L422 ), dialog.accept() is defined twice and dialog.dismiss () once. As shown in the error message Cannot accept dialog which is already handled!, the error is that the dialog cannot be controlled.

Details

The three locations where dialog controls such as dialog.accept() and dialog.dismiss() are defined are as follows.

The error described in "Error of The current behavior" above corresponds to dialog.accept() in the browserInfo.monitorWebConsole function at the third location.

By commenting out two of three locations, it is confirmed that the dialog control becomes one, and the scenario continues without error.

JacksonGL commented 1 year ago

Thank you for investigating this and for your detailed bug report. I will put up a commit to fix this.

I assume the button in the test page should trigger an alert call:

<button onclick="alert('test')">Button</button>