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

Cannot read property 'statusCode' of null #285

Closed
Selvatico opened this issue Dec 25, 2014 · 19 comments · Fixed by #290
Closed

Cannot read property 'statusCode' of null #285

Selvatico opened this issue Dec 25, 2014 · 19 comments · Fixed by #290
Assignees
Labels
bug
Milestone

Comments

@Selvatico
Copy link

@Selvatico Selvatico commented Dec 25, 2014

This error happens on small test server and after a lot of quick page refreshes in browser.

TypeError: Cannot read property 'statusCode' of null
    at internals.NetworkMonitor._onResponse (/Users/****/Documents/Projects/***/node_modules/good/lib/network.js:40:38)
    at EventEmitter.emit (events.js:95:17)
    at handler (/Users/****/Documents/Projects/***/node_modules/hapi/node_modules/kilt/lib/index.js:54:44)
    at EventEmitter.emit (events.js:117:20)
    at internals.Request._finalize (/Users/****/Documents/Projects/***/node_modules/hapi/lib/request.js:378:21)
    at internals.Request._reply (/Users/****/Documents/Projects/***/node_modules/hapi/lib/request.js:344:21)
    at /Users/****/Documents/Projects/***/node_modules/hapi/lib/request.js:325:18
    at done (/Users/****/Documents/Projects/***/node_modules/hapi/node_modules/items/lib/index.js:22:21)
    at /Users/****/Documents/Projects/***/node_modules/hapi/lib/request.js:314:24
    at iterate (/Users/****/Documents/Projects/***/node_modules/hapi/node_modules/items/lib/index.js:35:13)
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Dec 26, 2014

Can you provide some code that can reproduce this issue?

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 6, 2015

@Selvatico can you provide any additional information on this issue?

@chulkilee

This comment has been minimized.

Copy link

@chulkilee chulkilee commented Jan 6, 2015

It happened to me at the last few requests when running wrk.

I suspect the request is closed before reply function is invoked... I put console.log here and output was following:

{ [Error: Already closed]
  data: undefined,
  isBoom: true,
  output:
   { statusCode: 500,
     payload:
      { statusCode: 500,
        error: 'Internal Server Error',
        message: 'An internal server error occurred' },
     headers: {} },
  reformat: [Function] }

"Already closed" happens when the request is closed before reply function for the request is called - see this comment.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Jan 7, 2015

I'm having almost the same error.

I'm trying to redirect all my http calls to https.
I'm using this code:

server.ext('onRequest', function (request, reply) {
    if (request.connection.info.protocol === 'http') {
        return reply().redirect('https://' + request.headers.host + request.url.path).permanent();
    }

    reply.continue();
});

Although this works, traffic get's redirected, the server still throws a 500

Debug: internal, implementation, error
    TypeError: Cannot read property 'statusCodes' of undefined
    at internals.NetworkMonitor._onResponse (c:\Users\***\node_modules\good\lib\network.js:50:25)
    at emit (events.js:95:17)
    at handler (c:\Users\***\node_modules\hapi\node_modules\kilt\lib\index.js:54:44)
    at emit (events.js:117:20)
    at internals.Request._finalize (c:\Users\***\node_modules\hapi\lib\request.js:378:21)
    at c:\Users***\node_modules\hapi\lib\request.js:369:25
    at ServerResponse.end (c:\Users\***\node_modules\hapi\lib\transmit.js:289:13)
    at ServerResponse.g (events.js:180:16)
    at ServerResponse.emit (events.js:117:20)
    at ServerResponse.OutgoingMessage._finish (http.js:1021:8)

Any idea what might cause this? Or what I'm doing wrong?

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 7, 2015

Good news: I think I figured out what the issue is.
Bad news: I'm not sure how to fix it without cheating. I'm working on it now.

Try moving that extension point registration AFTER calling server.register. In other words, make sure good has registered all of it's extension points first, then add yours; see if that makes a difference.

@arb arb added the bug label Jan 7, 2015
@arb arb added this to the 5.1.1 milestone Jan 7, 2015
@arb arb self-assigned this Jan 7, 2015
@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Jan 7, 2015

On first tests that might seem to solve it, thanks!

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 7, 2015

Are you trying to redirect all http requests to https? If so, I don't think this is the best way to solve this problem. The way you are doing this does work, but will likely cause issues with other plugins or code that use extension points. It just so happens to be manifesting itself in good.

According to this issue you should have two connections (one for http and one for https), and just have a catch all route on http and redirect there, rather than using an extension point.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Jan 7, 2015

Moving the registration of the extension after the one of good worked.
I have 2 connections, 1 for http and another for https which has the tls options. But how do I register routes to just 1 of those connections? Aren't routes server binded instead of connection binded?

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 7, 2015

You can give each connection a label and then use server.select to get the specific connection and then ads routes to it.

