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

TypeError: Cannot read property 'incoming' of null #252

Open
ben-page opened this issue Mar 16, 2016 · 2 comments
Open

TypeError: Cannot read property 'incoming' of null #252

ben-page opened this issue Mar 16, 2016 · 2 comments

Comments

@ben-page
Copy link

I'm using node-spdy 3.2.3 on node 5.8.0. After pushing a couple files, spdy fails with this error.

TypeError: Cannot read property 'incoming' of null
    at ServerResponse.push (...\node_modules\spdy\lib\spdy\response.js:83:29)

lib\spdy\response.js

exports.push = function push(path, headers, callback) {
  var frame = {
    path: path,
    method: 'GET',
    status: 200,
    host: this.socket.parser.incoming.headers.host, //line 83
    headers: headers.request,
    response: headers.response
  };

  var stream = this.socket._handle._spdyState.stream;
  return stream.pushPromise(frame, callback);
};

I'm calling push() like this:

var stream = res.push(pushUrl, { response: headers });
stream.end(body);

If I step into res.push(), this is the ServerResponse object and socket is a Socket object, but parser is null.

@ben-page
Copy link
Author

The problem is occasionally the ServerResponse is sent (and ended) before the push. After the ServerResponse has ended, the parser is gone (and probably other things, too).

This isn't a bug in the spdy module (although being able to push without depending on a ServerResponse object would be helpful). But, I think this caveat should probably be mentioned in the documentation.

@anandsuresh
Copy link
Contributor

anandsuresh commented Jul 13, 2016

I seem to have a similar problem where in the underlying socket is closed and cleaned up, before the push stream is created. The way I see it, the culprit is this line: https://github.com/indutny/node-spdy/blob/master/lib/spdy/response.js#L83

Instead of depending on the parser to reference the IncomingMessage to grab the hostname, perhaps it would be better to use an alternate way of grabbing the hostname. Looking at https://github.com/indutny/node-spdy/blob/master/lib/spdy/server.js, it seems that a cleaner way to reference the request would be to get the request and response to reference each other (perhaps a weak reference, if needed; not sure of the dynamics there).

Something like the following in https://github.com/indutny/node-spdy/blob/master/lib/spdy/server.js#L229-L238

  req.isSpdy = true;
  req.spdyVersion = handle.getStream().connection.getVersion();
  req.spdyStream = handle.getStream();

  debug('override req/res');
  res.writeHead = spdy.response.writeHead;
  res.end = spdy.response.end;
  res.push = spdy.response.push;
  res.writeContinue = spdy.response.writeContinue;
  res.spdyStream = handle.getStream();

  req.res = res;   // Request gets a reference to its corresponding response stream
  res.req = req;   // Response gets a reference to its corresponding request stream

With this in place, .push() can then change to use this.req.headers.host to identify the host.

Does that seem alright @indutny?

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

No branches or pull requests

2 participants