"TypeError: Cannot set property 'port' of undefined" using .forever() with HTTPS #437

Closed
natevw opened this Issue Feb 21, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@natevw

natevw commented Feb 21, 2013

If I use request = require('request').forever() then I get the following error when making an SSL connection:

DEBUG: TypeError: Cannot set property 'port' of undefined
    at ForeverAgentSSL.createConnectionSSL [as createConnection] (/myproj/node_modules/request/forever.js:100:16)
    at ForeverAgentSSL.Agent.createSocket (http.js:1191:16)
    at ForeverAgentSSL.Agent.addRequest [as addRequestNoreuse] (http.js:1167:23)
    at ForeverAgentSSL.ForeverAgent.addRequest (/myproj/node_modules/request/forever.js:58:10)
    at new ClientRequest (http.js:1312:16)
    at Object.exports.request (https.js:105:10)
    at Request.start (/myproj/node_modules/request/main.js:559:30)
    at Request.end (/myproj/node_modules/request/main.js:1075:28)
    at IncomingMessage.onend (stream.js:66:10)
    at IncomingMessage.EventEmitter.emit (events.js:126:20)

There is some discussion here (looks like a workaround got patched in another app that uses request) and I've added a comment their referencing their actual commit.

@natevw natevw referenced this issue in AlgoTrader/betfair-sports-api Feb 21, 2013

Closed

TypeError: Cannot set property 'port' of undefined in forever.js #5

@natevw

This comment has been minimized.

Show comment Hide comment
@natevw

natevw Feb 21, 2013

The easy fix is to:

ForeverAgentSSL.prototype.createConnection = tls.connect //createConnectionSSL

(and get rid of createConnectionSSL function block)

Not submitting a pull request yet though as that may lose compatibility with older node versions and dealing with that could be a little more complex.

natevw commented Feb 21, 2013

The easy fix is to:

ForeverAgentSSL.prototype.createConnection = tls.connect //createConnectionSSL

(and get rid of createConnectionSSL function block)

Not submitting a pull request yet though as that may lose compatibility with older node versions and dealing with that could be a little more complex.

@natevw

This comment has been minimized.

Show comment Hide comment
@natevw

natevw Feb 21, 2013

This module's package.json specifies compat with node.js 0.3.6 and above. Going back through the TLS docs I see that back then tls.connect already took the same parameters as the old .createConnection expected. So atm I'm not even sure why that createConnectionSSL function exists — perhaps there was a point when TLS changed to the new options style before the Agent API did?

Unfortunately that API isn't documented so it's into the tagged sources…

natevw commented Feb 21, 2013

This module's package.json specifies compat with node.js 0.3.6 and above. Going back through the TLS docs I see that back then tls.connect already took the same parameters as the old .createConnection expected. So atm I'm not even sure why that createConnectionSSL function exists — perhaps there was a point when TLS changed to the new options style before the Agent API did?

Unfortunately that API isn't documented so it's into the tagged sources…

@natevw

This comment has been minimized.

Show comment Hide comment
@natevw

natevw Feb 21, 2013

D'oh! I didn't look carefully enough. tls.connect had host/port swapped from what the agent stuff has — although I'm not even seeing calls to createConnection in the 0.4 http/https implementations, just _getConnection.

natevw commented Feb 21, 2013

D'oh! I didn't look carefully enough. tls.connect had host/port swapped from what the agent stuff has — although I'm not even seeing calls to createConnection in the 0.4 http/https implementations, just _getConnection.

@natevw

This comment has been minimized.

Show comment Hide comment
@natevw

natevw Feb 21, 2013

Ok, AFAICT like .forever() isn't actually compatible with 0.4 right now anyway. My patch suggestion should work just fine in v0.6 since tls.connect is just an exported function that doesn't depend on its this === require('tls').

natevw commented Feb 21, 2013

Ok, AFAICT like .forever() isn't actually compatible with 0.4 right now anyway. My patch suggestion should work just fine in v0.6 since tls.connect is just an exported function that doesn't depend on its this === require('tls').

@mikeal

This comment has been minimized.

Show comment Hide comment
@mikeal

mikeal Feb 21, 2013

Owner

request doesn't support node 0.4 anymore.

Owner

mikeal commented Feb 21, 2013

request doesn't support node 0.4 anymore.

@mikeal mikeal closed this Aug 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment