fix(place-names): a load starts with no failures, so packs that failed earlier do not mark a retry that lands as failed

This commit is contained in:
Sameh Khamis committed 2026-11-08 20:04:40 -08:00
1 parent c5e1951f51
commit 2f31828d28
2 files changed
+49 -8

No files matched your search

+15 -8
View File
@@ -47,7 +47,11 @@ const PACKS = {
),
};
/** The packs on their way now, and those whose last load failed. */
/**
* The packs on their way now, and those that failed in the current load (or
* the last one): a load starts with no failures, so a retry that lands is
* not blamed for a pack that failed before.
*/
const loadingPacks = new Set();
const failedPacks = new Set();
// One lookup loads its packs one after another (countries and seas, then
@@ -67,9 +71,14 @@ function notifyLoading() {
}
}
/** Whether the status shows a load on its way. */
function loadingNow() {
return loadingPacks.size > 0 || (lookupLoaded && lookupsInFlight > 0);
}
/**
* A pack load that the loading status follows: on its way while it runs,
* failed from a failure until it is tried again.
* and failed, if it fails, until the next load starts.
* @template T
* @param {string} name
* @param {() => Promise<T>} load
@@ -77,15 +86,13 @@ function notifyLoading() {
*/
export function reportedPlacePackLoad(name, load) {
return async () => {
// A retry is a fresh try: its batch ends on what this load does.
failedPacks.delete(name);
// A fresh load forgets earlier failures: it ends on what it loads.
if (!loadingNow()) failedPacks.clear();
if (lookupsInFlight > 0) lookupLoaded = true;
loadingPacks.add(name);
notifyLoading();
try {
const result = await load();
failedPacks.delete(name);
return result;
return await load();
} catch (error) {
failedPacks.add(name);
throw error;
@@ -104,7 +111,7 @@ export function reportedPlacePackLoad(name, load) {
*/
export function placePacksStatus() {
return {
loading: loadingPacks.size > 0 || (lookupLoaded && lookupsInFlight > 0),
loading: loadingNow(),
failed: failedPacks.size > 0,
};
}
+34
View File
@@ -55,3 +55,37 @@ test('one lookup that loads several packs reads as one load', async () => {
assert.deepEqual(heard, [true, false], 'no gap between the packs');
stop();
});
test('a load is not blamed for packs that failed in an earlier one', async () => {
const place = await import('./placeBelow.js?two-failed');
let fail = true;
const pack = (name) =>
place.reportedPlacePackLoad(name, async () => {
if (fail) throw new Error('offline');
return name;
});
const divisions = pack('divisions');
const places = pack('places');
await assert.rejects(divisions());
await assert.rejects(places());
assert.equal(place.placePacksStatus().failed, true);
fail = false;
// One lookup retries both, one after the other.
const lookup = place.placeBelow(48.86, 2.35, {
detail: {
division: async () => divisions(),
near: async () => places(),
},
});
const statuses = [];
const stop = place.onPlacePacksLoadingChange(() =>
statuses.push(place.placePacksStatus()),
);
await lookup;
stop();
assert.ok(statuses.length > 0);
assert.ok(
statuses.every((status) => status.failed === false),
'the earlier failures do not colour this load',
);
});