Skip to content

Fix(nodejs/event_dispatcher): update error/resp handlers #123

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

Merged
merged 6 commits into from
Jun 25, 2018
Merged

Fix(nodejs/event_dispatcher): update error/resp handlers #123

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6085b72
Fix: add handler for http error event
Jun 22, 2018
e964ecf
also fix request callback
Jun 22, 2018
a60c31d
Expect error in first arg, log appropriate message
Jun 22, 2018
868beba
fix typo
Jun 22, 2018
977a3ca
just use 2 messages
Jun 22, 2018
bb7598b
revert error-logging changes
Jun 22, 2018
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: 10 additions & 2 deletions packages/optimizely-sdk/lib/optimizely/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,11 @@ Optimizely.prototype._sendImpressionEvent = function(experimentKey, variationKey
impressionEvent.url,
JSON.stringify(impressionEvent.params));
this.logger.log(LOG_LEVEL.DEBUG, dispatchedImpressionEventLogMessage);
var eventDispatcherCallback = function() {
var eventDispatcherCallback = function(err, response) {
if (err) {
this.logger.log(LOG_LEVEL.ERROR, sprintf(LOG_MESSAGES.FAILED_TO_TRACK_IMPRESSION_EVENT, MODULE_NAME, experimentKey, err));
return;
}
var activatedLogMessage = sprintf(LOG_MESSAGES.ACTIVATE_USER, MODULE_NAME, userId, experimentKey);
this.logger.log(LOG_LEVEL.INFO, activatedLogMessage);
}.bind(this);
Expand Down Expand Up @@ -263,7 +267,11 @@ Optimizely.prototype.track = function(eventKey, userId, attributes, eventTags) {
JSON.stringify(conversionEvent.params));
this.logger.log(LOG_LEVEL.DEBUG, dispatchedConversionEventLogMessage);

var eventDispatcherCallback = function() {
var eventDispatcherCallback = function(err) {
if (err) {
this.logger.log(LOG_LEVEL.ERROR, sprintf(LOG_MESSAGES.FAILED_TO_TRACK_CONVERSION_EVENT, MODULE_NAME, eventKey, err));
return;
}
var trackedLogMessage = sprintf(LOG_MESSAGES.TRACK_EVENT, MODULE_NAME, eventKey, userId);
this.logger.log(LOG_LEVEL.INFO, trackedLogMessage);
}.bind(this);
Expand Down
2 changes: 1 addition & 1 deletion packages/optimizely-sdk/lib/plugins/event_dispatcher/index.browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module.exports = {
req.setRequestHeader('Content-Type', 'application/json');
req.onreadystatechange = function() {
if (req.readyState === READYSTATE_COMPLETE && callback && typeof callback === 'function') {
callback(params);
callback(null, params);
}
};
req.send(JSON.stringify(params));
Expand Down
7 changes: 4 additions & 3 deletions packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,14 @@ module.exports = {
}
};

var requestCallback = function(error, response, body) {
if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400 && callback && typeof callback === 'function') {
callback();
var requestCallback = function(response) {
if (response && response.statusCode && response.statusCode >= 200 && response.statusCode < 400) {
callback(null, response);
}
};

var req = (parsedUrl.protocol === 'http:' ? http : https).request(requestOptions, requestCallback);
req.on('error', callback);
req.write(dataString);
req.end();
return req;
Expand Down
22 changes: 14 additions & 8 deletions packages/optimizely-sdk/lib/plugins/event_dispatcher/index.node.tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,7 @@ describe('lib/plugins/event_dispatcher/node', function() {
httpVerb: 'POST',
};

eventDispatcher.dispatchEvent(eventObj)
.on('response', function(response) {
assert.isTrue(response.statusCode === 200);
done();
})
.on('error', function(error) {
assert.fail('status code okay', 'status code not okay', '');
});
eventDispatcher.dispatchEvent(eventObj, done);
});

it('should execute the callback passed to event dispatcher', function(done) {
Expand Down Expand Up @@ -98,5 +91,18 @@ describe('lib/plugins/event_dispatcher/node', function() {
eventDispatcher.dispatchEvent(eventObj, callback);
});
});

it('calls callback with err when one is emitted', function(done) {
Copy link
Contributor Author

@tylerbrandt tylerbrandt Jun 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: before changing the event_dispatcher code, the .dispatchEvent call did indeed throw an error.

var eventObj = {
url: 'https://example',
params: {},
httpVerb: 'POST',
};

eventDispatcher.dispatchEvent(eventObj, function(err) {
assert.isNotNull(err);
done();
});
});
});
});
2 changes: 2 additions & 0 deletions packages/optimizely-sdk/lib/utils/enums/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ exports.LOG_MESSAGES = {
FEATURE_HAS_NO_EXPERIMENTS: '%s: Feature %s is not attached to any experiments.',
FAILED_TO_PARSE_VALUE: '%s: Failed to parse event value "%s" from event tags.',
FAILED_TO_PARSE_REVENUE: '%s: Failed to parse revenue value "%s" from event tags.',
FAILED_TO_TRACK_CONVERSION_EVENT: '%s: Failed to track conversion event for %s: %s',
FAILED_TO_TRACK_IMPRESSION_EVENT: '%s: Failed to track impression event for experiment %s: %s',
FORCED_BUCKETING_FAILED: '%s: Variation key %s is not in datafile. Not activating user %s.',
INVALID_OBJECT: '%s: Optimizely object is not valid. Failing %s.',
INVALID_CLIENT_ENGINE: '%s: Invalid client engine passed: %s. Defaulting to node-sdk.',
Expand Down