Bug 1663428 - Add a retry for region lookup r=Standard8

Differential Revision: https://phabricator.services.mozilla.com/D89937
This commit is contained in:
Dale Harvey 2020-09-21 08:06:19 +00:00
Родитель 63a7455a39
Коммит baf148dc4b
2 изменённых файлов: 106 добавлений и 18 удалений

Просмотреть файл

@ -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");
}

Просмотреть файл

@ -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);