diff --git a/services/settings/RemoteSettingsClient.jsm b/services/settings/RemoteSettingsClient.jsm index ae0297438247..222619c8420c 100644 --- a/services/settings/RemoteSettingsClient.jsm +++ b/services/settings/RemoteSettingsClient.jsm @@ -348,9 +348,45 @@ class RemoteSettingsClient extends EventEmitter { } = options; let { verifySignature = false } = options; - let hasLocalData; + let data; try { - hasLocalData = await Utils.hasLocalData(this); + let hasLocalData = await Utils.hasLocalData(this); + + if (syncIfEmpty && !hasLocalData) { + // .get() was called before we had the chance to synchronize the local database. + // We'll try to avoid returning an empty list. + if (!this._importingPromise) { + // Prevent parallel loading when .get() is called multiple times. + this._importingPromise = (async () => { + const importedFromDump = gLoadDump + ? await this._importJSONDump() + : -1; + if (importedFromDump < 0) { + // There is no JSON dump to load, force a synchronization from the server. + console.debug( + `${this.identifier} Local DB is empty, pull data from server` + ); + await this.sync({ loadDump: false }); + } + })(); + } + try { + await this._importingPromise; + } catch (e) { + // Report error, but continue because there could have been data + // loaded from a parrallel call. + Cu.reportError(e); + } finally { + // then delete this promise again, as now we should have local data: + delete this._importingPromise; + } + // No need to verify signature, because either we've just load a trusted + // dump (here or in a parallel call), or it was verified during sync. + verifySignature = false; + } + + // Read from the local DB. + data = await this.db.list({ filters, order }); } catch (e) { // If the local DB cannot be read (for unknown reasons, Bug 1649393) // We fallback to the packaged data, and filter/sort in memory. @@ -380,41 +416,6 @@ class RemoteSettingsClient extends EventEmitter { return this._filterEntries(data); } - if (syncIfEmpty && !hasLocalData) { - // .get() was called before we had the chance to synchronize the local database. - // We'll try to avoid returning an empty list. - if (!this._importingPromise) { - // Prevent parallel loading when .get() is called multiple times. - this._importingPromise = (async () => { - const importedFromDump = gLoadDump - ? await this._importJSONDump() - : -1; - if (importedFromDump < 0) { - // There is no JSON dump to load, force a synchronization from the server. - console.debug( - `${this.identifier} Local DB is empty, pull data from server` - ); - await this.sync({ loadDump: false }); - } - })(); - } - try { - await this._importingPromise; - } catch (e) { - // Report error, but continue because there could have been data - // loaded from a parrallel call. - Cu.reportError(e); - } finally { - // then delete this promise again, as now we should have local data: - delete this._importingPromise; - } - // No need to verify signature, because either we've just load a trusted - // dump (here or in a parallel call), or it was verified during sync. - verifySignature = false; - } - - // Read from the local DB. - const data = await this.db.list({ filters, order }); console.debug( `${this.identifier} ${data.length} records before filtering.` ); diff --git a/services/settings/test/unit/test_remote_settings.js b/services/settings/test/unit/test_remote_settings.js index 9b9524111b48..c29673e5196c 100644 --- a/services/settings/test/unit/test_remote_settings.js +++ b/services/settings/test/unit/test_remote_settings.js @@ -286,7 +286,7 @@ add_task(async function test_get_falls_back_to_dump_if_db_fails() { throw new Error("Unknown error"); }; - const records = await clientWithDump.get(); + const records = await clientWithDump.get({ dumpFallback: true }); ok(records.length > 0, "dump content is returned"); // If fallback is disabled, error is thrown. @@ -302,6 +302,32 @@ add_task(async function test_get_falls_back_to_dump_if_db_fails() { }); add_task(clear_state); +add_task(async function test_get_falls_back_to_dump_if_db_fails_later() { + if (IS_ANDROID) { + // Skip test: we don't ship remote settings dumps on Android (see package-manifest). + return; + } + const backup = clientWithDump.db.list; + clientWithDump.db.list = () => { + throw new Error("Unknown error"); + }; + + const records = await clientWithDump.get({ dumpFallback: true }); + ok(records.length > 0, "dump content is returned"); + + // If fallback is disabled, error is thrown. + let error; + try { + await clientWithDump.get({ dumpFallback: false }); + } catch (e) { + error = e; + } + equal(error.message, "Unknown error"); + + clientWithDump.db.list = 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).