فهرست منبع

fix(ui-settings): clear the mirror in-flight slot in the rerun check's segment

Yichen Jiang 1 ماه پیش
والد
کامیت
4db77d3988

+ 37 - 29
packages/client/ui-settings/src/client/settings-mirror.ts

@@ -89,7 +89,7 @@ export class SettingsDescribeMirror {
       this.rerun = true
       return this.inFlight
     }
-    const run = this.run().finally(() => { this.inFlight = undefined })
+    const run = this.run()
     this.inFlight = run
     return run
   }
@@ -132,33 +132,41 @@ export class SettingsDescribeMirror {
   }
 
   private async run(): Promise<void> {
-    do {
-      this.rerun = false
-      const generation = ++this.generation
-      const before = this.store.getSnapshot()
-      if (before.status === 'idle') this.store.set({ ...before, status: 'loading' })
-      let outcome: { view: SettingsDescribeView } | { failure: string }
-      try {
-        const response = await this.api.settings.describe({})
-        outcome = response.result.ok
-          ? { view: response.result.value }
-          : { failure: response.result.error.message }
-      } catch (error) {
-        outcome = { failure: error instanceof Error ? error.message : String(error) }
-      }
-      if (generation !== this.generation) continue
-      if ('view' in outcome) {
-        this.store.set({ status: 'ready', view: outcome.view, error: null })
-      } else {
-        const held = this.store.getSnapshot()
-        // No answer yet: fall back to idle so `ensure` retries; with one, the
-        // held view keeps serving and only the error field reports the miss.
-        this.store.set({
-          status: held.view === undefined ? 'idle' : 'ready',
-          view: held.view,
-          error: outcome.failure,
-        })
-      }
-    } while (this.rerun)
+    // The in-flight slot must clear in the same synchronous segment that
+    // observes `rerun` false (and on abrupt exit): a `.finally()` on the
+    // returned promise runs one microtask later, and a `load()` landing in
+    // that gap would mark a rerun nobody reads, losing the read.
+    try {
+      do {
+        this.rerun = false
+        const generation = ++this.generation
+        const before = this.store.getSnapshot()
+        if (before.status === 'idle') this.store.set({ ...before, status: 'loading' })
+        let outcome: { view: SettingsDescribeView } | { failure: string }
+        try {
+          const response = await this.api.settings.describe({})
+          outcome = response.result.ok
+            ? { view: response.result.value }
+            : { failure: response.result.error.message }
+        } catch (error) {
+          outcome = { failure: error instanceof Error ? error.message : String(error) }
+        }
+        if (generation !== this.generation) continue
+        if ('view' in outcome) {
+          this.store.set({ status: 'ready', view: outcome.view, error: null })
+        } else {
+          const held = this.store.getSnapshot()
+          // No answer yet: fall back to idle so `ensure` retries; with one, the
+          // held view keeps serving and only the error field reports the miss.
+          this.store.set({
+            status: held.view === undefined ? 'idle' : 'ready',
+            view: held.view,
+            error: outcome.failure,
+          })
+        }
+      } while (this.rerun)
+    } finally {
+      this.inFlight = undefined
+    }
   }
 }

+ 14 - 0
packages/client/ui-settings/tests/settings-mirror.client.spec.ts

@@ -125,6 +125,20 @@ describe('SettingsDescribeMirror', () => {
     expect(mirror.getSnapshot().view?.namespaces).toHaveLength(2)
   })
 
+  it('never loses a load landing between a run settling and its slot clearing', async () => {
+    // Regression: with the in-flight slot cleared by a promise .finally(),
+    // a load() in the one-microtask gap after the rerun check marked a rerun
+    // nobody read, and that refresh never reached the wire.
+    const describeCall = vi.fn().mockResolvedValue(described([view('theme', 1)]))
+    const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never)
+    void mirror.load()
+    await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(1) })
+    void mirror.load()
+    await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(2) })
+    void mirror.load()
+    await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(3) })
+  })
+
   it('suppresses a stale answer that lost to a newer generation', async () => {
     const slow = deferred<RpcResponse<SettingsDescribeView>>()
     const describeCall = vi.fn()