diff --git a/toolkit/modules/Region.jsm b/toolkit/modules/Region.jsm index b363983cf8e2..8e277ac89248 100644 --- a/toolkit/modules/Region.jsm +++ b/toolkit/modules/Region.jsm @@ -37,6 +37,16 @@ XPCOMUtils.defineLazyPreferenceGetter( 5000 ); +// Retry the region lookup every hour on failure, a failure +// is likely to be a service failure so this gives the +// service some time to restore. Setting to 0 disabled retries. +XPCOMUtils.defineLazyPreferenceGetter( + this, + "retryTimeout", + "browser.region.retry-timeout", + 60 * 60 * 1000 +); + XPCOMUtils.defineLazyPreferenceGetter( this, "loggingEnabled", @@ -74,6 +84,8 @@ const UPDATE_PREFIX = "browser.region.update"; // Currently set to 2 weeks. const UPDATE_INTERVAL = 60 * 60 * 24 * 14; +const MAX_RETRIES = 3; + /** * This module keeps track of the users current region (country). * so the SearchService and other consumers can apply region @@ -88,6 +100,9 @@ class RegionDetector { _rsClient = null; // Keep track of the wifi data across listener events. _wifiDataPromise = null; + // Keep track of how many times we have tried to fetch + // the users region during failure. + _retryCount = 0; // Topic for Observer events fired by Region.jsm. REGION_TOPIC = "browser-region"; // Verb for event fired when we update the region. @@ -143,6 +158,9 @@ class RegionDetector { * The country_code defining users current region. */ async _fetchRegion() { + if (this._retryCount >= MAX_RETRIES) { + return null; + } let startTime = Date.now(); let telemetryResult = this.TELEMETRY.SUCCESS; let result = null; @@ -152,6 +170,12 @@ class RegionDetector { } catch (err) { telemetryResult = this.TELEMETRY[err.message] || this.TELEMETRY.ERROR; log.error("Failed to fetch region", err); + if (retryTimeout) { + this._retryCount++; + setTimeout(() => { + Services.tm.idleDispatchToMainThread(this._fetchRegion.bind(this)); + }, retryTimeout); + } } let took = Date.now() - startTime; @@ -348,7 +372,8 @@ class RegionDetector { return res.country_code; } catch (err) { log.error("Error fetching region", err); - throw new Error("NO_RESULT"); + let errCode = err.message in this.TELEMETRY ? err.message : "NO_RESULT"; + throw new Error(errCode); } } @@ -694,7 +719,9 @@ class RegionDetector { async _timeout(timeout, controller) { await new Promise(resolve => setTimeout(resolve, timeout)); if (controller) { - controller.abort(); + // Yield so it is the TIMEOUT that is returned and not + // the result of the abort(). + setTimeout(() => controller.abort(), 0); } throw new Error("TIMEOUT"); } diff --git a/toolkit/modules/tests/xpcshell/test_Region.js b/toolkit/modules/tests/xpcshell/test_Region.js index 8a480c99acb0..6a9dd3984935 100644 --- a/toolkit/modules/tests/xpcshell/test_Region.js +++ b/toolkit/modules/tests/xpcshell/test_Region.js @@ -32,16 +32,23 @@ function waitForNotificationSubject(topic) { }); } +// Region.jsm will call init() on startup and sent a background +// task to fetch the region, ensure we have completed this before +// running the rest of the tests. +add_task(async function test_startup() { + setNetworkRegion("UK"); + await checkTelemetry(Region.TELEMETRY.SUCCESS); +}); + add_task(async function test_basic() { + Region._home = null; let srv = useHttpServer(REGION_PREF); srv.registerPathHandler("/", (req, res) => { res.setStatusLine("1.1", 200, "OK"); send(res, { country_code: "UK" }); }); // start to listen the notification - let notificationSubjectPromise = await waitForNotificationSubject( - "browser-region" - ); + let notificationSubjectPromise = waitForNotificationSubject("browser-region"); await Region._fetchRegion(); let notificationSub = await notificationSubjectPromise; @@ -58,10 +65,11 @@ add_task(async function test_basic() { add_task(async function test_invalid_url() { histogram.clear(); + Services.prefs.setIntPref("browser.region.retry-timeout", 0); Services.prefs.setCharPref(REGION_PREF, "http://localhost:0"); let result = await Region._fetchRegion(); Assert.ok(!result, "Should return no result"); - checkTelemetry(Region.TELEMETRY.ERROR); + await checkTelemetry(Region.TELEMETRY.NO_RESULT); }); add_task(async function test_invalid_json() { @@ -72,29 +80,26 @@ add_task(async function test_invalid_json() { ); let result = await Region._fetchRegion(); Assert.ok(!result, "Should return no result"); - checkTelemetry(Region.TELEMETRY.NO_RESULT); + await checkTelemetry(Region.TELEMETRY.NO_RESULT); }); add_task(async function test_timeout() { histogram.clear(); + Services.prefs.setIntPref("browser.region.retry-timeout", 0); Services.prefs.setIntPref("browser.region.timeout", RESPONSE_TIMEOUT); let srv = useHttpServer(REGION_PREF); - srv.registerPathHandler("/geo", (req, res) => { + srv.registerPathHandler("/", (req, res) => { res.processAsync(); do_timeout(RESPONSE_DELAY, () => { - res.setStatusLine("1.1", 200, "OK"); send(res, { country_code: "UK" }); + res.finish(); }); }); - try { - let result = await Region._fetchRegion(); - Assert.equal(result, null, "Region fetch should return null"); - } catch (err) { - Assert.ok(false, "fetchRegion doesn't throw"); - } - checkTelemetry(Region.TELEMETRY.TIMEOUT); + let result = await Region._fetchRegion(); + Assert.equal(result, null, "Region fetch should return null"); + await checkTelemetry(Region.TELEMETRY.TIMEOUT); await new Promise(r => srv.stop(r)); }); @@ -111,7 +116,7 @@ add_task(async function test_mismatched_probe() { setNetworkRegion("AU"); await Region._fetchRegion(); Assert.equal(Region.home, "AU", "Should have correct region"); - checkTelemetry(Region.TELEMETRY.SUCCESS); + await checkTelemetry(Region.TELEMETRY.SUCCESS); // We dont store probes for linux and on treeherder + // Mac there is no plaform countryCode so in these cases @@ -158,6 +163,60 @@ add_task(async function test_update() { Assert.equal(Region.home, "DE", "Should have updated now"); }); +add_task(async function test_max_retry() { + Region._home = null; + let requestsSeen = 0; + Services.prefs.setIntPref("browser.region.retry-timeout", RESPONSE_TIMEOUT); + Services.prefs.setIntPref("browser.region.timeout", RESPONSE_TIMEOUT); + let srv = useHttpServer(REGION_PREF); + srv.registerPathHandler("/", (req, res) => { + requestsSeen++; + res.setStatusLine("1.1", 200, "OK"); + res.processAsync(); + do_timeout(RESPONSE_DELAY, res.finish.bind(res)); + }); + + Region._fetchRegion(); + await TestUtils.waitForCondition(() => requestsSeen === 3); + /* eslint-disable mozilla/no-arbitrary-setTimeout */ + await new Promise(resolve => setTimeout(resolve, RESPONSE_DELAY)); + + Assert.equal(Region.home, null, "failed to fetch region"); + Assert.equal(requestsSeen, 3, "Retried 4 times"); + + Region._retryCount = 0; + await new Promise(r => srv.stop(r)); +}); + +add_task(async function test_retry() { + Region._home = null; + let requestsSeen = 0; + Services.prefs.setIntPref("browser.region.retry-timeout", RESPONSE_TIMEOUT); + Services.prefs.setIntPref("browser.region.timeout", RESPONSE_TIMEOUT); + let srv = useHttpServer(REGION_PREF); + srv.registerPathHandler("/", (req, res) => { + res.setStatusLine("1.1", 200, "OK"); + if (++requestsSeen == 2) { + res.setStatusLine("1.1", 200, "OK"); + send(res, { country_code: "UK" }); + } else { + res.processAsync(); + do_timeout(RESPONSE_DELAY, res.finish.bind(res)); + } + }); + + Region._fetchRegion(); + await TestUtils.waitForCondition(() => requestsSeen === 2); + /* eslint-disable mozilla/no-arbitrary-setTimeout */ + await new Promise(resolve => setTimeout(resolve, RESPONSE_DELAY)); + + Assert.equal(Region.home, "UK", "failed to fetch region"); + Assert.equal(requestsSeen, 2, "Retried 2 times"); + + Region._retryCount = 0; + await new Promise(r => srv.stop(r)); +}); + function setNetworkRegion(region) { Services.prefs.setCharPref( REGION_PREF, @@ -182,8 +241,10 @@ function send(res, json) { } async function checkTelemetry(aExpectedValue) { + // Wait until there is 1 result. await TestUtils.waitForCondition(() => { - return histogram.snapshot().sum == 1; + let snapshot = histogram.snapshot(); + return Object.values(snapshot.values).reduce((a, b) => a + b) == 1; }); let snapshot = histogram.snapshot(); Assert.equal(snapshot.values[aExpectedValue], 1);