Skip to content

Commit

Permalink
FRIDGE-461 Add error handling to axios requests (#393)
Browse files Browse the repository at this point in the history 
* Fix typings

* FRIDGE-450 Fix yarn

* FRIDGE-450 Add event

* FRIDGE-450 Remove duplicate interface WorkerEvents

* FRIDGE-450 Address comments to fix types

* FRIDGE-450 Additional fixes

* FRIDGE-450 Upgrade typed-emitter to version 2.1.0

* FRIDGE-461 Add axios error handling to get, post

* FRIDGE-461 Fixes

* FRIDGE-461 Align errors

* FRIDGE-461 Fix tests

* FRIDGE-461 Fix test

* FRIDGE-461 Fix sid

* FRIDGE-461 Fix sid

* FRIDGE-461 Fix sids

Co-authored-by: Jared Szechy <jared.szechy@gmail.com>
  • Loading branch information
@lqkallas @szechyjs
2 people authored and GitHub Enterprise committed Sep 27, 2023
1 parent 9895f04 commit 33fe95f
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 22 deletions.
6 changes: 5 additions & 1 deletion lib/util/Constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ const errors = [
{ name: 'GATEWAY_DISCONNECTED', message: 'Connection to Twilio\'s servers was lost.' },
{ name: 'INVALID_GATEWAY_MESSAGE', message: 'The JSON message received was malformed.' },

{ name: 'TASKROUTER_ERROR', message: 'TaskRouter failed to complete the request.' }
{ name: 'TASKROUTER_ERROR', message: 'TaskRouter failed to complete the request.' },
{ name: 'REQUEST_INVALID', message: 'The provided request is invalid.' },
{ name: 'NETWORK_ERROR', message: 'No response from the server.' },
{ name: 'SERVER_ERROR', message: 'Server has experienced an issue performing the request.' },
{ name: 'UNKNOWN_ERROR', message: 'An unknown error has occurred.' }
];

export const twilioErrors = errors.reduce((errs, error) => {
Expand Down
34 changes: 33 additions & 1 deletion lib/util/Request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import _ from 'lodash';
import * as axios from 'axios';
import * as https from 'https';
import Configuration from './Configuration';
import { DEFAULT_HTTP_TIMEOUT } from './Constants';
import Logger from './Logger';
import { DEFAULT_HTTP_TIMEOUT, twilioErrors as Errors } from './Constants';

const httpMethods = {
GET: 'GET',
Expand Down Expand Up @@ -49,6 +50,8 @@ export default class Request {
},
httpsAgent
});

this._log = new Logger(`Request-${this._config.getLogIdentifier()}`, this._config._logLevel);
}


Expand Down Expand Up @@ -86,6 +89,8 @@ export default class Request {
headers
}).then(response => {
return Promise.resolve(response.data.payload);
}).catch((error) => {
this.handleError(error);
});
}

Expand Down Expand Up @@ -117,6 +122,8 @@ export default class Request {
}
}).then(response => {
return Promise.resolve(response.data.payload);
}).catch((error) => {
this.handleError(error);
});
}

Expand All @@ -135,4 +142,29 @@ export default class Request {
token: this._config.token
});
}

/**
* @private
* @param {Error} error
* @return {void}
*/
handleError(error) {
if (error.response) {
// Server responded with an error status code (e.g. 4xx or 5xx)
if (error.response.status < 500) {
this._log.error('Request failed with ', error.response.status, error.response.data.message);
throw Errors.REQUEST_INVALID.clone(`Request failed with status code ${error.response.status}. ${error.response.data.message}`);
}
this._log.error('Server Error:', error.response.status, error.response.data.message);
throw Errors.SERVER_ERROR.clone(`Server responded with status code ${error.response.status}. ${error.response.data.message}`);
} else if (error.request) {
// No response received (e.g. network issue, timeout)
this._log.error('Network Error:', error.message);
throw Errors.NETWORK_ERROR.clone(`Network error has occurred. ${error.message}`);
} else {
// Setting up the request errored
this._log.error('Error:', error.message);
throw Errors.UNKNOWN_ERROR.clone(`Error: ${error.message}`);
}
}
}
7 changes: 1 addition & 6 deletions test/integration/spec/OutgoingTransfer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,7 @@ describe('OutgoingTransfer', () => {

// canceling the same outgoing task again
return transferredTask.transfers.outgoing.cancel().catch(err => {
assert.equal(err.response.status, 400,
envTwilio.getErrorMessage('400 not received when canceling same task twice', credentials.accountSid, credentials.multiTaskBobSid));

assert.equal(err.response.statusText, `Transfer ${canceledTransfer.sid} is already canceled . Cannot cancel transfer.`,
envTwilio.getErrorMessage('400 error message content mismatch', credentials.accountSid, credentials.multiTaskBobSid));

assert.equal(err.message, `Request failed with status code 400. Transfer ${canceledTransfer.sid} is already canceled . Cannot cancel transfer.`, envTwilio.getErrorMessage('400 not received when canceling same task twice', credentials.accountSid, credentials.multiTaskBobSid));
done();
});
});
Expand Down
175 changes: 161 additions & 14 deletions test/unit/spec/util/Request.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,19 @@ describe('Request', () => {
sandbox.restore();
});

