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

Uncaught TypeError: Cannot read property 'once' of undefined at EventEmitter.ee.once.req #427

Closed
austinfrey opened this issue Nov 26, 2017 · 35 comments

Comments

@austinfrey
Copy link

I'm attempting to use got from the browser with browserify. Looking through the issues I see others have had success, but I'm currently getting an error when using it. My request completes with the correct respsonse but an error come with it.

Uncaught TypeError: Cannot read property 'once' of undefined
    at EventEmitter.ee.once.req (bundle.js:10705)
    at EventEmitter.g (bundle.js:7590)
    at EventEmitter.emit (bundle.js:7521)
    at setImmediate (bundle.js:10740)
    at onNextTick (bundle.js:9657)
    at Item.run (bundle.js:8030)
    at drainQueue (bundle.js:8000)

The code is a fairly simple example.

const got = require('got')
global.setImmediate = require('timers').setImmediate

const options = {
        method: 'GET'
}

got('https://api.github.com', options)
        .then(x => console.log(x.body))
        .catch(console.log)

I'm running this command to make my bundle.js file

browserify index.js > bundle.js

Have I done something incorrectly?

@lukechilds
Copy link
Sponsor Contributor

I can recreate the issue, it's being caused by this line:

got/index.js

Line 229 in 974473a

