capricorn86 / happy-dom

A JavaScript implementation of a web browser without its graphical user interface
MIT License
3.27k stars 200 forks source link

Using `Browser` `close` method in `bun` throws TypeError for missing destroy function #1225

Open maxmilton opened 8 months ago

maxmilton commented 8 months ago

Describe the bug Attempting to use a Browser instance throws a TypeError when trying to call the browser instance's close method. This only occurs with bun and not with nodejs.

To Reproduce Steps to reproduce the behavior:

  1. Create a file test.mjs:

    import { Browser } from "happy-dom";
    
    const browser = new Browser();
    browser.newPage();
    await browser.close();
  2. Run bun ./test.mjs.
  3. Observe the resulting thrown error.

Expected behavior Using await browser.close() or await page.close() should run without throwing.

Screenshots

CLI output:

~/P/_/repro-happy-dom-browser ❱ bun ./test.mjs
32 |                 if (index !== -1) {
33 |                     frame.parentFrame.childFrames.splice(index, 1);
34 |                 }
35 |             }
36 |             // We need to destroy the Window instance before triggering any async tasks as Window.close() is not async.
37 |             frame.window[PropertySymbol.destroy]();
                 ^
TypeError: frame.window[PropertySymbol.destroy] is not a function. (In 'frame.window[PropertySymbol.destroy]()', 'frame.window[PropertySymbol.destroy]' is undefined)
      at /home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/utilities/BrowserFrameFactory.js:37:13
      at new Promise (:1:21)
      at destroyFrame (/home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/utilities/BrowserFrameFactory.js:25:16)
      at /home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/utilities/BrowserPageUtility.js:31:13
      at new Promise (:1:21)
      at closePage (/home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/utilities/BrowserPageUtility.js:22:16)
      at map (:1:21)
      at /home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/BrowserContext.js:28:27
      at close (/home/max/Projects/repro-happy-dom-browser/node_modules/happy-dom/lib/browser/BrowserContext.js:24:19)
      at map (:1:21)

Versions:

Additional context This seems like it could be something to do with how bun transpiles files before running them. Perhaps it's a race condition with the code execution being different from node?

Adding console.log(Object.getOwnPropertySymbols(frame.window)); before the call to the missing function shows that the symbol is indeed missing:

[ Symbol(listeners), Symbol(listenerOptions), Symbol(captureEventListenerCount),
  Symbol(mutationObservers), Symbol(readyStateManager), Symbol(happyDOMSettingsID)
]

Even though this is bun specific, I feel it's still worth tracking here.

shelterit commented 7 months ago

Did a bit of poking, and found maybe a couple of hints;

  1. There's one instance of BrowserWindow normally, but if I register with "GlobalRegistrator.register();" there's two instances (may be nothing, but the one I think register() creates doesn't have the issue described next which is a bit puzzling to me)
  2. The "this" has its context changed after setting up the VM with "VM.createContext(this);" causing issues. I botched together some code to test it;
this.__check('setup-vm');
this[PropertySymbol.setupVMContext]();
//   <-------- from this point on "this" is broken, and next line throws an error (" 'this.__check' is undefined ")
this.__check('classes');

I guess this points to some bug in Bun's implementation of the VM and setting up the context. I'll investigate further when I get a spare moment.

capricorn86 commented 2 weeks ago

Related to #1415