From b1393998a7f7282cb7f4a4727edc2e91bca61378 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Thu, 3 Sep 2026 18:08:53 +0100 Subject: [PATCH] Report re-entrancy depth and tag torn behaviors A single re-entrant emission can only strand a contiguous run of a behavior's subscribers, so a tile that is both 'speaking' and 'waiting for media' (a middle subscriber stranded) implies a nested re-entry. Report the nesting depth and log each new depth once instead of only the first re-entry, and tag splitBehavior-derived behaviors with their field name so the warning identifies which behavior tore. --- src/state/ObservableScope.test.ts | 26 ++++++++++++++---------- src/state/ObservableScope.ts | 33 ++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/state/ObservableScope.test.ts b/src/state/ObservableScope.test.ts index 353ecaed5..24b5d268d 100644 --- a/src/state/ObservableScope.test.ts +++ b/src/state/ObservableScope.test.ts @@ -239,27 +239,33 @@ describe("Reconcile", () => { }); describe("behavior", () => { - it("warns when a subscriber re-enters the behavior synchronously", () => { + it("warns with the tag and nesting depth, logging each new depth once", () => { const warn = vi.spyOn(logger, "warn").mockImplementation(() => {}); const scope = new ObservableScope(); const source$ = new Subject(); - const behavior$ = scope.behavior(source$, 0); - // A subscriber that reacts to the value 1 by synchronously emitting 2 + const behavior$ = scope.behavior(source$, 0, "tagged"); + // Re-enter once on value 1 (depth 2), then a nested re-entry on value 2 + // (depth 3), which is the shape needed to strand a middle subscriber. behavior$.subscribe((v) => { if (v === 1) source$.next(2); }); + behavior$.subscribe((v) => { + if (v === 2) source$.next(3); + }); const seen: number[] = []; behavior$.subscribe((v) => seen.push(v)); source$.next(1); - expect(warn).toHaveBeenCalledWith( - expect.stringContaining("Behavior re-entered"), - expect.any(String), - ); - // Documents the hazard the warning is about: the later subscriber ends up - // with the stale value 1 even though the behavior's value is 2. - expect(behavior$.value).toBe(2); + const messages = warn.mock.calls.map((c) => c[0] as string); + expect(messages).toEqual([ + expect.stringContaining("Behavior (tagged) re-entered at depth 2"), + expect.stringContaining("Behavior (tagged) re-entered at depth 3"), + ]); + warn.mock.calls.forEach((c) => expect(c[1]).toEqual(expect.any(String))); + // The behavior settles on the newest value while the last subscriber is + // left stranded on the oldest. + expect(behavior$.value).toBe(3); expect(seen.at(-1)).toBe(1); scope.end(); }); diff --git a/src/state/ObservableScope.ts b/src/state/ObservableScope.ts index 0a64e2a42..d0cfb7e93 100644 --- a/src/state/ObservableScope.ts +++ b/src/state/ObservableScope.ts @@ -68,6 +68,9 @@ export class ObservableScope { public behavior( setValue$: Observable, initialValue: T | typeof nothing = nothing, + // An optional label used only in the re-entrancy warning below, so that a + // torn behavior can be identified from a rageshake. + tag?: string, ): Behavior { const subject$ = new BehaviorSubject(initialValue); // Push values from the Observable into the BehaviorSubject. @@ -78,25 +81,29 @@ export class ObservableScope { // If a subscriber synchronously causes this same behavior to emit again, // rxjs delivers the nested value to every subscriber first and then // resumes delivering the outer (older) value to the remaining subscribers, - // leaving them permanently out of sync with the others. Log the first - // occurrence with a stack trace so that the re-entrant path can be found. - let delivering = false; - let reentryReported = false; + // leaving them permanently out of sync with the others. A single such + // re-entry can only strand a contiguous run of subscribers; stranding a + // subscriber in the middle of the list needs a nested (deeper) re-entry, so + // we report the depth and log each new depth (with a stack trace) rather + // than only the first occurrence, to make a nested re-entry visible. + let depth = 0; + let maxReportedDepth = 1; setValue$.pipe(this.bind(), distinctUntilChanged()).subscribe({ next(value) { - if (delivering && !reentryReported) { - reentryReported = true; + if (depth > 0 && depth + 1 > maxReportedDepth) { + maxReportedDepth = depth + 1; logger.warn( - "Behavior re-entered while delivering a value; later subscribers will be left with a stale value", + `Behavior${tag ? ` (${tag})` : ""} re-entered at depth ${ + depth + 1 + } while delivering a value; later subscribers will be left with a stale value`, new Error().stack, ); } - const wasDelivering = delivering; - delivering = true; + depth++; try { subject$.next(value); } finally { - delivering = wasDelivering; + depth--; } }, error(err: unknown) { @@ -193,7 +200,11 @@ export class ObservableScope { return Object.fromEntries( Object.keys(input$.value).map((key) => [ `${key}$`, - this.behavior(input$.pipe(map((input) => input[key as keyof T]))), + this.behavior( + input$.pipe(map((input) => input[key as keyof T])), + nothing, + key, + ), ]), ) as SplitBehavior; }