Skip to content

Commit 6d6bb24

Browse files
author
Tyler Brandt
authored
Fix(nodejs/event_dispatcher): update error/resp handlers (#123)
- (nodejs) Prevent crash when `http`/`https` emits an error by adding an 'error' listener - (nodejs) Fix `requestCallback` to conform to its role as a 'response' listener; notably, ensure it is called when the first argument is interpeted as the `http.IncomingMessage`, and _not_ called in the event of an error (as expected by `Optimizely#_sendImpressionEvent`/`Optimizely#track`). Tested that this version (as 2.1.2-0) _does_ emit the messages expected in the demo app. Fixes #122 Fixes #124
1 parent 3ba71a0 commit 6d6bb24

File tree

2 files changed

+23
-20
lines changed

2 files changed

+23
-20
lines changed

packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ module.exports = {
3030
dispatchEvent: function(eventObj, callback) {
3131
// Non-POST requests not supported
3232
if (eventObj.httpVerb !== 'POST') {
33-
callback(new Error('httpVerb not supported: ' + eventObj.httpVerb));
3433
return;
3534
}
3635

@@ -52,13 +51,15 @@ module.exports = {
5251
}
5352
};
5453

55-
var requestCallback = function(error, response, body) {
56-
if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400 && callback && typeof callback === 'function') {
57-
callback();
54+
var requestCallback = function(response) {
55+
if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400) {
56+
callback(response);
5857
}
5958
};
6059

6160
var req = (parsedUrl.protocol === 'http:' ? http : https).request(requestOptions, requestCallback);
61+
// Add no-op error listener to prevent this from throwing
62+
req.on('error', function() {});
6263
req.write(dataString);
6364
req.end();
6465
return req;

packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,9 @@ describe('lib/plugins/event_dispatcher/node', function() {
4949
httpVerb: 'POST',
5050
};
5151

52-
eventDispatcher.dispatchEvent(eventObj)
53-
.on('response', function(response) {
54-
assert.isTrue(response.statusCode === 200);
52+
eventDispatcher.dispatchEvent(eventObj, function(resp) {
53+
assert.equal(200, resp.statusCode);
5554
done();
56-
})
57-
.on('error', function(error) {
58-
assert.fail('status code okay', 'status code not okay', '');
5955
});
6056
});
6157

@@ -70,15 +66,15 @@ describe('lib/plugins/event_dispatcher/node', function() {
7066

7167
eventDispatcher.dispatchEvent(eventObj, stubCallback.callback)
7268
.on('response', function(response) {
73-
done();
7469
sinon.assert.calledOnce(stubCallback.callback);
70+
done();
7571
})
7672
.on('error', function(error) {
7773
assert.fail('status code okay', 'status code not okay', '');
7874
});
7975
});
8076

81-
it('rejects GET httpVerb', function(done) {
77+
it('rejects GET httpVerb', function() {
8278
var eventObj = {
8379
url: 'https://cdn.com/event',
8480
params: {
@@ -87,16 +83,22 @@ describe('lib/plugins/event_dispatcher/node', function() {
8783
httpVerb: 'GET',
8884
};
8985

90-
var callback = function(err) {
91-
if (err) {
92-
done();
93-
} else {
94-
done(new Error('expected error not thrown'));
95-
}
96-
};
97-
86+
var callback = sinon.spy();
9887
eventDispatcher.dispatchEvent(eventObj, callback);
88+
sinon.assert.notCalled(callback);
9989
});
10090
});
91+
92+
it('does not throw in the event of an error', function() {
93+
var eventObj = {
94+
url: 'https://example',
95+
params: {},
96+
httpVerb: 'POST',
97+
};
98+
99+
var callback = sinon.spy();
100+
eventDispatcher.dispatchEvent(eventObj, callback);
101+
sinon.assert.notCalled(callback);
102+
});
101103
});
102104
});

0 commit comments

Comments
 (0)