diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a2463d..b0590a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,20 @@ All notable changes to the LaunchDarkly Node.js SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [5.1.0] - 2018-06-26 + +### Added: +- The new event `"failed"` will fire if client initialization failed due to any of the unrecoverable errors described below. If you prefer to use Promises, there is a new method `waitForInitialization()`, which behaves exactly like `waitUntilReady()` except that its Promise will be rejected if the "failed" event fires. (For backward compatibility, the Promise returned by `waitUntilReady()` will never be rejected.) ([#96](https://github.com/launchdarkly/node-client/issues/96)) + +### Changed: +- The client now treats most HTTP 4xx errors as unrecoverable: that is, after receiving such an error, it will not make any more HTTP requests for the lifetime of the client instance, in effect taking the client offline. This is because such errors indicate either a configuration problem (invalid SDK key) or a bug, which is not likely to resolve without a restart or an upgrade. This does not apply if the error is 400, 408, 429, or any 5xx error. + +### Fixed: +- Fixed a bug that would cause a null reference error if you called `close()` on an offline client. (Thanks, [dylanlingelbach](https://github.com/launchdarkly/node-client/pull/100)!) + +### Deprecated: +- The `waitUntilReady()` method is now deprecated in favor of `waitForInitialization()` (see above). + ## [5.0.2] - 2018-06-15 ### Fixed: diff --git a/README.md b/README.md index fbad381..ab8074e 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,11 @@ LaunchDarkly SDK for Node.js [![Circle CI](https://circleci.com/gh/launchdarkly/node-client/tree/master.svg?style=svg)](https://circleci.com/gh/launchdarkly/node-client/tree/master) +Supported Node versions +----------------------- + +This version of the LaunchDarkly SDK has been tested with Node versions 6.14 and up. + Quick setup ----------- diff --git a/errors.js b/errors.js index 5901a86..74b7495 100644 --- a/errors.js +++ b/errors.js @@ -17,3 +17,10 @@ exports.LDStreamingError = createCustomError('LaunchDarklyStreamingError'); exports.LDUnexpectedResponseError = createCustomError('LaunchDarklyUnexpectedResponseError'); exports.LDInvalidSDKKeyError = createCustomError('LaunchDarklyInvalidSDKKeyError'); exports.LDClientError = createCustomError('LaunchDarklyClientError'); + +exports.isHttpErrorRecoverable = function(status) { + if (status >= 400 && status < 500) { + return status === 408 || status === 429; + } + return true; +} diff --git a/event_processor.js b/event_processor.js index ccca40e..ab453b1 100644 --- a/event_processor.js +++ b/event_processor.js @@ -3,6 +3,7 @@ var request = require('request'); var EventSummarizer = require('./event_summarizer'); var UserFilter = require('./user_filter'); var errors = require('./errors'); +var messages = require('./messages'); var wrapPromiseCallback = require('./utils/wrapPromiseCallback'); function EventProcessor(sdkKey, config, errorReporter) { @@ -200,15 +201,12 @@ function EventProcessor(sdkKey, config, errorReporter) { } } if (resp.statusCode > 204) { - var err = new errors.LDUnexpectedResponseError("Unexpected status code " + resp.statusCode + "; events may not have been processed", - resp.statusCode); + var err = new errors.LDUnexpectedResponseError(messages.httpErrorMessage(resp.statusCode, 'event posting', 'some events were dropped')); errorReporter && errorReporter(err); - if (resp.statusCode === 401) { + if (!errors.isHttpErrorRecoverable(resp.statusCode)) { reject(err); - var err1 = new errors.LDInvalidSDKKeyError("Received 401 error, no further events will be posted since SDK key is invalid"); - errorReporter && errorReporter(err1); shutdown = true; - } else if (resp.statusCode >= 500) { + } else { retryOrReject(err); } } else { diff --git a/eventsource.js b/eventsource.js index 9b567bb..4cee815 100644 --- a/eventsource.js +++ b/eventsource.js @@ -4,7 +4,9 @@ var parse = require('url').parse , events = require('events') , https = require('https') , http = require('http') - , util = require('util'); + , util = require('util') + , errors = require('./errors') + , messages = require('./messages'); function isPlainObject(obj) { return Object.getPrototypeOf(obj) === Object.prototype; @@ -104,18 +106,15 @@ function EventSource(url, eventSourceInitDict) { } if (res.statusCode !== 200) { - // reconnect after an error, unless it's a 401 - if (res.statusCode === 401) { + // reconnect after an error, unless it's an unrecoverable error + _emit('error', new Event('error', { + message: messages.httpErrorMessage(res.statusCode, 'streaming connection', 'will retry'), + status: res.statusCode + })); + + if (!errors.isHttpErrorRecoverable(res.statusCode)) { readyState = EventSource.CLOSED; - _emit('error', new Event('error', { - message: 'Received 401 error, no further streaming connection will be made since SDK key is invalid', - status: 401 - })); } else { - _emit('error', new Event('error', { - message: 'Streaming connection returned status code: ' + res.statusCode, - status: res.statusCode - })); backoffDelay = Math.min(backoffDelay * 2, 15000); setTimeout(connect, backoffDelay); } diff --git a/index.d.ts b/index.d.ts index 994a963..e37f8e4 100644 --- a/index.d.ts +++ b/index.d.ts @@ -376,10 +376,26 @@ declare module "ldclient-node" { initialized: () => boolean; /** + * Returns a Promise that will be resolved if and when the client is successfully initialized. + * If initialization fails, the Promise will not resolve, but will not be rejected either + * (unlike waitForInitialization). + * + * This method is deprecated and will be removed in a future release. Instead, use + * waitForInitialization(), which waits for either success or failure. + * * @returns a Promise containing the initialization state of the client */ waitUntilReady: () => Promise; + /** + * Returns a Promise that will be resolved if the client successfully initializes, or + * rejected if client initialization has irrevocably failed (for instance, if it detects + * that the SDK key is invalid). The sucess and failure cases can also be detected by listening + * for the events "ready" and "failed". + * @returns a Promise containing the initialization state of the client + */ + waitForInitialization: () => Promise; + /** * Retrieves a flag's value. * diff --git a/index.js b/index.js index ec8670c..cc58ae1 100644 --- a/index.js +++ b/index.js @@ -53,6 +53,7 @@ function NullUpdateProcessor() { var newClient = function(sdkKey, config) { var client = new EventEmitter(), initComplete = false, + failure, queue = [], requestor, updateProcessor, @@ -110,6 +111,8 @@ var newClient = function(sdkKey, config) { } maybeReportError(error); + client.emit('failed', error); + failure = error; } else if (!initComplete) { initComplete = true; client.emit('ready'); @@ -121,6 +124,8 @@ var newClient = function(sdkKey, config) { }; client.waitUntilReady = function() { + config.logger.warn(messages.deprecated("waitUntilReady", "waitForInitialization")); + if (initComplete) { return Promise.resolve(); } @@ -130,6 +135,20 @@ var newClient = function(sdkKey, config) { }); }; + client.waitForInitialization = function() { + if (initComplete) { + return Promise.resolve(); + } + if (failure) { + return Promise.reject(failure); + } + + return new Promise(function(resolve, reject) { + client.once('ready', resolve); + client.once('failed', reject); + }); + }; + client.variation = function(key, user, defaultVal, callback) { return wrapPromiseCallback(new Promise(function(resolve, reject) { sanitizeUser(user); diff --git a/messages.js b/messages.js index 95ae5e9..5b52836 100644 --- a/messages.js +++ b/messages.js @@ -1,4 +1,12 @@ +var errors = require('./errors'); exports.deprecated = function(oldName, newName) { return '"' + oldName + '" is deprecated, please use "' + newName + '"'; -} +}; + +exports.httpErrorMessage = function(status, context, retryMessage) { + return 'Received error ' + status + + (status == 401 ? ' (invalid SDK key)' : '') + + ' for ' + context + + ' - ' + (errors.isHttpErrorRecoverable(status) ? retryMessage : 'giving up permanently'); +}; diff --git a/package.json b/package.json index 4afacf6..279ccaa 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ldclient-node", - "version": "5.0.2", + "version": "5.1.0", "description": "LaunchDarkly SDK for Node.js", "main": "index.js", "scripts": { diff --git a/polling.js b/polling.js index f58baf4..5eb9a3c 100644 --- a/polling.js +++ b/polling.js @@ -1,4 +1,5 @@ var errors = require('./errors'); +var messages = require('./messages'); var dataKind = require('./versioned_data_kind'); function PollingProcessor(config, requestor) { @@ -22,8 +23,8 @@ function PollingProcessor(config, requestor) { sleepFor = Math.max(config.pollInterval * 1000 - elapsed, 0); config.logger.debug("Elapsed: %d ms, sleeping for %d ms", elapsed, sleepFor); if (err) { - cb(new errors.LDPollingError('Failed to fetch all feature flags: ' + (err.message || JSON.stringify(err))), err.status); - if (err.status === 401) { + cb(new errors.LDPollingError(messages.httpErrorMessage(err.status, 'polling request', 'will retry'))); + if (!errors.isHttpErrorRecoverable(err.status)) { config.logger.error('Received 401 error, no further polling requests will be made since SDK key is invalid'); } else { // Recursively call poll after the appropriate delay diff --git a/test/LDClient-test.js b/test/LDClient-test.js index d6aea6c..22dd209 100644 --- a/test/LDClient-test.js +++ b/test/LDClient-test.js @@ -24,7 +24,7 @@ describe('LDClient', function() { var updateProcessor = { start: function(callback) { - setImmediate(callback, null); + setImmediate(callback, updateProcessor.error); } }; @@ -32,6 +32,7 @@ describe('LDClient', function() { logger.info = jest.fn(); logger.warn = jest.fn(); eventProcessor.events = []; + updateProcessor.error = null; }); it('should trigger the ready event in offline mode', function(done) { @@ -315,4 +316,54 @@ describe('LDClient', function() { }).catch(done.error) }); }); + + describe('waitForInitialization()', function () { + it('should resolve when ready', function(done) { + var callback = jest.fn(); + var client = createOnlineClientWithFlags({}); + + client.waitForInitialization().then(callback) + .then(() => { + expect(callback).toHaveBeenCalled(); + done(); + }).catch(done.error) + }); + + it('should resolve even if the client is already ready', function(done) { + var callback = jest.fn(); + var client = createOnlineClientWithFlags({}); + + client.waitForInitialization() + .then(() => { + client.waitForInitialization().then(callback) + .then(() => { + expect(callback).toHaveBeenCalled(); + done(); + }).catch(done.error) + }).catch(done.error) + }); + + it('should be rejected if initialization fails', function(done) { + updateProcessor.error = { status: 403 }; + var client = createOnlineClientWithFlags({}); + + client.waitForInitialization() + .catch(err => { + expect(err).toEqual(updateProcessor.error); + done(); + }); + }); + }); + + describe('failed event', function() { + it('should be fired if initialization fails', function(done) { + updateProcessor.error = { status: 403 }; + var client = createOnlineClientWithFlags({}); + + client.on('failed', err => { + expect(err).toEqual(updateProcessor.error); + done(); + }); + }); + }) }); diff --git a/test/event_processor-test.js b/test/event_processor-test.js index e42b4ea..acdfebc 100644 --- a/test/event_processor-test.js +++ b/test/event_processor-test.js @@ -25,6 +25,7 @@ describe('EventProcessor', function() { if (ep) { ep.close(); } + nock.cleanAll(); }); function flushAndGetRequest(options, cb) { @@ -394,13 +395,13 @@ describe('EventProcessor', function() { }); }); - it('stops sending events after a 401 error', function(done) { + function verifyUnrecoverableHttpError(done, status) { ep = EventProcessor(sdkKey, defaultConfig); var e = { kind: 'identify', creationDate: 1000, user: user }; ep.sendEvent(e); - flushAndGetRequest({ status: 401 }, function(body, headers, error) { - expect(error.message).toContain("status code 401"); + flushAndGetRequest({ status: status }, function(body, headers, error) { + expect(error.message).toContain("error " + status); ep.sendEvent(e); @@ -412,21 +413,41 @@ describe('EventProcessor', function() { done(); }); }); - }); + } - it('retries once after a 5xx error', function(done) { + function verifyRecoverableHttpError(done, status) { ep = EventProcessor(sdkKey, defaultConfig); var e = { kind: 'identify', creationDate: 1000, user: user }; ep.sendEvent(e); - nock(eventsUri).post('/bulk').reply(503); - nock(eventsUri).post('/bulk').reply(503); + nock(eventsUri).post('/bulk').reply(status); + nock(eventsUri).post('/bulk').reply(status); // since we only queued two responses, Nock will throw an error if it gets a third. ep.flush().then( function() {}, function(err) { - expect(err.message).toContain('Unexpected status code 503'); + expect(err.message).toContain('error ' + status); done(); }); + } + + it('stops sending events after a 401 error', function(done) { + verifyUnrecoverableHttpError(done, 401); + }); + + it('stops sending events after a 403 error', function(done) { + verifyUnrecoverableHttpError(done, 403); + }); + + it('retries after a 408 error', function(done) { + verifyRecoverableHttpError(done, 408); + }); + + it('retries after a 429 error', function(done) { + verifyRecoverableHttpError(done, 429); + }); + + it('retries after a 503 error', function(done) { + verifyRecoverableHttpError(done, 503); }); });