diff --git a/src/data/placeBelow.js b/src/data/placeBelow.js index e8456973a..609c90907 100644 --- a/src/data/placeBelow.js +++ b/src/data/placeBelow.js @@ -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} 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, }; } diff --git a/src/data/placePacksLoading.test.mjs b/src/data/placePacksLoading.test.mjs index 6aa3f69d3..e58fa9948 100644 --- a/src/data/placePacksLoading.test.mjs +++ b/src/data/placePacksLoading.test.mjs @@ -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', + ); +});