Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Standardized Error Messages returned where possible #108

Merged
merged 2 commits into from
May 11, 2022
Merged

Standardized Error Messages returned where possible #108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00a0cc2
#106: standardizes error messages returned
May 7, 2022
9a7ff7c
#106: Error handling in util
May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/auth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,17 @@ module.exports = class Auth {
}
return this.getAccessToken(forceRefresh, remainingAttempts);
} else {
// Expired Error
if (ex?.response?.data?.message) {
ex.message = ex.response.data.message;
ex.code = ex.response.data.errorcode;
}
// Unauthenticated
else if (ex?.response?.data?.error_description) {
ex.message = ex.response.data.error_description;
ex.code = ex.response.data.error;
}
JoernBerkefeld marked this conversation as resolved.
Show resolved Hide resolved

throw ex;
}
}
Expand All @@ -180,6 +191,7 @@ module.exports = class Auth {

/**
* Helper to get back list of scopes supported by SDK
*
* @returns {Array[String]} array of potential scopes
*/
getSupportedScopes() {
Expand Down
4 changes: 4 additions & 0 deletions lib/rest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ module.exports = class Rest {
//only retry once on refresh since there should be no reason for this token to be invalid
return this._apiRequest(requestOptions, 1);
} else {
if (ex?.response?.data?.message) {
ex.message = ex.response.data.message;
ex.code = ex.response.data.errorcode;
}
JoernBerkefeld marked this conversation as resolved.
Show resolved Hide resolved
throw ex;
}
}
Expand Down
7 changes: 3 additions & 4 deletions lib/soap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ module.exports = class Soap {
// need to wait as it may error
return await _parseResponse(response, options.key);
} catch (ex) {
if (ex.errorMessage === 'Token Expired' && remainingAttempts) {
if (ex.message === 'Token Expired' && remainingAttempts) {
// force refresh due to url related issue
await this.auth.getAccessToken(true);
// set to no more retries as after token refresh it should always work
Expand Down Expand Up @@ -526,10 +526,9 @@ class SOAPError extends Error {
}
JoernBerkefeld marked this conversation as resolved.
Show resolved Hide resolved
// Payload Error
else if (soapBody && soapBody['soap:Fault']) {
super('Error in SOAP Payload');
const fault = soapBody['soap:Fault'];
this.errorCode = fault.faultcode;
this.errorMessage = fault.faultstring;
super(fault.faultstring);
this.code = fault.faultcode;
}
// Request Error
else if (response.status > 299) {
Expand Down
4 changes: 1 addition & 3 deletions lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
* @param {object} obj Object to check
* @returns {boolean} true if is simple Object
*/
module.exports.isObject = (obj) =>
Object.prototype.toString.call(obj) === '[object Object]' ||
Object.prototype.toString.call(obj) === '[object Array]';
module.exports.isObject = (obj) => Object.prototype.toString.call(obj) === '[object Object]';

/**
* Method to check if Object passed is a valid payload for API calls
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"main": "./lib/index.js",
"scripts": {
"test": "nyc --reporter=text mocha",
"lint": "eslint ./lib",
"lint:fix": "eslint ./lib --fix"
"lint": "eslint ./lib && eslint ./test",
"lint:fix": "eslint ./lib --fix && eslint ./test --fix"
},
"repository": {
"type": "git",
Expand Down
38 changes: 19 additions & 19 deletions test/auth.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ const { defaultSdk, mock } = require('./utils.js');
const resources = require('./resources/auth.json');
const { isConnectionError } = require('../lib/util');

describe('auth', () => {
afterEach(() => {
describe('auth', function () {
JoernBerkefeld marked this conversation as resolved.
Show resolved Hide resolved
afterEach(function () {
mock.reset();
});
it('should return an auth payload with token', async () => {
it('should return an auth payload with token', async function () {
//given
const { success } = resources;

Expand All @@ -20,7 +20,7 @@ describe('auth', () => {
assert.lengthOf(mock.history.post, 1);
return;
});
it('should return an auth payload with previous token and one request', async () => {
it('should return an auth payload with previous token and one request', async function () {
//given
const { success } = resources;
mock.onPost(success.url).reply(success.status, success.response);
Expand All @@ -33,7 +33,7 @@ describe('auth', () => {
assert.lengthOf(mock.history.post, 1);
return;
});
it('should return an unauthorized error', async () => {
it('should return an unauthorized error', async function () {
//given
const { unauthorized } = resources;
mock.onPost(unauthorized.url).reply(unauthorized.status, unauthorized.response);
Expand All @@ -49,10 +49,10 @@ describe('auth', () => {

return;
});
it('should return an incorrect account_id error', async () => {
it('should return an incorrect account_id error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: 'XXXXX',
client_secret: 'YYYYYY',
auth_url: 'https://mct0l7nxfq2r988t1kxfy8sc47ma.auth.marketingcloudapis.com/',
Expand All @@ -68,10 +68,10 @@ describe('auth', () => {
}
return;
});
it('should return an incorrect auth_url error', async () => {
it('should return an incorrect auth_url error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: 'XXXXX',
client_secret: 'YYYYYY',
auth_url: 'https://x.auth.marketingcloudapis.com/',
Expand All @@ -87,10 +87,10 @@ describe('auth', () => {
}
return;
});
it('should return an incorrect client_id error', async () => {
it('should return an incorrect client_id error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: '',
client_secret: 'YYYYYY',
auth_url: 'https://mct0l7nxfq2r988t1kxfy8sc47ma.auth.marketingcloudapis.com/',
Expand All @@ -103,10 +103,10 @@ describe('auth', () => {
}
return;
});
it('should return an incorrect client_key error', async () => {
it('should return an incorrect client_key error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: 'XXXXX',
client_secret: '',
auth_url: 'https://mct0l7nxfq2r988t1kxfy8sc47ma.auth.marketingcloudapis.com/',
Expand All @@ -119,10 +119,10 @@ describe('auth', () => {
}
return;
});
it('should return an invalid scope error', async () => {
it('should return an invalid scope error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: 'XXXXX',
client_secret: 'YYYYYY',
auth_url: 'https://mct0l7nxfq2r988t1kxfy8sc47ma.auth.marketingcloudapis.com/',
Expand All @@ -136,10 +136,10 @@ describe('auth', () => {
}
return;
});
it('should return an invalid scope type error', async () => {
it('should return an invalid scope type error', async function () {
try {
//given
sfmc = new SDK({
new SDK({
client_id: 'XXXXX',
client_secret: 'YYYYYY',
auth_url: 'https://mct0l7nxfq2r988t1kxfy8sc47ma.auth.marketingcloudapis.com/',
Expand All @@ -154,7 +154,7 @@ describe('auth', () => {
return;
});

it('RETRY: should return an success, after a connection issues', async () => {
it('RETRY: should return an success, after a connection issues', async function () {
//given
const { success } = resources;

Expand All @@ -169,7 +169,7 @@ describe('auth', () => {
assert.lengthOf(mock.history.post, 2);
return;
});
it('FAILED RETRY: should return an error, after multiple connection issues', async () => {
it('FAILED RETRY: should return an error, after multiple connection issues', async function () {
//given
const { success } = resources;

Expand Down
38 changes: 19 additions & 19 deletions test/rest.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ const resources = require('./resources/rest.json');
const authResources = require('./resources/auth.json');
const { isConnectionError } = require('../lib/util');

describe('rest', () => {
beforeEach(() => {
describe('rest', function () {
beforeEach(function () {
mock.onPost(authResources.success.url).reply(
authResources.success.status,
authResources.success.response
);
});
afterEach(() => {
afterEach(function () {
mock.reset();
});
it('GET Bulk: should return 6 journey items', async () => {
it('GET Bulk: should return 6 journey items', async function () {
//given
const { journeysPage1, journeysPage2 } = resources;
mock.onGet(journeysPage1.url).reply(journeysPage1.status, journeysPage1.response);
Expand All @@ -28,7 +28,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.get, 2);
return;
});
it('GET: should return 5 journey items', async () => {
it('GET: should return 5 journey items', async function () {
//given
const { journeysPage1 } = resources;
mock.onGet(journeysPage1.url).reply(journeysPage1.status, journeysPage1.response);
Expand All @@ -42,7 +42,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.get, 1);
return;
});
it('GETCOLLECTION: should return 2 identical payloads', async () => {
it('GETCOLLECTION: should return 2 identical payloads', async function () {
//given
const { journeysPage1 } = resources;
mock.onGet(journeysPage1.url).reply(journeysPage1.status, journeysPage1.response);
Expand All @@ -58,7 +58,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.get, 2);
return;
});
it('POST: should create Event Definition', async () => {
it('POST: should create Event Definition', async function () {
//given
const { eventcreate } = resources;
mock.onPost(eventcreate.url).reply(eventcreate.status, eventcreate.response);
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.post, 2);
return;
});
it('POST: should add an entry to a Data Extension', async () => {
it('POST: should add an entry to a Data Extension', async function () {
//given
const { dataExtensionUpsert } = resources;
mock.onPost(dataExtensionUpsert.url).reply(
Expand All @@ -107,7 +107,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.post, 2);
return;
});
it('PUT: should update Event Definition', async () => {
it('PUT: should update Event Definition', async function () {
//given
const { eventupdate } = resources;
mock.onPut(eventupdate.url).reply(eventupdate.status, eventupdate.response);
Expand All @@ -130,7 +130,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.put, 1);
return;
});
it('PATCH: should update Contact', async () => {
it('PATCH: should update Contact', async function () {
//given
const { contactPatch } = resources;
mock.onPatch(contactPatch.url).reply(contactPatch.status, contactPatch.response);
Expand Down Expand Up @@ -180,7 +180,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.patch, 1);
return;
});
it('DELETE: should delete Campaign', async () => {
it('DELETE: should delete Campaign', async function () {
//given
const { campaignDelete } = resources;
mock.onDelete(campaignDelete.url).reply(campaignDelete.status, campaignDelete.response);
Expand All @@ -192,7 +192,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.delete, 1);
return;
});
it('should retry auth one time on first failure then work', async () => {
it('should retry auth one time on first failure then work', async function () {
//given
mock.reset(); // needed to avoid before hook being used
const { expired, success } = authResources;
Expand All @@ -211,7 +211,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.delete, 1);
return;
});
it('should retry auth one time on first failure then fail', async () => {
it('should retry auth one time on first failure then fail', async function () {
//given
mock.reset(); // needed to avoid before hook being used
const { unauthorized } = authResources;
Expand All @@ -230,7 +230,7 @@ describe('rest', () => {
}
return;
});
it('should fail to delete campaign', async () => {
it('should fail to delete campaign', async function () {
//given
const { campaignFailed } = resources;
mock.onDelete(campaignFailed.url).reply(campaignFailed.status, campaignFailed.response);
Expand All @@ -249,7 +249,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.delete, 1);
return;
});
it('RETRY: should return 5 journey items, after a connection error', async () => {
it('RETRY: should return 5 journey items, after a connection error', async function () {
//given
const { journeysPage1 } = resources;
mock.onGet(journeysPage1.url)
Expand All @@ -266,7 +266,7 @@ describe('rest', () => {
assert.lengthOf(mock.history.get, 2);
return;
});
it('FAILED RETRY: should return error, after 2 connection timeout errors', async () => {
it('FAILED RETRY: should return error, after 2 connection timeout errors', async function () {
//given
const { journeysPage1 } = resources;
mock.onGet(journeysPage1.url).timeout();
Expand All @@ -283,11 +283,11 @@ describe('rest', () => {

return;
});
it('FAILED RETRY: should return error, after 2 ECONNRESET errors', async () => {
it('FAILED RETRY: should return error, after 2 ECONNRESET errors', async function () {
//given
const { journeysPage1 } = resources;

mock.onGet(journeysPage1.url).reply((res) => {
mock.onGet(journeysPage1.url).reply(() => {
const connectionError = new Error();
connectionError.code = 'ECONNRESET';
throw connectionError;
Expand All @@ -305,7 +305,7 @@ describe('rest', () => {

return;
});
it('LogRequest & Response: should run middleware for logging ', async () => {
it('LogRequest & Response: should run middleware for logging ', async function () {
//given
const { journeysPage1 } = resources;
mock.onGet(journeysPage1.url).reply(journeysPage1.status, journeysPage1.response);
Expand Down
Loading