const setUpSuccessfulResponse = (stub) => {
const mockResponse = {
data: {
payload: 'someData'
}
};
stub.resolves(Promise.resolve(mockResponse));
};

const setUpErrorResponse = (stub, error) => {
stub.rejects(error);
};

describe('constructor', () => {
it('should throw an error if configuration is invalid', () => {
(() => {
Expand All @@ -39,20 +52,15 @@ describe('Request', () => {
beforeEach(() => {
request = new Request(config);

const mockResponse = {
data: {
payload: 'someData'
}
};

// set up the stub to check the headers and also mock the response, instead of using axios
stub = sandbox.stub(request._postClient, 'post').resolves(Promise.resolve(mockResponse));
stub = sandbox.stub(request._postClient, 'post');

// build the expected request body
requestBody = request.buildRequest('POST', requestURL, requestParams);
});

it('adds default headers', () => {
setUpSuccessfulResponse(stub);
return request.post(requestURL, requestParams, API_V1).then(() => {
sinon.assert.calledWith(stub, config.EB_SERVER, requestBody, {
headers: {
Expand All @@ -65,6 +73,7 @@ describe('Request', () => {
// eslint-disable-next-line no-warning-comments
// TODO FLEXSDK-2255: unskip this test once the versioning bug is fixed
it.skip('adds object version to If-Match header', () => {
setUpSuccessfulResponse(stub);
const version = '1';

return request.post(requestURL, requestParams, API_V1, version).then(() => {
Expand All @@ -77,6 +86,76 @@ describe('Request', () => {
});
});

it('should throw request invalid error', async() => {
const error = new Error();
error.response = {
status: 400,
data: {
message: 'Invalid request'
},
};

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Request failed with status code 400. Invalid request');
}
});

it('should throw server error', async() => {
const error = new Error();
error.response = {
status: 500,
data:
{
message: 'Unexpected server error'
}
};

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Server responded with status code 500. Unexpected server error');
}
});

it('should throw network error', async() => {
const error = new Error('Timeout');
error.request = true;

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Network error has occurred. Timeout');
}
});

it('should throw unknown error', async() => {
const error = new Error('Something unexpected happened!');

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Error: Something unexpected happened!');
}
});

it('should throw an error if required parameters are missing', () => {
(() => {
request.post();
Expand All @@ -100,20 +179,15 @@ describe('Request', () => {
beforeEach(() => {
request = new Request(config);

const mockResponse = {
data: {
payload: 'someData'
}
};

// set up the stub to check the headers and also mock the response, instead of using axios
stub = sandbox.stub(request._postClient, 'post').resolves(Promise.resolve(mockResponse));
stub = sandbox.stub(request._postClient, 'post');

// build the expected request body
requestBody = request.buildRequest('GET', requestURL, requestParams);
});

it('add default headers', () => {
setUpSuccessfulResponse(stub);
return request.get(requestURL, API_V1, requestParams).then(() => {
sinon.assert.calledWith(stub, config.EB_SERVER, requestBody, {
headers: {
Expand All @@ -124,6 +198,7 @@ describe('Request', () => {
});

it('add empty params object if no params given', () => {
setUpSuccessfulResponse(stub);
requestBody = request.buildRequest('GET', requestURL, {});
return request.get(requestURL, API_V1).then(() => {
sinon.assert.calledWith(stub, config.EB_SERVER, requestBody, {
Expand All @@ -134,7 +209,79 @@ describe('Request', () => {
});
});

it('should throw request invalid error', async() => {
const error = new Error();
error.response = {
status: 400,
data:
{
message: 'Invalid request'
}
};

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Request failed with status code 400. Invalid request');
}
});

it('should throw server error', async() => {
const error = new Error();
error.response = {
status: 500,
data:
{
message: 'Unexpected server error'
}
};

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Server responded with status code 500. Unexpected server error');
}
});

it('should throw network error', async() => {
const error = new Error('Timeout');
error.request = true;

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Network error has occurred. Timeout');
}
});

it('should throw unknown error', async() => {
const error = new Error('Something unexpected happened!');

setUpErrorResponse(stub, error);
const version = '1';

try {
await request.post(requestURL, requestParams, API_V1, version);
throw new Error('Expected an error to be thrown');
} catch (thrownError) {
thrownError.message.should.equal('Error: Something unexpected happened!');
}
});

it('should throw an error if required parameters are missing', () => {
setUpSuccessfulResponse(stub);
(() => {
request.get();
}).should.throw(/<string>url is a required parameter/);
Expand Down

0 comments on commit 33fe95f

Please sign in to comment.