http://hapijs.com/tutorials/plugins#serverselect

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Jan 8, 2015

I will try that thanks! An idea on a solution? Since it would be weird to require that good is required before any server.ext() is called.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Jan 8, 2015

I implemented it with the server.select() and it works great. Thanks for all the help.

For reference you can define a catch all http and redirect to https route like this:

{
    method: '*',
    path: '/{p*}',
    handler: function (request, reply) {

        // redirect all http trafic to https
        return reply().redirect('https://' + request.headers.host + request.url.path).permanent();
    }
}
@chulkilee

This comment has been minimized.

Copy link

@chulkilee chulkilee commented Jan 9, 2015

I'm still seeing this problem on hapi 8.1.0 and good 5.1.1. @geek @arb can we reopen this issue?

Debug: internal, implementation, error
    TypeError: Cannot read property 'statusCode' of null
    at internals.NetworkMonitor._onResponse (/somewhere/node_modules/good/lib/network.js:40:38)
    at emit (events.js:95:17)
    at handler (/somewhere/node_modules/hapi/node_modules/kilt/lib/index.js:54:44)
    at emit (events.js:95:17)
    at internals.Request._finalize (/somewhere/node_modules/hapi/lib/request.js:378:21)
    at internals.Request._reply (/somewhere/node_modules/hapi/lib/request.js:344:21)
    at /somewhere/node_modules/hapi/lib/request.js:325:18
    at done (/somewhere/node_modules/hapi/node_modules/items/lib/index.js:22:21)
    at /somewhere/node_modules/hapi/lib/request.js:314:24
    at iterate (/somewhere/node_modules/hapi/node_modules/items/lib/index.js:35:13)
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 9, 2015

Can you post any code? Did you try changing around the order your extension methods are handled?

It is likely an issue in one of your extension methods that is creating this error condition. My guess is you are replying early somewhere and causing the issue.

@chulkilee

This comment has been minimized.

Copy link

@chulkilee chulkilee commented Jan 9, 2015

I'm not using server.extdo not use extension. Here is how I call server.register.

var Hapi = require('hapi');
var server = new Hapi.Server();

function registerAuth(callback) {
  server.register(require('hapi-auth-bearer-token'), function(err) {
    if (err) { throw err; }
    server.auth.strategy('token', 'bearer-access-token', true, {
      allowQueryToken: false,
      allowMultipleHeaders: false,
      tokenType: 'Token',
      validateFunc: function(token, cb) {
        if (!token) { return cb(null, false, { token: token }); }

        var parts = token.split('=');
        if (parts[0] !== 'token') { return cb(null, false, {}); }

        auth.validateToken(parts[1], function(err, data) {
          if (err) { cb(null, false, {}); }
          else { cb(null, true, { data: data}); }
        });
      }
    });
    callback();
  });
}

function registerLogging(callback) {
  var options = {
    opsInterval: 60000,
    reporters: [
      {
        reporter: require('good-console'),
        args: [{
          error: '*',
          log: '*',
          ops: '*',
          request: '*',
          response: '*'
        }]
      }
    ]
  };
  server.register(
    { register: require('good'), options: options },
    function (err) {
      if (err) { throw err; }
      callback();
    }
  );
}

server.connection({
  host: config.host,
  port: config.port,
  routes: { cors: true }
});

server.route(require('./src/routes'));

registerLogging(function() {
  registerAuth(function() {
    server.start(function() {
      server.log('start', 'server started at: ' + server.info.uri);
    });
  });
});
@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 9, 2015

What about in your handler functions?

@chulkilee

This comment has been minimized.

Copy link

@chulkilee chulkilee commented Jan 9, 2015

Looks like a hapi bug. response event should include response but it didn't in the case. created an issue on hapi - hapijs/hapi#2344

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 9, 2015

I don't think the open hapi issue is related here. The problem isn't that request.response is null, the problem is that whatever you're doing (probably short-circuting the request lifecycle somehow) doesn't invoke the response-internal code.

That means that this._requests[port] can be undefined. Which is what is causing the issue.

@chulkilee

This comment has been minimized.

Copy link

@chulkilee chulkilee commented Jan 9, 2015

I do think the issue is related this "symptom". #290 fixes just one situation this can happen.

I could reproduce it with hapi test code - see test at hapijs/hapi#2345. If you change the event time from tail to response, you will get the empty response (and failing test).

Why do you think this._requests[port] is causing the issue? From my stack trace above, it is happening because response is null. If you think request.response is set to null somehow incorrectly, then it's hapi bug, not good bug. That's what I'm saying here.

Please comment on the hapi issue if needed.

@arb

This comment has been minimized.

Copy link
Contributor

@arb arb commented Jan 10, 2015

My bad. There are two related issues going on in this one. You're right, YOUR stack indicates response is null. The other stack was related to my point above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.