Keep fallback retries single-flight and recover imagery and terrain independently

An activation that re-showed a fallback while a retry was in flight armed a
second timer, so two retries could run at once and land out of order, leaving
the cache and the screen on different providers. Retries now run one at a
time; a request made meanwhile re-arms once the current one ends, and a
result that is obsolete or no longer wanted is disposed.

Imagery and terrain retries are now handled as each settles, so a stalled
terrain attempt no longer keeps recovered imagery off screen, or the reverse.

changelog.d/1111-map-fallback-retry.md

@@ -0,0 +1,5 @@
+- A failed Esri Satellite or keyless terrain load still shows OSM or flat terrain
+  at once, but no longer lasts the whole session: while the fallback is on
+  screen, the real provider is retried in the background (30 seconds, doubling
+  to a 10-minute cap) and replaces it live once it loads. Map switches never
+  wait for a retry (Aditya, @Rohilalala, #1111).

docs/CURRENT-STATE.md

@@ -386,8 +386,9 @@ upstream exposes it. One console warning is logged per cooldown window. Other
 failures keep Cesium's handling and the flat `EllipsoidTerrainProvider`
 fallback; while that flat terrain is on a shown globe the map controller
 retries Re:Earth in the background (30 s, doubling to a 10-minute cap) and
-installs the real terrain without waiting for a map switch. Behind a 3D tileset
-it neither retries nor installs; the next globe activation does.
+installs the real terrain without waiting for a map switch or an imagery
+retry. Behind a 3D tileset it neither retries nor installs; the next globe
+activation does.
 `scripts/qa-terrain-429.mjs` reproduces the burst against a keyless
 dev server, counts real throttles with their headers as evidence, synthesises
 throttles with `--inject N` when the upstream is not throttling, and fails if

src/maps/controller.js

@@ -61,6 +61,8 @@ export class MapSourceController {
     this._terrainMode = null;
     this._terrainFallback = null;
     this._retryTimer = null;
+    this._retrying = false;
+    this._retryRequested = false;
     this._retryDelay = FALLBACK_RETRY_MS;
     this._subscribers = new Set();
     this._destroyed = false;
@@ -329,9 +331,14 @@ export class MapSourceController {
    * screen: Esri's OSM stand-in, or flat terrain on a shown globe. Fallbacks
    * stay cached and shown meanwhile, so no switch waits on a retry; a
    * fallback out of use is left alone until an activation shows it again.
+   * One retry runs at a time; a request made meanwhile re-arms once it ends.
    */
   _scheduleRetry() {
-    if (this._retryTimer) return;
+    if (this._retryTimer || this._destroyed) return;
+    if (this._retrying) {
+      this._retryRequested = true;
+      return;
+    }
     this._retryTimer = setTimeout(() => {
       this._retryTimer = null;
       void this._retryFallbacks();
@@ -345,6 +352,8 @@ export class MapSourceController {
     let terrain = flat && this._sources.get(this._activeId)?.terrain;
     if (terrain?.id !== this._terrainMode) terrain = null;
     if (!id && !terrain) return;
+    this._retrying = true;
+    this._retryRequested = false;
     const attempt = async (create) => {
       try {
         return await create?.({ signal: this._abort.signal });
@@ -352,18 +361,16 @@ export class MapSourceController {
         return null; // Still unavailable: keep the fallback.
       }
     };
-    const [provider, real] = await Promise.all([
-      id && attempt(this._sources.get(id).imagery),
-      terrain && attempt(terrain.create),
-    ]);
-    if (this._destroyed) {
+    // Each recovers as soon as it loads, never waiting on the other.
+    const retryImagery = async () => {
+      const cached = this._imageryProviders.get(id);
+      const provider = await attempt(this._sources.get(id).imagery);
+      // Obsolete: destroyed, or the cache moved on while this was in flight.
+      if (this._destroyed || this._imageryProviders.get(id) !== cached) {
         this._dispose(provider);
-      this._dispose(real?.provider);
-      return;
+        return true;
       }
-    let failed = (id && !provider) || (terrain && (!real || real.fallback));
-    try {
-      if (provider) {
+      if (!provider) return false;
       this._imageryProviders.set(id, Promise.resolve(resolved(provider, id)));
       // Still showing the fallback once any switch settles: use the real one.
       const recover = () => {
@@ -377,22 +384,47 @@ export class MapSourceController {
           recover();
         });
       }
+      return true;
+    };
+    const retryTerrain = async () => {
+      const cached = this._terrainProviders.get(terrain.id);
+      const real = await attempt(terrain.create);
+      if (
+        this._destroyed ||
+        this._terrainProviders.get(terrain.id) !== cached ||
+        !real ||
+        real.fallback
+      ) {
+        this._dispose(real?.provider);
+        return Boolean(real && !real.fallback);
       }
-      if (terrain && real && !real.fallback) {
       this._terrainProviders.set(terrain.id, Promise.resolve(real));
       if (this._terrainFallback === flat && this.viewer.scene.globe.show) {
         this._applyTerrain(real);
         this._requestRender('map-stack');
       } else this._terrainMode = null; // The next globe activation installs it.
-      }
-    } catch (error) {
+      return true;
+    };
+    const settled = (retry) =>
+      retry().catch((error) => {
         console.warn('[MapSourceController] fallback recovery failed:', error);
-      failed = true;
+        return false;
+      });
+    let failed = false;
+    try {
+      const results = await Promise.all([
+        id && settled(retryImagery),
+        terrain && settled(retryTerrain),
+      ]);
+      failed = results.includes(false);
+    } finally {
+      this._retrying = false;
     }
+    if (this._destroyed) return;
     this._retryDelay = failed
       ? Math.min(this._retryDelay * 2, FALLBACK_RETRY_MAX_MS)
       : FALLBACK_RETRY_MS;
-    if (failed) this._scheduleRetry();
+    if (failed || this._retryRequested) this._scheduleRetry();
   }
 
   _cached(cache, id, create) {

src/maps/controller.test.mjs

@@ -1098,3 +1098,85 @@ test('destroy cancels a pending retry and releases one in flight', async (t) =>
   await settle();
   assert.equal(lateEsri.attempts, 2);
 });
+
+test('retries stay single-flight when an activation re-shows the fallback mid-retry', async (t) => {
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  const env = publicFixture();
+  const created = [],
+    pending = [];
+  let attempts = 0;
+  env.registry.sources.find(
+    (source) => source.descriptor.id === 'esri-imagery',
+  ).imagery = async () => {
+    if (++attempts === 1) throw new Error('Esri unreachable');
+    const gate = deferred();
+    pending.push(gate);
+    await gate.promise;
+    const provider = { id: `esri-${attempts}`, errorEvent: event() };
+    provider.destroy = () => (provider.destroyed = true);
+    created.push(provider);
+    return provider;
+  };
+  await env.controller.setStack('esri-imagery');
+  t.mock.timers.tick(30_000);
+  await settle();
+  assert.equal(attempts, 2, 'retry #1 is in flight');
+  await env.controller.setStack('osm');
+  await env.controller.setStack('esri-imagery');
+  assert.equal(env.controller.getActiveId(), 'osm', 'the fallback is shown');
+  t.mock.timers.tick(30_000);
+  await settle();
+  assert.equal(attempts, 2, 'no second retry while one is in flight');
+  for (const gate of pending.reverse()) {
+    gate.resolve();
+    await settle();
+  }
+  const shown = env.imagery.at(-1).provider;
+  assert.equal(env.controller.getActiveId(), 'esri-imagery');
+  await env.controller.setStack('osm');
+  await env.controller.setStack('esri-imagery');
+  assert.equal(env.imagery.at(-1).provider, shown, 'the cached one is shown');
+  env.controller.destroy();
+  await settle();
+  assert.ok(created.length > 0);
+  for (const provider of created)
+    assert.equal(provider.destroyed, true, `${provider.id} is disposed`);
+});
+
+test('a stalled terrain retry does not hold back recovered imagery', async (t) => {
+  const stalled = deferred();
+  const { env, terrain } = terrainFixture(t, async (n) => {
+    if (n > 1) await stalled.promise;
+    return flatTerrain(n);
+  });
+  const esri = failingEsri(env);
+  await env.controller.setStack('esri-imagery');
+  assert.equal(env.controller.getActiveId(), 'osm');
+  esri.failing = false;
+  t.mock.timers.tick(30_000);
+  await settle();
+  assert.equal(terrain.calls, 2, 'the terrain retry is in flight');
+  assert.equal(env.controller.getActiveId(), 'esri-imagery');
+  assert.equal(env.imagery.at(-1).provider, env.providers.get('esri-imagery'));
+  stalled.resolve();
+  await settle();
+  env.controller.destroy();
+});
+
+test('a stalled imagery retry does not hold back recovered terrain', async (t) => {
+  const real = { id: 'real' };
+  const { env, installed } = terrainFixture(t, async (n) =>
+    n > 1 ? { provider: real } : flatTerrain(n),
+  );
+  const esri = failingEsri(env);
+  await env.controller.setStack('esri-imagery');
+  const stalled = deferred();
+  esri.next = stalled.promise;
+  t.mock.timers.tick(30_000);
+  await settle();
+  assert.equal(esri.attempts, 2, 'the imagery retry is in flight');
+  assert.equal(installed.at(-1), real, 'real terrain is installed');
+  stalled.resolve();
+  await settle();
+  env.controller.destroy();
+});