Backed out changeset 7c0e462cb0ba (bug 1772136) for causing mochitest failures on browser_remotesettingsexperimentloader_remote_defaults.js

browser/components/newtab/test/browser/browser_asrouter_experimentsAPILoader.js

@@ -174,8 +174,6 @@ const client = RemoteSettings("nimbus-desktop-experiments");
 // no `add_task` because we want to run this setup before each test not before
 // the entire test suite.
 async function setup(experiment) {
-  // Store the experiment in RS local db to bypass synchronization.
-  await client.db.importChanges({}, Date.now(), [experiment], { clear: true });
   await SpecialPowers.pushPrefEnv({
     set: [
       ["app.shield.optoutstudies.enabled", true],
@@ -185,6 +183,8 @@ async function setup(experiment) {
       ],
     ],
   });
+
+  await client.db.importChanges({}, Date.now(), [experiment], { clear: true });
 }
 
 async function cleanup() {

services/settings/RemoteSettingsClient.jsm

@@ -393,14 +393,13 @@ class RemoteSettingsClient extends EventEmitter {
   /**
    * Lists settings.
    *
-   * @param  {Object} options                    The options object.
-   * @param  {Object} options.filters            Filter the results (default: `{}`).
-   * @param  {String} options.order              The order to apply (eg. `"-last_modified"`).
-   * @param  {boolean} options.dumpFallback      Fallback to dump data if read of local DB fails (default: `true`).
-   * @param  {boolean} options.emptyListFallback Fallback to empty list if no dump data and read of local DB fails (default: `true`).
-   * @param  {boolean} options.loadDumpIfNewer   Use dump data if it is newer than local data (default: `true`).
-   * @param  {boolean} options.syncIfEmpty       Synchronize from server if local data is empty (default: `true`).
-   * @param  {boolean} options.verifySignature   Verify the signature of the local data (default: `false`).
+   * @param  {Object} options                  The options object.
+   * @param  {Object} options.filters          Filter the results (default: `{}`).
+   * @param  {String} options.order            The order to apply (eg. `"-last_modified"`).
+   * @param  {boolean} options.dumpFallback    Fallback to dump data if read of local DB fails (default: `true`).
+   * @param  {boolean} options.loadDumpIfNewer Use dump data if it is newer than local data (default: `true`).
+   * @param  {boolean} options.syncIfEmpty     Synchronize from server if local data is empty (default: `true`).
+   * @param  {boolean} options.verifySignature Verify the signature of the local data (default: `false`).
    * @return {Promise}
    */
   async get(options = {}) {
@@ -408,13 +407,11 @@ class RemoteSettingsClient extends EventEmitter {
       filters = {},
       order = "", // not sorted by default.
       dumpFallback = true,
-      emptyListFallback = true,
       loadDumpIfNewer = true,
       syncIfEmpty = true,
     } = options;
     let { verifySignature = false } = options;
 
-    const hasParallelCall = !!this._importingPromise;
     let data;
     try {
       let lastModified = await this.db.getLastModified();
@@ -481,12 +478,8 @@ class RemoteSettingsClient extends EventEmitter {
             verifySignature = false;
           }
         } catch (e) {
-          if (!hasParallelCall) {
-            // Sync or load dump failed. Throw.
-            throw e;
-          }
           // Report error, but continue because there could have been data
-          // loaded from a parallel call.
+          // loaded from a parrallel call.
           Cu.reportError(e);
         } finally {
           // then delete this promise again, as now we should have local data:
@@ -498,7 +491,7 @@ class RemoteSettingsClient extends EventEmitter {
       data = await this.db.list({ filters, order });
     } catch (e) {
       // If the local DB cannot be read (for unknown reasons, Bug 1649393)
-      // or sync failed, we fallback to the packaged data, and filter/sort in memory.
+      // We fallback to the packaged data, and filter/sort in memory.
       if (!dumpFallback) {
         throw e;
       }
@@ -509,15 +502,11 @@ class RemoteSettingsClient extends EventEmitter {
       );
       if (data !== null) {
         lazy.console.info(`${this.identifier} falling back to JSON dump`);
-      } else if (emptyListFallback) {
+      } else {
         lazy.console.info(
           `${this.identifier} no dump fallback, return empty list`
         );
         data = [];
-      } else {
-        // Obtaining the records failed, there is no dump, and we don't fallback
-        // to an empty list. Throw the original error.
-        throw e;
       }
       if (!lazy.ObjectUtils.isEmpty(filters)) {
         data = data.filter(r => lazy.Utils.filterObject(filters, r));

services/settings/docs/index.rst

@@ -73,9 +73,6 @@ Options
 * ``verifySignature``: verify the content signature of the local data (default: ``false``).
   An error is thrown if the local data was altered. This hurts performance, but can be used if your use case needs to be secure from local tampering.
 
-* ``emptyListFallback``: return an empty list if obtaining the records fails (default: ``true``).
-  If an error occurs during the reading of local data, or during synchronization, then an empty list is returned.
-
 
 Events
 ------

services/settings/test/unit/test_remote_settings.js

@@ -379,23 +379,6 @@ add_task(async function test_get_falls_back_to_dump_if_db_fails_later() {
 });
 add_task(clear_state);
 
-add_task(async function test_get_falls_back_to_dump_if_network_fails() {
-  if (IS_ANDROID) {
-    // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
-    return;
-  }
-  const backup = clientWithDump.sync;
-  clientWithDump.sync = () => {
-    throw new Error("Sync error");
-  };
-
-  const records = await clientWithDump.get();
-  ok(records.length > 0, "dump content is returned");
-
-  clientWithDump.sync = backup;
-});
-add_task(clear_state);
-
 add_task(async function test_get_does_not_sync_if_empty_dump_is_provided() {
   if (IS_ANDROID) {
     // Skip test: we don't ship remote settings dumps on Android (see package-manifest).
@@ -432,7 +415,7 @@ add_task(
 );
 add_task(clear_state);
 
-add_task(async function test_get_ignores_synchronization_errors_by_default() {
+add_task(async function test_get_ignores_synchronization_errors() {
   // The monitor endpoint won't contain any information about this collection.
   let data = await RemoteSettings("some-unknown-key").get();
   equal(data.length, 0);
@@ -442,22 +425,6 @@ add_task(async function test_get_ignores_synchronization_errors_by_default() {
 });
 add_task(clear_state);
 
-add_task(async function test_get_throws_if_no_empty_fallback() {
-  // The monitor endpoint won't contain any information about this collection.
-  try {
-    await RemoteSettings("some-unknown-key").get({
-      emptyListFallback: false,
-    });
-    Assert.ok(false, ".get() should throw");
-  } catch (error) {
-    Assert.ok(
-      error.message.includes("Response from server unparseable"),
-      "Server error was thrown"
-    );
-  }
-});
-add_task(clear_state);
-
 add_task(async function test_get_verify_signature_no_sync() {
   // No signature in metadata, and no sync if empty.
   let error;

toolkit/components/nimbus/ExperimentAPI.jsm

@@ -279,8 +279,6 @@ const ExperimentAPI = {
         filters: { slug },
       });
     } catch (e) {
-      // If an error occurs in .get(), an empty list is returned and the destructuring
-      // assignment will throw.
       Cu.reportError(e);
       recipe = undefined;
     }

toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.jsm

@@ -206,10 +206,7 @@ class _RemoteSettingsExperimentLoader {
     let loadingError = false;
 
     try {
-      recipes = await this.remoteSettingsClient.get({
-        // Throw instead of returning an empty list.
-        emptyListFallback: false,
-      });
+      recipes = await this.remoteSettingsClient.get();
       lazy.log.debug(`Got ${recipes.length} recipes from Remote Settings`);
     } catch (e) {
       lazy.log.debug("Error getting recipes from remote settings.");
@@ -293,10 +290,7 @@ class _RemoteSettingsExperimentLoader {
     try {
       recipes = await lazy
         .RemoteSettings(collection || lazy.COLLECTION_ID)
-        .get({
-          // Throw instead of returning an empty list.
-          emptyListFallback: false,
-        });
+        .get();
     } catch (e) {
       Cu.reportError(e);
       throw new Error("Error getting recipes from remote settings.");

toolkit/components/normandy/lib/RecipeRunner.jsm

@@ -333,10 +333,7 @@ var RecipeRunner = {
       // Fetch recipes before execution in case we fail and exit early.
       let recipesAndSignatures;
       try {
-        recipesAndSignatures = await lazy.gRemoteSettingsClient.get({
-          // Do not return an empty list if an error occurs.
-          emptyListFallback: false,
-        });
+        recipesAndSignatures = await lazy.gRemoteSettingsClient.get();
       } catch (e) {
         await lazy.Uptake.reportRunner(lazy.Uptake.RUNNER_SERVER_ERROR);
         return;