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

error when newSocket is undefined in lib/_http_agent.js #13831

Closed
k1tzu opened this issue Jun 20, 2017 · 1 comment · Fixed by #13839
Closed

error when newSocket is undefined in lib/_http_agent.js #13831

k1tzu opened this issue Jun 20, 2017 · 1 comment · Fixed by #13839
Labels
async_hooks Issues and PRs related to the async hooks subsystem. http Issues or PRs related to the http subsystem.

Comments

@k1tzu
Copy link

k1tzu commented Jun 20, 2017

in https://github.com/nodejs/node/blob/master/lib/_http_agent.js
you have the code

const newSocket = self.createConnection(options, oncreate);
...
function oncreate(err, s) {
...
if (err)
      return cb(err); 
... 
}

//cb returns us to to
this.createSocket(req, options, function(err, newSocket) {
      if (err) {
        nextTick(newSocket._handle.getAsyncId(), function() {
          req.emit('error', err);
        });
...

How can you call _handle on newSocket in case there was an error creating that newSocket?
Looks not right to me.

For example where I run into it:
I have a need in a simple custom .createConnection where I do

    this.createConnection = (options, callback) => {
    let localhandle = net._createServerHandle(options.localAddress);
    if (typeof localhandle === 'number') {
        return callback("handle creation error: "+localhandle);
    }
...

And since one of your latest releases I'm receiving this error from time to time when localhandle is an error:

TypeError: Cannot read property '_handle' of undefined
                                              at _http_agent.js:186:27
                                              at oncreate (_http_agent.js:230:14)
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Jun 20, 2017
@mscdex mscdex added the async_hooks Issues and PRs related to the async hooks subsystem. label Jun 21, 2017
@mscdex
Copy link
Contributor

mscdex commented Jun 21, 2017

This is a duplicate of the second part of this issue which I've now re-opened.

refack added a commit to refack/node that referenced this issue Jul 3, 2017
PR-URL: nodejs#13839
Fixes: nodejs#13045
Fixes: nodejs#13831
Refs: nodejs#13352
Refs: nodejs#13548 (comment)
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
addaleax pushed a commit to addaleax/node that referenced this issue Jul 3, 2017
PR-URL: nodejs#13839
Fixes: nodejs#13045
Fixes: nodejs#13831
Refs: nodejs#13352
Refs: nodejs#13548 (comment)
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
addaleax pushed a commit that referenced this issue Jul 11, 2017
PR-URL: #13839
Fixes: #13045
Fixes: #13831
Refs: #13352
Refs: #13548 (comment)
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
addaleax pushed a commit that referenced this issue Jul 18, 2017
PR-URL: #13839
Fixes: #13045
Fixes: #13831
Refs: #13352
Refs: #13548 (comment)
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fishrock123 pushed a commit that referenced this issue Jul 19, 2017
PR-URL: #13839
Fixes: #13045
Fixes: #13831
Refs: #13352
Refs: #13548 (comment)
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async_hooks Issues and PRs related to the async hooks subsystem. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants