Throw error when request returns an HTTP status code greater than 400

[carlosvillademor]

Hello @mko,

I am using the library to import some data from Runkeeper. I was having some issues where some of the requests where returning 401 or 406 HTTP status code. I think when using request like on:

request(request_details, function(error, response, body) { 
    var parsed;
    try {
        parsed = JSON.parse(body);
    } catch(e) {
        error = new Error('Body reply is not a valid JSON string.');
        error.runkeeperBody = body;
    } finally {
        callback(error, parsed);
    }
});

The response.statusCode and the error should be check before carrying on to do anything with body.

request(request_details, function(error, response, body) { 
    if(error || response.statusCode !=== 200) {
        return callback(error || 'Status code' + response.statusCode);
    }
    var parsed;
    try {
        parsed = JSON.parse(body);
    } catch(e) {
    error = new Error('Body reply is not a valid JSON string.');
        error.runkeeperBody = body;
    } finally {
        callback(error, parsed);
    }
});

If there is an error or the status code of the response is not 200, we execute the callback with the error. I'll get a pull request for you to merge if you agree this change needs to be integrated.

I was getting an undefined access_token because of the same issue on:

request(request_details, function(error, response, body) {
    callback(error, JSON.parse(body)['access_token']);
});

The response status code was 401 or 406, the callback was called with no error and an undefined body.

[mko]

Thanks for finding this. I've not been actively working with the library recently, but it makes sense to add these checks. Submit the PR, and I'll move forward with a new release on NPM later this week.