req.connection.once('connect', () => {

Have I done something incorrectly?

Nope, we just released v8.0.0, one of the new features was progress events (introduced in #322) which uses request.connection to listen for the socket connection. Looks like browserify's http polyfill doesn't support request.connection.

@sindresorhus We could easily stop errors in browsers by just testing request.connection exists before using it. Not sure if the browser is actually an intended target though, do you want a fix for this?

@austinfrey
Copy link
Author

@lukechilds thanks for following up. good to know I'm not crazy :)

@sindresorhus
Copy link
Owner

I think this should be fixed in the Browserify http polyfill instead. That way it would benefit every HTTP lib, not just Got. Working around Browserify polyfills is a slippery slope.

@lukechilds
Copy link
Sponsor Contributor

Good point, I'm not sure if browser APIs expose enough to implement request.connection though. It gives access to the underlying socket.

I'll take a proper look tomorrow.

@lukechilds
Copy link
Sponsor Contributor

Yeah, I've had a look and I don't think there's a reliable way to implement request.connection in the browser.

We could probably implement the functionality we need for progress with browser APIs and expose it through request.connection which would work for our specific use case. But then all Browserify targets would have a half implemented request.connection which is probably worse than no implementation at all.

@zckrs
Copy link

zckrs commented Nov 28, 2017

I have same issue in Node environnement. I try isolate code to demonstrate the bug.
Currently I know bug occurs when I use p-map with a infinite concurrency or superior to length of first argument.

@jnotte
Copy link

jnotte commented Nov 28, 2017

I'm having the same issue but only pops up when I'm using an agent in the options (node.js 8.9.1).
Also tried different agents (caw, tunnel) and both have the same result. Version 7.1.0 does not have any issues.

The code that reproduced this issue: (error is thrown on the await statement).

let agent = new HttpsProxyAgent(this._proxy);
let options = {
    decompress: true,
    headers: headers,
    json: true,
    agent: agent,
    retries: 2
};

try {
    let response = await got(this._url, options);
} catch (error) {
    console.log(error);
}

@zckrs
Copy link

zckrs commented Nov 28, 2017

@NotteJacob o_O

Override agent fix problem
got(url, {json: true, agent: ''})

@jnotte
Copy link

jnotte commented Nov 28, 2017

@zckrs I need the agent for a proxied request. I'm currently using got version 7.1.0 which works without any issues

@zckrs
Copy link

zckrs commented Nov 28, 2017

I'm just surprised to fix my problem with this override.

(node: v6.9.4 here)

@jnotte
Copy link

jnotte commented Nov 28, 2017

@zckrs oh, like that :-) So there might be something up with the agent handling of version 8.0.0

@lukechilds
Copy link
Sponsor Contributor

lukechilds commented Nov 30, 2017

@zckrs @NotteJacob are you guys getting the exact same error as @austinfrey?

Uncaught TypeError: Cannot read property 'once' of undefined
    at EventEmitter.ee.once.req ...

Because that's caused by the Browserify http shim and shouldn't occur in Node.js.

Bugs caused by using with p-map or opts.agent should be totally unrelated. Can you open a separate issue if you have a different error.

@jnotte
Copy link

jnotte commented Nov 30, 2017

@lukechilds I'm getting the exact same error when I use an http proxy agent in nodejs. I am not getting this error with got version 7.1.0, only with 8.0.0, haven't tried earlier version though.
The code to reproduce the error:

"use strict";

let got = require("got");
let HttpsProxyAgent = require("https-proxy-agent");

let agent = new HttpsProxyAgent("{username}:{password}@{hostname}:{port}"); // http proxy
let options = {
    decompress: true,
    json: true,
    agent: agent
};

let request = got("{url}", options);
let body;
let error;
async function doRequest() {
    try {
        let response = await request;
        body = response.body;
    } catch (err) {
        error = err;
    } finally {
        console.log(error, body);
    }
}

doRequest().then().catch();

This screenshot is my test script to reproduce the error with in the bottom left the error it throws
when running it. In the bottom right you see the node version and npm version.
screen shot 2017-11-30 at 08 12 29

@zckrs
Copy link

zckrs commented Nov 30, 2017

Exactly same :-/

Maybe cacheable-request return sometime a req without connection pprt.

screenshot from 2017-11-30 10-04-17

@lukechilds
Copy link
Sponsor Contributor

lukechilds commented Nov 30, 2017

Maybe cacheable-request return sometime a req without connection pprt.

@zckrs Spot on! Sorry, I totally overlooked that. request.connection is a reference to the network socket. If a response is returned from cache we obviously can't return a reference to a socket (there is none).

@NotteJacob That still doesn't explain the agent issues you're experiencing though.

Can you guys test this branch? It should resolve your problems.

#429

@zckrs
Copy link

zckrs commented Nov 30, 2017

No problem. 😃

#429 resolve problem 🎉

@lukechilds
Copy link
Sponsor Contributor

@zckrs cany you just confirm, you have cache enabled right?

@zckrs
Copy link

zckrs commented Nov 30, 2017

No. Cache is disable :-/

@lukechilds
Copy link
Sponsor Contributor

Hmmn, ok, are you able to provide me with a reproducibe code example?

@jnotte
Copy link

jnotte commented Nov 30, 2017

#429 resolved my problem

@zckrs
Copy link

zckrs commented Nov 30, 2017

@lukechilds sorry I try to isolate code who reproduce bug but without success.
And I can't share the complete application.

@lukechilds
Copy link
Sponsor Contributor

lukechilds commented Nov 30, 2017

Ok, this is going to be quite difficult to fix without being able to reproduce.

Also, just to clarify, the issues you are experiencing (@NotteJacob and @zckrs) are completely seperate from the original post. It's the same property that doesn't exist but in a browser it's not expected to exist. In Node.js it should (unless coming from the cache). So #429 isn't a valid solution, it's just surpressing the errors.

@lukechilds
Copy link
Sponsor Contributor

@zckrs @NotteJacob Are either of you interested in putting a fully working reproducible scenario together for me?

Would love to get this fixed for you but I'm limited in what I can do without a repro.

@jnotte
Copy link

jnotte commented Dec 4, 2017

@lukechilds this zip file contains the index.js and package.json file which reproduces the error on my machine on Node version 8.9.1.
reproduce.zip

@lukechilds
Copy link
Sponsor Contributor

@NotteJacob Thanks!

@ghost
Copy link

ghost commented Dec 13, 2017

I recently ran into the same issue reported by @zckrs and @NotteJacob while using a tool called renovate on a RHEL 7 system using Node 9.3.0.

The error message I receive is:

/app/node_modules/got/index.js:230
				req.connection.once('connect', () => {
				               ^
 
TypeError: Cannot read property 'once' of null
    at EventEmitter.ee.once.req (/app/node_modules/got/index.js:230:20)
    at Object.onceWrapper (events.js:254:19)
    at EventEmitter.emit (events.js:164:20)
    at Immediate.setImmediate (/app/node_modules/got/index.js:265:8)
    at runCallback (timers.js:773:18)
    at tryOnImmediate (timers.js:734:5)
    at processImmediate [as _immediateCallback] (timers.js:711:5)
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.

If I set the agent property to an empty string, (Found in renovate here) I no longer receive the error:

$ git diff HEAD~1
diff --git c/lib/manager/npm/registry.js w/lib/manager/npm/registry.js
index ec5531c..34a06ee 100644
--- c/lib/manager/npm/registry.js
+++ w/lib/manager/npm/registry.js
@@ -55,6 +55,7 @@ async function getDependency(name) {
       cache: map,
       json: true,
       headers,
+      agent: ``,
     })).body;
     // Determine repository URL
     let repositoryUrl;

I also tried removing the cache option, and not setting agent, but I still received the same error. Setting agent to an empty string seemed to be the only way to avoid the error.

I have not tried #429, but I assume that because it checks for req.connection that it would work to resolve the issue I am encountering.

@lukechilds
Copy link
Sponsor Contributor

Thanks for the extra info @destroyerofbuilds.

Is it only a certain url/scenario that gives you the error or does it always occur unless you add {agent: ''}?

I'm limited on how much time I can spend on OSS at the moment, however this is high on my list of OSS priorities so hopefully will get a chance to look further into this very soon.

@ghost
Copy link

ghost commented Dec 15, 2017

Is it only a certain url/scenario that gives you the error or does it always occur unless you add {agent: ''}?

The URLs are all requests to an Artifactory deployment that's acting as a proxy to the npm registry. The URL requests are of the form:

I haven't been able to figure out which request is actually causing the crash (Requests are in parallel, and I can't reproduce this issue myself locally; only seeing this issue in a server environment)

@gary-archer
Copy link

I can reproduce this in a NodeJS OAuth Secured API that uses the OpenId-Client Library which in turn uses GOT.

The API makes introspection calls to validate received tokens. The problem only occurs when I want to view these HTTPS requests in a debugger (Fiddler or Charles), in which case my code needs to configure an agent:

if (process.env.HTTPS_PROXY) {
OpenIdClient.Issuer.defaultHttpOptions = {
agent: createMyAgent()
}
}

That is, any developer running a server side process that uses GOT is unable to look at HTTP(S) calls, which of course is a common requirement when developing.

  • TypeError: Cannot read property 'once' of null (index.js 229:19)

Looking at code, the req.connection object is always null when running Fiddler or Charles:

  • req.connection.once('connect', () => {

So it feels like the earlier suggestion to check whether req.connection is present would be useful for server side usage also.

My code is available here by the way, and is a referred to in a blog I'm writing.

Other that that GOT is a very nice library - but for now I'm using an old version of OpenId-Client to work around the problem.

@ghost
Copy link

ghost commented Jan 8, 2018

As soon as setImmediate is called on line 265, a connection property is available on the request object in all but a few cases.

The following URLs had undefined connection properties:

I ran the following code through a tight loop (1000 simultaneous requests) to see if I could replicate the issue:

  got(`http://artifactory.example.com/artifactory/api/npm/npm/watchify`, {
    json: true,
    headers: {},
  });

The options passed to got are identical to those used in Renovate (where I can replicate this issue).

So far I haven't been able to replicate the issue using the code above.

I also discovered that I only encounter this issue when using Renovate and an Artifactory proxy. I cannot replicate this issue using Renovate and the standard npm registry.

@lukechilds
Copy link
Sponsor Contributor

@sindresorhus This is obviously causing a lot of problems for people. I'd recommend we merge #429 to suppress the issue.

It will stop the errors but obviously progress events will not work in these scenarios.

I would like to spend more time in the future to figure out exactly why request.connection sometimes doesn't exist on Node.js but unfortunately I don't have the time right now.

My gut feeling is that certain proxy agents are manipulating/recreating request and not updating/adding the connection property. This is incompatible with the new progress events introduced in #322.

If that's the case it may even be that #429 is the only valid solution and we should just document that progress events won't work with some proxy agents.

@austinfrey
Copy link
Author

haven't checked this in awhile but thanks to everyone who worked on it!

@gary-archer
Copy link

Just to say that this is fixed and using Fiddler / Charles in the latest version is fine - great work guys

@rwlaschin
Copy link

I'm seeing this problem :(

I'm using babel-node/fastify, connecting to mysql db using mysql2. Everything is running off of localhost.

npx clinic doctor --autocannon -- ./node_modules/.bin/babel-node server/index.js

"clinic": "^2.2.1",
"@babel/cli": "^7.2.3",
"autocannon": "^3.2.0",

/Users/rwlaschin/Work/Wholosophy/wholosophy-web/node_modules/on-net-listen/index.js:14
resource.owner.once('listening', function () {
^

TypeError: Cannot read property 'once' of null
at /Users/rwlaschin/Work/Wholosophy/wholosophy-web/node_modules/on-net-listen/index.js:14:26
at process._tickCallback (internal/process/next_tick.js:61:11)
at Function.Module.runMain (internal/modules/cjs/loader.js:745:11)
at Object. (/Users/rwlaschin/Work/Wholosophy/wholosophy-web/node_modules/@babel/node/lib/_babel-node.js:234:23)
at Module._compile (internal/modules/cjs/loader.js:689:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
at Module.load (internal/modules/cjs/loader.js:599:32)
at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
at Function.Module._load (internal/modules/cjs/loader.js:530:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
/Users/rwlaschin/Work/Wholosophy/wholosophy-web/node_modules/clinic/bin.js:376
if (err) throw err

@szmarczak
Copy link
Collaborator

@rwlaschin This doesn't look like a Got issue 🤔

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

No branches or pull requests

8 participants