http module error out while sending empty response body

[chetan988]

Please consider below test case:

file test.js

var express = require('express');
var app = express();

app.get('/test_1', function (req, res) {
  var options = {
    'url'    : 'http://localhost:3000/test_2',
    'timeout': 10
  };
  require('request').get(options,function(err,response,data){
     response = response || {};
     data = data || {};
     if (err) {
        console.log("some error " + err);
     }
     res.status(response.statusCode).send(data);
  });
});

app.get('/test_2', function(req,res){
  setTimeout(function(){res.status(200).send('ok')},100);
  //res.status(200).send('ok');
});

var server = app.listen(3000, function () {
  console.log('Test app listening on 3000');
});

If I will run this program and hit http://localhost:3000/test_1 I am getting below error:

some error Error: ETIMEDOUT
TypeError: Cannot call method 'toString' of undefined
    at ServerResponse.writeHead (http.js:1183:45)
    at ServerResponse.res.writeHead (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/node_modules/express/node_modules/connect/lib/patch.js:85:22)
    at ServerResponse._implicitHeader (http.js:1134:8)
    at ServerResponse.OutgoingMessage.end (http.js:919:10)
    at ServerResponse.res.send (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/node_modules/express/lib/response.js:159:8)
    at ServerResponse.res.json (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/node_modules/express/lib/response.js:201:15)
    at ServerResponse.res.send (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/node_modules/express/lib/response.js:125:21)
    at Request._callback (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/test.js:15:38)
    at self.callback (/Users/chetan.das/Documents/workspace/working_dir/tfs_vts/consumer_app/node_modules/request/request.js:236:22)
    at Request.emit (events.js:95:17)

Possible reason:

request time out is 10ms where as /test_2 return the response after 100ms so request module stop waiting for http response and return to call back with err = " ETIMEDOUT", response and data = undefined.
To stop breaking the code I have added

response = response || {};
data = data || {};

But code is breaking while sending the body.

Work around:

var express = require('express');
var app = express();

app.get('/test_1', function (req, res) {
  var options = {
    'url'    : 'http://localhost:3000/test_2',
    'timeout': 10
  };
  require('request').get(options,function(err,response,data){
     response = response || {"statusCode" : 500};
     data = data || {};
     if (err) {
    console.log("some error " + err);
     }
     res.status(response.statusCode).send(data);
  });
});

app.get('/test_2', function(req,res){
  setTimeout(function(){res.status(200).send('ok')},100);
  //res.status(200).send('ok');
}); 

var server = app.listen(3000, function () {
  console.log('Test app listening on 3000');
});

Note change : response = response || {"statusCode" : 500};
With this change code is running fine.

Please let me know if I am doing anything wrong here or if there is any issue in http module.

Thanks

[chetan988]

I raised the same issue in node community please checkout the detail here : nodejs/node#3269 (comment)

[dougwilson]

Hi! This is indeed a Node.js core issue. You can find the following server will have the same issue and does not use Express at all:

var http = require('http');

var server = http.createServer(function (req, res) {
  switch (req.url) {
    case '/test_1':
      var options = {
        'url'    : 'http://localhost:3000/test_2',
        'timeout': 10
      };
      require('request').get(options,function(err,response,data){
        response = response || {};
         data = data || {};
         if (err) {
            console.log("some error " + err);
         }
         res.statusCode = response.statusCode;
         res.end(data);
       });
      break;

    case '/test_2':
      setTimeout(function(){res.end('ok');},100);
      break;
  }
});

server.listen(3000, function () {
  console.log('Test app listening on 3000');
});

You're basically encountering two problems:

1. A bug in Node.js core when you set res.statusCode property to an object (like null or undefined that does not have a toString() method--Node.js core should probably be using the String() function to stringify the value instead.
2. You are not using the correct error-handling inside a callback, as per Node.js standards. Basically, if the first argument (error) is populated with a value in the callback, you MUST ignore all other values and never attempt to use them. Your code continues to try and use the values even when error is populated. This would be a bug in your code above.

I hope this helps!

[dougwilson]

And as an additional followup, here is the correct way to write your example in Express:

var express = require('express');
var app = express();

app.get('/test_1', function (req, res, next) {
  var options = {
    'url'    : 'http://localhost:3000/test_2',
    'timeout': 10
  };
  require('request').get(options,function(err,response,data){
     if (err) return next(err); // this is the key
     res.status(response.statusCode).send(data);
  });
});

app.get('/test_2', function(req,res){
  setTimeout(function(){res.status(200).send('ok')},100);
});

var server = app.listen(3000, function () {
  console.log('Test app listening on 3000');
});

[chetan988]

Thank you dougwilson for your express reply.

As you pointed out we should not proceed in the callback if there is a error. But in our production env, we are using node as a proxy server between app and backend engine. So in case of error we have to send some error response to app to let app know a problem occurred in server but for some api we do not have a default error response so we tried to send empty response and body to app but unfortunately node started crashing.
Anyway we have our workaround to move forward with.

Thanks again for the help. Have a good day :)

[dougwilson]

So in case of error we have to send some error response to app to let app know a problem occurred in server but for some api we do not have a default error response so we tried to send empty response and body to app but unfortunately node started crashing.

Gotcha. By default next(error) will cause Express to actually send a 500 response for you, so I'm not sure how that's not working for you. In addition, the app is crashing due to a bug in Node.js core, as explained in #2775 (comment) if you want to file a bug report on Node.js core.

[chetan988]

Yes, next(error) is working as expected. I actually did not tried that example, sorry for that.