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

Cannot make a node-fetch request, TypeError: Cannot read property 'Symbol(requestOptions)' of undefined #36364

Closed
seven-deuce opened this issue Dec 3, 2020 · 6 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@seven-deuce
Copy link

  • Version: 14.15.1
  • Platform: MINGW64_NT-10.0-17763 DESKTOP-GJB54B0 3.0.7-338.x86_64 2019-04-30 21:52 UTC x86_64 Msys
  • Subsystem:

What steps will reproduce the bug?

Any call to foreign api, using a trusted library like "node-fetch", creates error and crash servers, on versions higher than 12.18
Here is the error:

_http_agent.js:444
      options = req[kRequestOptions];
                   ^

TypeError: Cannot read property 'Symbol(requestOptions)' of undefined
    at Agent.removeSocket (_http_agent.js:444:20)
    at TLSSocket.onClose (_http_agent.js:371:11)
    at TLSSocket.emit (events.js:327:22)
    at net.js:673:12
    at TCP.done (_tls_wrap.js:563:7)

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

All the time, not in version 12.18 or lower

What is the expected behavior?

a normal fetch, returning the result from any api, mostly a json

What do you see instead?

app crashes!

Additional information

@RaisinTen
Copy link
Contributor

I'm having some difficulty reproducing this:

❯ cat index.js
const fetch = require('node-fetch');

(async () => {
	const response = await fetch('https://api.github.com/users/github');
	const json = await response.json();

	console.log(json);
})();
❯ node index.js &> /dev/null
❯ echo $?
0
❯ node -v
v15.2.0
❯ uname -a
Linux hp 4.15.0-124-generic #127-Ubuntu SMP Fri Nov 6 10:54:24 UTC 2020 i686 i686 i686 GNU/Linux

As you can see, it exits with a zero error code.

@seven-deuce
Copy link
Author

I tracked down the issue and found out that one of the libs in the project, was defining methods on Object and Array Prototype.
When I removed it, it all started to work,
I wonder, what has changed after node v.12.18, that does not allow defining methods on Object/Array prototype??

@targos targos self-assigned this Dec 6, 2020
@targos targos added the http Issues or PRs related to the http subsystem. label Dec 6, 2020
targos added a commit to targos/node that referenced this issue Dec 6, 2020
@targos targos removed their assignment Dec 6, 2020
targos added a commit to targos/node that referenced this issue Dec 7, 2020
@Trott
Copy link
Member

Trott commented Dec 9, 2020

I tracked down the issue and found out that one of the libs in the project, was defining methods on Object and Array Prototype.
When I removed it, it all started to work,
I wonder, what has changed after node v.12.18, that does not allow defining methods on Object/Array prototype??

This may be due to expanded use of what we call primordials on the project. The idea is that if a user changes the Object prototype, it should affect their code but not Node.js internal code. This is a security precaution but it may cause difficulty for people who "monkey-patch" Node.js internals like AMP vendors. I'd be curious to know what library is exhibiting this problem so we can track down whether this is expected breakage or a bug in Node.js that we need to fix.

@Trott
Copy link
Member

Trott commented Dec 9, 2020

I tracked down the issue and found out that one of the libs in the project, was defining methods on Object and Array Prototype.
When I removed it, it all started to work,
I wonder, what has changed after node v.12.18, that does not allow defining methods on Object/Array prototype??

This may be due to expanded use of what we call primordials on the project. The idea is that if a user changes the Object prototype, it should affect their code but not Node.js internal code. This is a security precaution but it may cause difficulty for people who "monkey-patch" Node.js internals like AMP vendors. I'd be curious to know what library is exhibiting this problem so we can track down whether this is expected breakage or a bug in Node.js that we need to fix.

Though it looks like @targos has already self-assigned this so they probably have a good idea what the bug is and what the fix is.

@targos
Copy link
Member

targos commented Dec 9, 2020

#36409 and #36410 are the pull requests that fix this issue. I referenced the issue in the PRs but forgot to do the opposite here, sorry!

@targos
Copy link
Member

targos commented Dec 9, 2020

To explain the bug: there was a semver-minor change that introduced a for ... in loop on the requests property of http Agent.
If enumerable properties were added to Object.prototype, they would be iterated by this loop, triggering this bug because they probably wouldn't have an object at index 0.

@targos targos closed this as completed in 2ef9a76 Dec 13, 2020
targos added a commit that referenced this issue Dec 13, 2020
Fixes: #36364

PR-URL: #36410
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 8, 2021
Fixes: #36364

PR-URL: #36410
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
ruyadorno pushed a commit that referenced this issue Feb 8, 2021
Fixes: #36364

PR-URL: #36410
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
ruyadorno pushed a commit that referenced this issue Feb 10, 2021
Fixes: #36364

PR-URL: #36410
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants