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

defining Object.prototype.then causes esm loader to throw an error #45035

Closed
KhafraDev opened this issue Oct 17, 2022 · 2 comments · Fixed by #45044
Closed

defining Object.prototype.then causes esm loader to throw an error #45035

KhafraDev opened this issue Oct 17, 2022 · 2 comments · Fixed by #45044

Comments

@KhafraDev
Copy link
Member

Version

v18.11.0

Platform

Microsoft Windows NT 10.0.19043.0 x64

Subsystem

No response

What steps will reproduce the bug?

Object.prototype.then = (onFulfilled) => {
	delete Object.prototype.then
	onFulfilled({ value: undefined, done: true })
}

How often does it reproduce? Is there a required condition?

N/A

What is the expected behavior?

It would be great not to throw an error.

What do you see instead?

If you import/dynamically import something before setting Object.prototype.then, you get this error:

node:internal/modules/esm/loader:527
        .then(({ module }) => module.getNamespace());
                                     ^

TypeError: Cannot read properties of undefined (reading 'getNamespace')
    at node:internal/modules/esm/loader:527:38
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

Node.js v18.11.0

however if you use a dynamic import after it, you get a different error:

node:internal/errors:484
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected a url string to be returned for the "url" from the "node:internal/modules/esm/resolve 'resolve'" 
function but got type undefined.
    at new NodeError (node:internal/errors:393:5)
    at ESMLoader.resolve (node:internal/modules/esm/loader:872:13)
    at async ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:7)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async file:///C:/Users/me/Documents/test.mjs:6:1 {
  code: 'ERR_INVALID_RETURN_PROPERTY_VALUE'
}

Additional information

this is a very niche use case

@devsnek
Copy link
Member

devsnek commented Oct 17, 2022

This is not really possible to fix. If you're trying to monkey-patch some code, I'd suggest modifying the specific objects you're working with rather than Object.prototype.

@KhafraDev
Copy link
Member Author

I'm (trying) to run a WPT that modifies Object.prototype and crashes the entire runner. If it's not possible to fix it can be closed :)

aduh95 added a commit to aduh95/node that referenced this issue Oct 17, 2022
nodejs-github-bot pushed a commit that referenced this issue Oct 19, 2022
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Nov 1, 2022
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Nov 10, 2022
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
Fixes: #45035
PR-URL: #45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants