Bug 1460525 - Execute every Remote Settings event listeners when one fails r?mgoodwin
MozReview-Commit-ID: KOqESGbzapC
--- a/services/common/docs/RemoteSettings.rst
+++ b/services/common/docs/RemoteSettings.rst
@@ -63,19 +63,16 @@ The ``sync`` event allows to be notified
RemoteSettings("a-key").on("sync", event => {
const { data: { current } } = event;
for(const entry of current) {
// Do something with entry...
// await InternalAPI.reload(entry.id, entry.label, entry.weight);
}
});
-.. 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 <https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js>`_ (~ every 24H).
File attachments
----------------
When an entry has a file attached to it, it has an ``attachment`` attribute, which contains the file related information (url, hash, size, mimetype, etc.). Remote files are not downloaded automatically.
--- a/services/common/remote-settings.js
+++ b/services/common/remote-settings.js
@@ -183,18 +183,18 @@ class RemoteSettingsClient {
constructor(collectionName, { bucketName, signerName, filterFunc = jexlFilterFunc, lastCheckTimePref }) {
this.collectionName = collectionName;
this.bucketName = bucketName;
this.signerName = signerName;
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;
}
get identifier() {
return `${this.bucketName}/${this.collectionName}`;
}
@@ -203,21 +203,46 @@ class RemoteSettingsClient {
const identifier = OS.Path.join(...this.identifier.split("/"));
return `${identifier}.json`;
}
get lastCheckTimePref() {
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);
}
/**
* Open the underlying Kinto collection, using the appropriate adapter and
* options. This acts as a context manager where the connection is closed
* once the specified `callback` has finished.
*
* @param {callback} function the async function to execute with the open SQlite connection.
@@ -404,24 +429,19 @@ class RemoteSettingsClient {
const updatedFilteredIds = new Set(updatedFiltered.map(e => e.id));
const updated = allUpdated.filter(({ new: { id } }) => updatedFilteredIds.has(id));
// If every changed entry is filtered, we don't even fire the event.
if (created.length || updated.length || deleted.length) {
// 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;
}
}
// Track last update.
this._updateLastCheck(serverTime);
--- a/services/common/tests/unit/test_remote_settings.js
+++ b/services/common/tests/unit/test_remote_settings.js
@@ -11,16 +11,18 @@ const BinaryInputStream = CC("@mozilla.o
let server;
let client;
async function clear_state() {
// Clear local DB.
const collection = await client.openCollection();
await collection.clear();
+ // Reset event listeners.
+ client._listeners.set("sync", []);
}
function run_test() {
// Set up an HTTP Server
server = new HttpServer();
server.start(-1);
@@ -136,16 +138,36 @@ add_task(async function test_sync_event_
equal(eventData.current.length, 1);
equal(eventData.created.length, 0);
equal(eventData.updated.length, 0);
equal(eventData.deleted.length, 1);
equal(eventData.deleted[0].website, "https://www.other.org/signin");
});
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();
const startHistogram = getUptakeTelemetrySnapshot(client.identifier);
await client.maybeSync(3000, serverTime);
// No Telemetry was sent.