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

[Trott]

I may just be misunderstanding your question but I don't see anything in the behavior you describe that suggests there is a problem with the http module.

If you want someone to review your code above to suggest improvements or find possible bugs, you might be better served filing the issue in https://github.com/nodejs/help/issues. If discussion there reveals that there is a bug in http after all, you can always re-open this issue or open another one.

I'm going to close this issue, but if you think I'm just misunderstanding what you're trying to ask, feel free to re-open.

[chetan988]

Thank you Trott for the reply.
The above is a test case for the issue we are facing in our production.
Let me explain our scenario, there is a http node proxy server between app and our backend engines and we wrote a wrapper using request module to call proper backend engines and send the response to app. So in case of timeout happen in the backend engine (as in the test case while calling test_2) we tried sending empty body and response to app (app will validate if body is empty then show default err ) as we have to send the app something in case of error (return on error will make the app to hang ). But due to empty body and response nodejs is crashing.

Hope I am clear with my issue .

[Trott]

If I'm understanding what correctly, the problem is that when err is ETIMEOUT, response has no statusCode property and so the app crashes when you run res.status(response.statusCode).

A few things:

• Your code is using the request module from npm, so whether that is expected behavior is probably better to ask in the request module repo.
• I don't know if this is documented as expected behavior by request, but it sure is the behavior I would expect. The general idiom in Node is you get either an error object in your callback or the rest of the stuff the callback gets. The sample code in the request npm page seems to work that way:

    if (!error && response.statusCode == 200) {

If a response.statusCode were expected to be there regardless of whether there was an error or not, then the check for !error would not be necessary.

[chetan988]

res.status(response.statusCode).send() will definitely crash if response is undefined so response = response || {} is the workaround for that and the code is breaking because we are trying to call res.status(null).send({}).

as the status field is undefined node is breaking.
Should it be handled or it is expected?

[Trott]

I'm sorry but this is not the correct forum for this. res.status() is a part of Express and not Node core. The answer is whatever the Express developers intended. You can try one of the many resources listed on the ExpressJS Community page.

[chetan988]

Thank you very much for your help.

[chetan988]

As per express community , this looks like a nodejs issue. expressjs/express#2775 (comment)

Please comment.

[bnoordhuis]

You mean Doug's comment?

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.

Personally, I'd say it's a case of GIGO.

[chetan988]

Yes, I agree .
We learned it in hard way.

Thanks