diff --git a/lib/api-request-test.js b/lib/api-request-test.js index 5c17114..debe761 100644 --- a/lib/api-request-test.js +++ b/lib/api-request-test.js @@ -3,14 +3,16 @@ const nock = require('nock'); const request = require('request'); describe('api', () => { - const HOST = 'https://example.com', - API_KEY = '123', - USER_AGENT = 'lolbot 0.1'; + const HOST = 'https://example.com'; + const API_KEY = '123'; + const USER_AGENT = 'lolbot 0.1'; + const METHOD = 'get'; + const ENDPOINT = '/whoopie-do'; + const OPTIONS = { endpoint: ENDPOINT }; it('makes requests with configured methods', () => { const apiRequest = api.apiRequest(HOST, API_KEY, USER_AGENT); - const OPTIONS = { endpoint: '/whoopie-do' }, - EXPECTED_OPTIONS = { url: `${HOST}/whoopie-do`, headers: { 'User-Agent': USER_AGENT } }; + const EXPECTED_OPTIONS = { url: `${HOST}${ENDPOINT}`, headers: { 'User-Agent': USER_AGENT } }; ['get', 'put', 'post', 'delete'].forEach(method => { const authSpy = jasmine.createSpy('auth'); const CALLBACK = () => {}; @@ -25,10 +27,7 @@ describe('api', () => { it('calls the given callback with response', (done) => { const apiRequest = api.apiRequest(HOST, API_KEY, USER_AGENT); - const METHOD = 'get', - ENDPOINT = '/whoopie-do', - OPTIONS = { endpoint: ENDPOINT }, - RESPONSE_BODY = { ha: 'ha' }; + const RESPONSE_BODY = { ha: 'ha' }; nock(HOST)[METHOD](ENDPOINT) .matchHeader('User-Agent', USER_AGENT) @@ -46,4 +45,56 @@ describe('api', () => { done(); }); }); + + describe('when the response status is not 200', () => { + describe('and the response JSON is not formatted correctly', () => { + it('returns an appropriate error', (done) => { + const apiRequest = api.apiRequest(HOST, API_KEY, USER_AGENT); + const BAD_JSON_RESPONSE_BODY = { i: 'sorry' }; + + nock(HOST)[METHOD](ENDPOINT) + .reply(400, BAD_JSON_RESPONSE_BODY); + + apiRequest(METHOD, OPTIONS, (err, body) => { + expect(body).toBeUndefined() + expect(err).toEqual(new Error('Unknown error')) + done(); + }); + }); + }); + }); + + describe('when the response is not JSON formatted', () => { + describe('and the response status is 200', () => { + it('does not blow up', (done) => { + const apiRequest = api.apiRequest(HOST, API_KEY, USER_AGENT); + const BAD_RESPONSE_BODY = 'sorry'; + + nock(HOST)[METHOD](ENDPOINT) + .reply(200, BAD_RESPONSE_BODY); + + apiRequest(METHOD, OPTIONS, (err, body) => { + expect(body).toBeUndefined() + expect(err).toEqual(new Error('The response from the server was not valid JSON')) + done(); + }); + }); + }); + + describe('and the response status is not 200', () => { + it('does not blow up', (done) => { + const apiRequest = api.apiRequest(HOST, API_KEY, USER_AGENT); + const BAD_RESPONSE_BODY = 'sorry'; + + nock(HOST)[METHOD](ENDPOINT) + .reply(400, BAD_RESPONSE_BODY); + + apiRequest(METHOD, OPTIONS, (err, body) => { + expect(body).toBeUndefined() + expect(err).toEqual(new Error('The response from the server was not valid JSON')) + done(); + }); + }); + }); + }); }); diff --git a/lib/api-request.js b/lib/api-request.js index a1dbad4..5ff6f04 100644 --- a/lib/api-request.js +++ b/lib/api-request.js @@ -10,9 +10,23 @@ module.exports = { return; } if (response.statusCode >= 200 && response.statusCode < 300) { - cb(null, JSON.parse(body)); + try { + const parsedBody = JSON.parse(body); + cb(null, parsedBody); + } catch (e) { + cb(new Error('The response from the server was not valid JSON')); + } } else { - cb(new Error(JSON.parse(body).error.message)); + try { + const parsedBody = JSON.parse(body); + if (!parsedBody || !parsedBody.error || !!parsedBody.error.message) { + cb(new Error('Unknown error')); + } else { + cb(new Error(parsedBody.error.message)); + } + } catch (e) { + cb(new Error('The response from the server was not valid JSON')); + } } } const requestOptions = {