spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

Res.socket is nil on CONNECT #48

Closed igrigorik closed 12 years ago

igrigorik commented 12 years ago

Trying to figure out what's going here.. For some reason, if I issue a "CONNECT" query against the server, the response object does not have the socket? This fails later when we try to create the SYN_REPLY.

debug> n
< ...writeHead: undefined
break in node_modules/spdy/lib/spdy/response.js:34
 32   console.log('...writeHead: ' + this.socket)
 33 
 34   if (this._headerSent) return;
 35   this._headerSent = true;
 36 
debug> 

Frame example:

<   frame: 
<    { type: 'SYN_STREAM',
<      id: 1,
<      associated: 0,
<      priority: 0,
<      fin: false,
<      unidir: false,
<      _offset: 10,
<      headers: 
<       { host: 'google.com',
<         method: 'CONNECT',
<         path: 'google.com:443',
<         url: 'google.com:443',
<         version: 'HTTP/1.1',
<         'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/537.1 (KHTML, like Gecko) Chrome/21.0.1180.0 Safari/537.1' } },
<   writable: true,
<   readable: true }

Any ideas?

eee-c commented 12 years ago

What versions of node and node-spdy are you using?

igrigorik commented 12 years ago

v0.7.10-pre for node.js. Checked out the latest and greatest v3 branch.. Tried with master and others, same behavior.

eee-c commented 12 years ago

What are you using for the upstream app? Node-spdy doesn't establish the request / response objects, but re-uses and decorates them. Still, I can't think why an upstream app wouldn't have a socket associated with it -- especially if it had called writeHead...

igrigorik commented 12 years ago

Tracked it down... Turns out node's handling of upgrade and connect currently exposes a different API: https://github.com/joyent/node/pull/3036

Current API: https://github.com/joyent/node/commit/08a91acd76cd107dc2f3914f9ea7e277bb85206e#L2R56

With that, got my code to work. Annoying, because I have to replicate the SYN_REPLY frame construction in my server, but does the trick. Speaking of which, we need to extend node-spdy to emit connect events -- I'll open a separate pull for that.