Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm reading Professional Node.js: Building Javascript Based Scalable Software by Pedro Teixeira and created a web page responding with www.google.de. Could you take a look at my code and let me know if the implementation is not good?

var request = require('request');
var url = 'http://www.google.de'; // input your url here

// use a timeout value of 10 seconds
var timeoutInMilliseconds = 10 * 10000;
var opts = {
    url : url,
    timeout : timeoutInMilliseconds
};

require('http').createServer(function(req, res) {

    res.writeHead(200, {
        'Content-Type' : 'text/html'
    });

    request(opts, function(err, webRes, body) {
        if (err) {
            console.dir(err);
            return;
        }
        var statusCode = webRes.statusCode;
        console.log('status code: ' + statusCode);
        res.write(body);
    });
}).listen(4000);
share|improve this question

1 Answer 1

up vote 2 down vote accepted

Here are my suggestions:

  • Get rid of the anonymous functions. They are notoriously difficult to test. I am hoping that you have written the test cases. If not, now would be a good time to try writing these tests.
  • There is either a documentation or implementation bug where you want the timeout to be 10s (10000ms) but are actually using a value of 10*10000ms which is 100s.
  • Rather than calling a method directly on require('http'), I'd like it if you import it the way you have imported request.
  • You are always sending back a response code of 200. You should wait till after the request has finished fetching the page and set the status code accordingly unless you have some way of serving the right content (using a cahed copy, for example).

Other than that, I think that you have the actual functionality pretty much nailed down.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.