diff --git a/services/common/docs/RemoteSettings.rst b/services/common/docs/RemoteSettings.rst index 52234eb6c9f2..457b8f874db6 100644 --- a/services/common/docs/RemoteSettings.rst +++ b/services/common/docs/RemoteSettings.rst @@ -68,9 +68,6 @@ The ``sync`` event allows to be notified when the remote settings are changed on } }); -.. important:: - If one of the event handler fails, the others handlers for the same remote settings collection won't be executed. - .. note:: Currently, the update of remote settings is triggered by the `nsBlocklistService `_ (~ every 24H). diff --git a/services/common/remote-settings.js b/services/common/remote-settings.js index 8755883cd099..5e64b20668c8 100644 --- a/services/common/remote-settings.js +++ b/services/common/remote-settings.js @@ -188,8 +188,8 @@ class RemoteSettingsClient { this.filterFunc = filterFunc; this._lastCheckTimePref = lastCheckTimePref; - this._callbacks = new Map(); - this._callbacks.set("sync", []); + this._listeners = new Map(); + this._listeners.set("sync", []); this._kinto = null; } @@ -208,11 +208,36 @@ class RemoteSettingsClient { return this._lastCheckTimePref || `services.settings.${this.bucketName}.${this.collectionName}.last_check`; } + /** + * Event emitter: will execute the registered listeners in the order and + * sequentially. + * + * Note: we don't use `toolkit/modules/EventEmitter` because we want to throw + * an error when a listener fails to execute. + * + * @param {string} event the event name + * @param {Object} payload the event payload to call the listeners with + */ + async emit(event, payload) { + const callbacks = this._listeners.get("sync"); + let firstError; + for (const cb of callbacks) { + try { + await cb(payload); + } catch (e) { + firstError = e; + } + } + if (firstError) { + throw firstError; + } + } + on(event, callback) { - if (!this._callbacks.has(event)) { + if (!this._listeners.has(event)) { throw new Error(`Unknown event type ${event}`); } - this._callbacks.get(event).push(callback); + this._listeners.get(event).push(callback); } /** @@ -409,14 +434,9 @@ class RemoteSettingsClient { // Read local collection of records (also filtered). const { data: allData } = await collection.list(); const current = await this._filterEntries(allData); - // Fire the event: execute callbacks in order and sequentially. - // If one fails everything fails. - const event = { data: { current, created, updated, deleted } }; - const callbacks = this._callbacks.get("sync"); + const payload = { data: { current, created, updated, deleted } }; try { - for (const cb of callbacks) { - await cb(event); - } + await this.emit("sync", payload); } catch (e) { reportStatus = UptakeTelemetry.STATUS.APPLY_ERROR; throw e; diff --git a/services/common/tests/unit/test_remote_settings.js b/services/common/tests/unit/test_remote_settings.js index 75795f07c9e2..941e75b6ba75 100644 --- a/services/common/tests/unit/test_remote_settings.js +++ b/services/common/tests/unit/test_remote_settings.js @@ -16,6 +16,8 @@ async function clear_state() { // Clear local DB. const collection = await client.openCollection(); await collection.clear(); + // Reset event listeners. + client._listeners.set("sync", []); } @@ -141,6 +143,26 @@ add_task(async function test_sync_event_provides_information_about_records() { }); add_task(clear_state); +add_task(async function test_all_listeners_are_executed_if_one_fails() { + const serverTime = Date.now(); + + let count = 0; + client.on("sync", () => { count += 1; }); + client.on("sync", () => { throw new Error("boom"); }); + client.on("sync", () => { count += 2; }); + + let error; + try { + await client.maybeSync(2000, serverTime); + } catch (e) { + error = e; + } + + equal(count, 3); + equal(error.message, "boom"); +}); +add_task(clear_state); + add_task(async function test_telemetry_reports_up_to_date() { await client.maybeSync(2000, Date.now() - 1000); const serverTime = Date.now();