Skip to content

Commit 64d094d

Browse files
tot-ratot-ra
authored
Merge pull request #51 from pipedrive/genericresponse_err_handling_fix
genericResponse error handling improvement
2 parents 6d9b323 + 4d76cfa commit 64d094d

File tree

2 files changed

+185
-21
lines changed

2 files changed

+185
-21
lines changed

‎lib/restHandlers.js‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,53 +18,62 @@
1818
}
1919

2020
RestHandlers.prototype.genericResponse = function(method, object, responseBody, callback, rawRequest, rawResponse) {
21-
var statusCodeSeries = (rawResponse.statusCode || 0).toString().substr(0, 1);
21+
log('Handling response of ' + method + ' ' + object + ': ' + rawRequest.url.href + (rawRequest.url.href.indexOf(rawRequest.url.query) === -1 ? '?' + rawRequest.url.query : ''));
22+
23+
if (!_.isFunction(callback)) {
24+
return;
25+
}
26+
27+
var statusCodeGroup = parseInt((rawResponse.statusCode || 0).toString().substr(0, 1), 10),
28+
responseError = null,
29+
responseData = {},
30+
responseAdditionalData = {};
2231

2332
if (_.isString(responseBody)) {
2433
try {
2534
responseBody = JSON.parse(responseBody);
2635
}
27-
catch (err) {}
28-
}
29-
30-
log('Handling response of ' + method + ' ' + object + ': ' + rawRequest.url.href + (rawRequest.url.href.indexOf(rawRequest.url.query) === -1 ? '?' + rawRequest.url.query : ''));
31-
32-
if (responseBody.success === true) {
33-
if (_.isFunction(callback)) {
34-
callback(null, responseBody.data, responseBody.additional_data, rawRequest, rawResponse);
36+
catch (err) {
37+
responseError = new Error('Malformed Pipedrive API response');
3538
}
3639
}
37-
else if (statusCodeSeries == '4' || statusCodeSeries == '5') {
38-
var errorObject = new Error();
3940

41+
if (!responseError && responseBody.success === true) {
42+
responseError = null;
43+
responseData = responseBody.data;
44+
responseAdditionalData = responseBody.additional_data;
45+
}
46+
else if (statusCodeGroup === 4 || statusCodeGroup === 5) {
4047
if (_.isObject(rawResponse._error)) {
41-
errorObject = rawResponse._error;
48+
responseError = rawResponse._error;
4249
}
4350
else {
44-
errorObject = new Error('Got HTTP response ' + rawResponse.statusCode + ' from Pipedrive API');
51+
responseError = new Error('Got HTTP response ' + rawResponse.statusCode + ' from Pipedrive API');
4552

4653
if (responseBody.error) {
47-
errorObject = new Error('Pipedrive API error:' + responseBody.error);
54+
responseError = new Error('Pipedrive API error:' + responseBody.error);
4855
}
4956
else {
5057
try {
5158
var errorBody = JSON.parse(rawResponse.rawEncoded.toString('utf8'));
5259
if (!_.isUndefined(errorBody.error)) {
53-
errorObject = new Error('Pipedrive API error:' + errorBody.error);
60+
responseError = new Error('Pipedrive API error:' + errorBody.error);
5461
}
5562
}
56-
catch (err) {}
63+
catch (err) {
64+
responseError = new Error('Malformed Pipedrive API response');
65+
}
5766
}
5867
}
5968

60-
if (_.isFunction(callback)) {
61-
callback(errorObject, null, null, rawRequest, rawResponse);
62-
}
69+
responseData = null;
70+
responseAdditionalData = null;
6371
}
64-
else if (_.isFunction(callback)) {
65-
callback(null, responseBody.data || {}, responseBody.additional_data || {}, rawRequest, rawResponse);
72+
else if (responseBody.error) {
73+
responseError = new Error(responseBody.error);
6674
}
6775

76+
callback(responseError, responseData, responseAdditionalData, rawRequest, rawResponse);
6877
};
6978

7079
// GET /items

‎test/unit/restHandlers.js‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
var should = require('should');
2+
3+
describe('lib/restHandlers', function () {
4+
5+
var defaultRequest = {
6+
url: {
7+
href: '',
8+
query: ''
9+
}
10+
};
11+
12+
var defaultResponse = {
13+
statusCode: null
14+
};
15+
16+
var RestHandlers = require('./../../lib/restHandlers');
17+
var restHandlers = new RestHandlers({});
18+
19+
describe('genericResponse()', function () {
20+
it('should pass data in callback on success', function () {
21+
var responseBody = '{"success":true, "data":[{"id":1}], "additional_data":[]}';
22+
23+
var called = false;
24+
var callback = function (responseError, responseData) {
25+
called = true;
26+
should(responseData.length).be.equal(1);
27+
should(responseData[0].id).be.equal(1);
28+
};
29+
30+
restHandlers.genericResponse(
31+
'GET', {}, responseBody, callback, defaultRequest, defaultResponse
32+
);
33+
should(called).be.equal(true);
34+
});
35+
36+
it('should pass error in callback on response containing error', function () {
37+
var responseBody = '{"success":false, "error":"Backend error"}';
38+
39+
var called = false;
40+
var callback = function (responseError) {
41+
called = true;
42+
should(responseError.message).be.equal('Backend error');
43+
};
44+
45+
restHandlers.genericResponse(
46+
'GET', {}, responseBody, callback, defaultRequest, defaultResponse
47+
);
48+
should(called).be.equal(true);
49+
});
50+
51+
describe('error cases', function () {
52+
it('should return undefined if callback is not present', function () {
53+
var result = restHandlers.genericResponse(
54+
'GET', {}, {}, null, defaultRequest
55+
);
56+
57+
should(result).be.equal(undefined);
58+
});
59+
60+
it('should throw basic error if response body is not json (plain text)', function () {
61+
var responseBody = 'Oops something went wrong';
62+
63+
var called = false;
64+
var callback = function (responseError) {
65+
called = true;
66+
should(responseError.message).be.equal('Malformed Pipedrive API response');
67+
};
68+
69+
restHandlers.genericResponse(
70+
'GET', {}, responseBody, callback, defaultRequest, defaultResponse
71+
);
72+
should(called).be.equal(true);
73+
});
74+
75+
76+
describe('handle 4xx/5xx http statuses', function () {
77+
it('should throw error, if has .error in response', function () {
78+
var responseBody = '{"error": "Backend error"}';
79+
80+
var called = false;
81+
var callback = function (responseError) {
82+
called = true;
83+
should(responseError.message).be.equal('Pipedrive API error:Backend error');
84+
};
85+
86+
restHandlers.genericResponse(
87+
'GET', {}, responseBody, callback, defaultRequest, {
88+
statusCode: 500
89+
}
90+
);
91+
should(called).be.equal(true);
92+
});
93+
94+
it('should throw "Malformed" error, if response is not json', function () {
95+
var responseBody = "Not valid json response";
96+
97+
var called = false;
98+
var callback = function (responseError) {
99+
called = true;
100+
should(responseError.message).be.equal('Malformed Pipedrive API response');
101+
};
102+
103+
restHandlers.genericResponse(
104+
'GET', {}, responseBody, callback, defaultRequest, {
105+
statusCode: 404,
106+
rawEncoded: responseBody
107+
}
108+
);
109+
should(called).be.equal(true);
110+
});
111+
112+
describe('raw response handling', function () {
113+
114+
it('should throw custom error object, if has internal raw ._error passed from rest.js', function () {
115+
var responseBody = 'Not found';
116+
117+
var called = false;
118+
var callback = function (responseError) {
119+
called = true;
120+
should(responseError.message).be.equal('Custom error');
121+
};
122+
123+
restHandlers.genericResponse(
124+
'GET', {}, responseBody, callback, defaultRequest, {
125+
statusCode: 404,
126+
_error: {
127+
message: "Custom error"
128+
}
129+
}
130+
);
131+
should(called).be.equal(true);
132+
});
133+
134+
it('should throw custom error object, if body.error is missing, but rawEncoded.error is passed from rest.js', function () {
135+
var responseBody = 'Not found';
136+
137+
var called = false;
138+
var callback = function (responseError) {
139+
called = true;
140+
should(responseError.message).be.equal('Pipedrive API error:Custom backend error');
141+
};
142+
143+
restHandlers.genericResponse(
144+
'GET', {}, responseBody, callback, defaultRequest, {
145+
statusCode: 404,
146+
rawEncoded: '{"error":"Custom backend error"}'
147+
}
148+
);
149+
should(called).be.equal(true);
150+
});
151+
});
152+
});
153+
});
154+
});
155+
});

0 commit comments

Comments
 (0)