feedback3 WJ: emote manifest timeout falls back to the glyph

public/js/data.js

@@ -6,7 +6,10 @@
 // GAME_FILES) in the background once the player is in a room, and the match screen waits for them, so no text of the
 // match UI ever appears late. A transient failure (network error, HTTP 5xx) is retried twice (RETRY_DELAYS_MS); files
 // that stay unavailable (404, repeated failures, bad JSON) resolve to `null` and are reported once on the console — the
-// UI must degrade gracefully while data is being generated.
+// UI must degrade gracefully while data is being generated. The emote manifests (`local`, `assets`) are the exception:
+// one attempt is abandoned after ART_MANIFEST_TIMEOUT_MS (GitHub #99) and reported `missing` at once, so the 交流 button
+// draws its glyph instead of staying blank; a later retry that succeeds replaces the glyph. Other files stay `loading`
+// across their retries.
 //
 // Each file is indexed tolerantly so the getters work whether a file is
 //   - an array of records carrying an id field (id / chessId / bondId / itemId / …),
@@ -78,6 +81,16 @@ export function buildIndex(name, json) {
 /** Waits (ms) before retrying a data file whose download failed transiently (network error, HTTP 5xx / 408 / 429). */
 export const RETRY_DELAYS_MS = Object.freeze([600, 2000]);
 
+/**
+ * How long one attempt at an emote manifest (`local`, `assets`) may hang before it counts as a failure.
+ * [ASSUMED] 8 s: long enough for a slow link, short enough that the 交流 button does not stay blank for the session
+ * (GitHub #99). Other files are not on this clock.
+ */
+export const ART_MANIFEST_TIMEOUT_MS = 8000;
+
+/** Manifests the emote button waits on. A hang here used to leave every cell blank (GitHub #99). */
+const ART_MANIFESTS = new Set(['local', 'assets']);
+
 /**
  * A failed load worth retrying: the request itself failed (network error) or the server answered 5xx / 408 / 429 — not a
  * definite 4xx (the file is not there) nor a delivered file that is not valid JSON (`badJson`).
@@ -92,14 +105,19 @@ const transientFailure = (err) => {
  * Create a data store bound to a fetch implementation (injectable for tests).
  * A file is downloaded once per page (the texts of the game are static data, never fetched again during a match —
  * user playtest #3 item 9); a transient failure is retried (RETRY_DELAYS_MS) while the file stays 'loading', so a
- * network hiccup does not leave the texts of a whole session missing.
- * @param {{ fetch?: typeof fetch, base?: string, retryDelays?: number[], wait?: (ms: number) => Promise<void> }} [opts]
+ * network hiccup does not leave the texts of a whole session missing. `local` and `assets` are reported `missing` on
+ * the first failure or timeout (the emote glyph) and stay that way through a retry; a later success is `ready`.
+ * @param {{ fetch?: typeof fetch, base?: string, retryDelays?: number[], wait?: (ms: number) => Promise<void>, timeoutMs?: number, setTimeout?: typeof setTimeout, clearTimeout?: typeof clearTimeout }} [opts]
+ *   `timeoutMs` 0 turns the art-manifest clock off. `setTimeout` / `clearTimeout` let a test fire that clock.
  */
 export function createDataStore(opts = {}) {
   const base = opts.base ?? '/data/';
   const doFetch = opts.fetch || ((...a) => globalThis.fetch(...a));
   const retryDelays = Array.isArray(opts.retryDelays) ? opts.retryDelays : RETRY_DELAYS_MS;
   const wait = opts.wait || ((ms) => new Promise((resolve) => setTimeout(resolve, ms)));
+  const timeoutMs = opts.timeoutMs === undefined ? ART_MANIFEST_TIMEOUT_MS : Number(opts.timeoutMs);
+  const armTimer = opts.setTimeout || ((fn, ms) => setTimeout(fn, ms));
+  const disarmTimer = opts.clearTimeout || ((id) => clearTimeout(id));
   /** @type {Map<string, { status: 'loading'|'ready'|'missing', promise: Promise<any>, value: any, index: Map<string, any>|null }>} */
   const entries = new Map();
   const listeners = new Set();
@@ -113,40 +131,88 @@ export function createDataStore(opts = {}) {
 
   const urlFor = (name) => base + (DATA_FILES[name] || `${name}.json`);
 
+  /**
+   * One attempt at a file. Art manifests reject with a transient `{ timeout: true }` when the clock fires; a response
+   * that arrives after that is ignored (the retry is a new request). [ASSUMED]
+   */
+  function readJson(name) {
+    const run = async () => {
+      const res = await doFetch(urlFor(name), { cache: 'no-cache' });
+      if (!res || !res.ok) throw Object.assign(new Error(`HTTP ${res ? res.status : '???'}`), { status: res ? res.status : null });
+      try {
+        return await res.json();
+      } catch (err) {
+        throw Object.assign(err instanceof Error ? err : new Error(String(err)), { badJson: true });
+      }
+    };
+    if (!ART_MANIFESTS.has(name) || !(timeoutMs > 0)) return run();
+    let timer = null;
+    let done = false;
+    const finish = () => {
+      if (done) return false;
+      done = true;
+      if (timer != null) disarmTimer(timer);
+      return true;
+    };
+    const timed = new Promise((_, reject) => {
+      timer = armTimer(() => {
+        if (!finish()) return;
+        reject(Object.assign(new Error('timeout'), { timeout: true }));
+      }, timeoutMs);
+    });
+    const req = run().then(
+      (json) => { finish(); return json; },
+      (err) => { finish(); throw err; },
+    );
+    return Promise.race([req, timed]);
+  }
+
   function load(name) {
     if (typeof name !== 'string' || !/^[A-Za-z0-9_-]+$/.test(name)) return Promise.resolve(null);
     const cur = entries.get(name);
     if (cur) return cur.promise;
     const entry = { status: 'loading', promise: null, value: null, index: null };
+    const art = ART_MANIFESTS.has(name);
     entry.promise = (async () => {
+      let toldMissing = false;
       for (let attempt = 0; ; attempt++) {
         try {
-          const res = await doFetch(urlFor(name), { cache: 'no-cache' });
-          if (!res || !res.ok) throw Object.assign(new Error(`HTTP ${res ? res.status : '???'}`), { status: res ? res.status : null });
-          let json;
-          try { json = await res.json(); } catch (err) { throw Object.assign(err instanceof Error ? err : new Error(String(err)), { badJson: true }); }
-          entry.value = json;
+          entry.value = await readJson(name);
           entry.status = 'ready';
           break;
         } catch (err) {
-          // a transient failure is tried again (still 'loading'), unless the load was superseded meanwhile
-          if (transientFailure(err) && attempt < retryDelays.length && entries.get(name) === entry) {
+          const current = entries.get(name) === entry;
+          // Decided before notify. Invalidating inside that notify leaves `again` true; the wait then sees
+          // the superseded entry and stops, so this attempt does not fetch again.
+          const again = transientFailure(err) && attempt < retryDelays.length && current;
+          // Emote art: glyph now. Stay `missing` through the retry wait — flipping back to `loading` blanks the button.
+          if (art && current && entry.status !== 'missing') {
+            entry.value = null;
+            entry.index = null;
+            entry.status = 'missing';
+            toldMissing = true;
+            notify(name);
+          }
+          if (again) {
             await wait(retryDelays[attempt]);
             if (entries.get(name) === entry) continue;
-            entry.status = 'missing'; // superseded by invalidate() meanwhile: the new load reports for itself
             break;
           }
-          if (!warned.has(name)) {
-            warned.add(name);
-            console.warn(`[data] ${urlFor(name)} unavailable (${err?.message || err}); continuing without it`);
+          if (current) {
+            if (!warned.has(name)) {
+              warned.add(name);
+              console.warn(`[data] ${urlFor(name)} unavailable (${err?.message || err}); continuing without it`);
+            }
+            entry.value = null;
+            entry.index = null;
+            entry.status = 'missing';
           }
-          entry.value = null;
-          entry.status = 'missing';
           break;
         }
       }
       // A load superseded by invalidate() must not announce itself (its entry is no longer cached).
-      if (entries.get(name) === entry) notify(name);
+      // An art manifest already announced `missing` does not announce that same status again.
+      if (entries.get(name) === entry && !(toldMissing && entry.status === 'missing')) notify(name);
       return entry.value;
     })();
     entries.set(name, entry);

public/js/ui/emotes.js

@@ -4,8 +4,9 @@
 //
 //   EmoteArt    the emote picture: the local-client art (data/local-assets.json → emoticon/<dir>/<picId>) first, else
 //               the copy setup downloads from the public mirror (data/assets.json → ui['emoticon/<dir>/<picId>'];
-//               GitHub issue #42), each tried in turn when one fails to load; a neutral glyph when neither is there,
-//               an empty box while a manifest is still loading.
+//               GitHub issue #42), each tried in turn when one fails to load; a neutral glyph when neither is there.
+//               An empty box only while a manifest is still in flight; a timeout or a failed manifest (GitHub #99)
+//               uses that glyph, and a later success replaces it.
 //   EmoteBubble the pop bubble beside the sender's avatar in the team panel (official emoji_bubble_bkg: a dark rounded
 //               square with a tail pointing left + the icon only); pop-in, 3 s, fade. The parent keys it by the emote's
 //               seq so a newer emote replaces the old one and pops again, and passes the arrival time (`at`) so a
@@ -68,7 +69,7 @@ export function emoteArtUrl(id) {
   return emoteArtUrls(id)[0] || null;
 }
 
-/** True while the local-art manifest or the asset manifest has not settled (EmoteArt shows an empty box meanwhile). */
+/** True while local or assets is still in flight (idle or loading). A timeout or a failure is `missing`: the glyph. */
 const artManifestsPending = () => ['local', 'assets'].some((n) => { const st = data.status(n); return st === 'loading' || st === 'idle'; });
 
 /** Official emote UI sprite (ui/battle: emoji_bubble_bkg, emoji_bkg, emoji_cell_bkg, emoji_btn, emoji_btn_disable). */

test/ui/emotes.test.js

@@ -351,6 +351,125 @@ describe('public/js/ui/emotes.js helpers', () => {
     assert.match(readFileSync(path.join(ROOT, 'public/index.html'), 'utf8'), /href="\/css\/emotes\.css"/);
   });
 
+  test('a hung emote manifest is missing when the clock fires (glyph), and a later retry replaces it', async () => {
+    const { createDataStore } = await import('../../public/js/data.js');
+    const timers = [];
+    let calls = 0;
+    let releaseWait;
+    const store = createDataStore({
+      timeoutMs: 1000,
+      retryDelays: [0],
+      wait: () => new Promise((r) => { releaseWait = r; }),
+      setTimeout: (fn, ms) => { timers.push({ fn, ms }); return timers.length; },
+      clearTimeout: () => {},
+      fetch: () => {
+        calls++;
+        if (calls === 1) return new Promise(() => {});
+        return Promise.resolve({ ok: true, status: 200, json: async () => ({ ok: true, groups: { a: { pic: { path: '/p.png' } } } }) });
+      },
+    });
+    const states = [];
+    store.subscribe(() => states.push(store.status('local')));
+    const p = store.load('local');
+    assert.equal(store.status('local'), 'loading');
+    assert.equal(timers.length, 1);
+    assert.equal(timers[0].ms, 1000);
+    timers[0].fn();
+    await new Promise((r) => setImmediate(r));
+    assert.equal(store.status('local'), 'missing', 'glyph as soon as the attempt times out, before the retry');
+    assert.equal(calls, 1, 'still on the first attempt while the retry waits');
+    releaseWait();
+    const value = await p;
+    assert.equal(calls, 2);
+    assert.equal(store.status('local'), 'ready');
+    assert.equal(value.ok, true);
+    assert.equal(store.get('local').groups.a.pic.path, '/p.png');
+    assert.deepEqual(states, ['missing', 'ready']);
+    for (const t of timers) t.fn();
+    assert.equal(store.status('local'), 'ready', 'a timer that fires after success does not demote the file');
+  });
+
+  test('a failed emote manifest (no clock) is missing before the retry, then ready; a 404 stays missing', async () => {
+    const { createDataStore } = await import('../../public/js/data.js');
+    let calls = 0;
+    const store = createDataStore({
+      timeoutMs: 1000,
+      retryDelays: [0],
+      wait: async () => {},
+      setTimeout: () => 1,
+      clearTimeout: () => {},
+      fetch: async () => {
+        calls++;
+        if (calls === 1) throw new TypeError('Failed to fetch');
+        return { ok: true, status: 200, json: async () => ({ groups: { emoticon: {} } }) };
+      },
+    });
+    const states = [];
+    store.subscribe(() => states.push(store.status('assets')));
+    const value = await store.load('assets');
+    assert.equal(value.groups.emoticon !== undefined, true);
+    assert.deepEqual(states, ['missing', 'ready']);
+    const warn = console.warn;
+    console.warn = () => {};
+    try {
+      const timers = [];
+      const gone = createDataStore({
+        timeoutMs: 5,
+        retryDelays: [],
+        setTimeout: (fn) => { timers.push(fn); return timers.length; },
+        clearTimeout: () => {},
+        fetch: () => new Promise(() => {}),
+      });
+      const p = gone.load('local');
+      assert.equal(timers.length, 1);
+      timers[0]();
+      assert.equal(await p, null);
+      assert.equal(gone.status('local'), 'missing');
+      let n = 0;
+      const missing = createDataStore({
+        timeoutMs: 5,
+        retryDelays: [0],
+        wait: async () => {},
+        setTimeout: () => 1,
+        clearTimeout: () => {},
+        fetch: async () => { n++; return { ok: false, status: 404, json: async () => ({}) }; },
+      });
+      assert.equal(await missing.load('assets'), null);
+      assert.equal(missing.status('assets'), 'missing');
+      assert.equal(n, 1, 'a 404 is not retried');
+    } finally { console.warn = warn; }
+  });
+
+  test('the art-manifest clock is not armed for other files; the default wait is ART_MANIFEST_TIMEOUT_MS', async () => {
+    const { createDataStore, ART_MANIFEST_TIMEOUT_MS } = await import('../../public/js/data.js');
+    assert.equal(ART_MANIFEST_TIMEOUT_MS, 8000);
+    const arms = [];
+    let release;
+    const store = createDataStore({
+      setTimeout: (fn, ms) => { arms.push(ms); return arms.length; },
+      clearTimeout: () => {},
+      fetch: (url) => new Promise((resolve) => { release = () => resolve(String(url).includes('chess')
+        ? { ok: false, status: 404, json: async () => ({}) }
+        : { ok: true, status: 200, json: async () => ({ ok: 1 }) }); }),
+    });
+    const local = store.load('local');
+    assert.deepEqual(arms, [8000]);
+    release();
+    assert.equal((await local).ok, 1);
+    arms.length = 0;
+    const warn = console.warn;
+    console.warn = () => {};
+    try {
+      const chess = store.load('chess');
+      await new Promise((r) => setImmediate(r));
+      assert.deepEqual(arms, [], 'chess is not on the emote clock');
+      assert.equal(store.status('chess'), 'loading');
+      release();
+      await chess;
+      assert.equal(store.status('chess'), 'missing');
+    } finally { console.warn = warn; }
+  });
+
   test('emote UI never renders an emote label as text (aria-label only)', () => {
     const src = readFileSync(path.join(ROOT, 'public/js/ui/emotes.js'), 'utf8');
     assert.doesNotMatch(src, /EMOTE_TEXT|EMOTE_LABEL/, 'no text table in the UI');