fix: end tracking when a selector throws

src/trackSelector.ts

@@ -1,5 +1,12 @@
 import { computeSelector } from './helpers';
-import type { ObserveOptions, ListenerParams, ObserveEvent, Selector, GetOptions } from './observableInterfaces';
+import type {
+    GetOptions,
+    ListenerParams,
+    ObserveEvent,
+    ObserveOptions,
+    Selector,
+    TrackingState,
+} from './observableInterfaces';
 import { setupTracking } from './setupTracking';
 import { beginTracking, endTracking, tracking } from './tracking';
 
@@ -14,14 +21,21 @@ export function trackSelector<T>(
     let dispose: undefined | (() => void);
     let resubscribe: (() => () => void) | undefined;
     let updateFn = update;
+    let value: T;
+    let tracker: TrackingState | undefined;
+    let nodes: TrackingState['nodes'];
 
     beginTracking();
-    const value = selector
-        ? computeSelector(selector, getOptions, observeEvent, observeOptions?.fromComputed)
-        : selector;
-    const tracker = tracking.current;
-    const nodes = tracker!.nodes;
-    endTracking();
+    try {
+        value = selector ? computeSelector(selector, getOptions, observeEvent, observeOptions?.fromComputed) : selector;
+    } finally {
+        // The selector can throw: an observer's component render that suspends (a thrown promise)
+        // or errors. Tracking has to end either way. Otherwise this tracker stays as tracking.current
+        // with trackCount above zero, and every later get() anywhere adds its node to it for good.
+        tracker = tracking.current;
+        nodes = tracker!.nodes;
+        endTracking();
+    }
 
     if ((process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'test') && tracker && nodes) {
         tracker.traceListeners?.(nodes);

tests/react.test.tsx

@@ -15,6 +15,7 @@ import { useObserve } from '../src/react/useObserve';
 import { useObserveEffect } from '../src/react/useObserveEffect';
 import { use$, useSelector } from '../src/react/useSelector';
 import { getNode } from '../src/globals';
+import { tracking } from '../src/tracking';
 import { Memo } from '../src/react/Memo';
 import { GlobalRegistrator } from '@happy-dom/global-registrator';
 import { useComputed } from '../src/react/useComputed';
@@ -580,6 +581,49 @@ describe('useSelector', () => {
             expect(items[0].textContent).toEqual('hi');
         });
     });
+    test('suspense thrown inside observer leaves no tracking behind', async () => {
+        await supressActWarning(async () => {
+            let isResolved = false;
+            const promise = new Promise<void>((resolve) =>
+                setTimeout(() => {
+                    isResolved = true;
+                    resolve();
+                }, 10),
+            );
+            const obs$ = observable({ value: 'hi' });
+            const isDone$ = observable(false);
+            const Test = observer(function Test() {
+                const value = obs$.value.get();
+                // A suspending hook from outside Legend-State (a data library's useSuspenseQuery)
+                // throws its promise from inside the observed render
+                if (!isResolved) {
+                    throw promise;
+                }
+                isDone$.set(true);
+
+                return createElement('div', undefined, value);
+            });
+            function App() {
+                return createElement(
+                    Suspense,
+                    { fallback: createElement('div', undefined, 'fallback') },
+                    createElement(Test),
+                );
+            }
+            const { container } = render(createElement(App));
+            let items = container.querySelectorAll('div');
+            expect(items[0].textContent).toEqual('fallback');
+            // The suspended render's tracker must not stay current, or every later get()
+            // anywhere would keep adding nodes to it. Compared as a boolean: printing a
+            // leaked tracker on failure would walk its proxies forever.
+            expect(tracking.current === undefined).toBe(true);
+
+            expect(await when(isDone$)).toEqual(true);
+            items = container.querySelectorAll('div');
+            expect(items[0].textContent).toEqual('hi');
+            expect(tracking.current === undefined).toBe(true);
+        });
+    });
     test('use$ with array length', async () => {
         await supressActWarning(async () => {
             const obs$ = observable<{ todos: number[]; total: number }>({

tests/tracking.test.ts

@@ -1,5 +1,6 @@
 import { event } from '../src/event';
 import { observable } from '../src/observable';
+import { trackSelector } from '../src/trackSelector';
 import { beginTracking, endTracking, tracking } from '../src/tracking';
 
 beforeEach(() => {
@@ -140,3 +141,27 @@ describe('Tracking', () => {
         expect(nodes[0].track).toEqual(true);
     });
 });
+
+describe('trackSelector', () => {
+    test('a selector that throws still ends its tracking', () => {
+        const obs = observable({ test: 'hi' });
+        const outer = tracking.current;
+        const noop = () => undefined;
+
+        expect(() =>
+            trackSelector(() => {
+                obs.test.get();
+                throw new Error('thrown in selector');
+            }, noop),
+        ).toThrow('thrown in selector');
+
+        // The thrown selector's tracker is gone and the outer context is current again.
+        // Compared as booleans and sizes: printing a leaked tracker on failure would walk its proxies forever.
+        expect(tracking.current === outer).toBe(true);
+        expect(tracking.current!.nodes?.size ?? 0).toEqual(0);
+
+        // so later gets are recorded where they belong, not in a leaked tracker
+        obs.test.get();
+        expect(tracking.current!.nodes!.size).toEqual(1);
+    });
+});