fix(client): deferRegistration rolls its subscription back when construction throws
The immediate registration attempt runs after the subscription is installed; a synchronous failure (declared slot already occupied) escaped the constructor without returning a handle, leaving a subscription owned by nothing — later ledger flushes would fire it against a failed fiber. Construction failure now unsubscribes before rethrowing, with a dedicated deferred.spec covering both arms (ds-review-bot on the flow registrations; the effect-level rollback in the flow packages keeps covering earlier successfully-constructed deferrals).
This commit is contained in:
@@ -37,6 +37,8 @@ export interface DeferredRegistration {
|
|||||||
* @param component - the component whose ledger presence marks "registered".
|
* @param component - the component whose ledger presence marks "registered".
|
||||||
* @param register - performs the actual registration; returns its disposer.
|
* @param register - performs the actual registration; returns its disposer.
|
||||||
* @returns the deferral handle (dispose in the owning effect's disposer).
|
* @returns the deferral handle (dispose in the owning effect's disposer).
|
||||||
|
* @throws the immediate registration's failure, after removing the
|
||||||
|
* just-installed subscription — a throwing construction leaves nothing live.
|
||||||
*/
|
*/
|
||||||
export function deferRegistration(
|
export function deferRegistration(
|
||||||
registry: DeferralRegistry,
|
registry: DeferralRegistry,
|
||||||
@@ -51,7 +53,15 @@ export function deferRegistration(
|
|||||||
dispose = register()
|
dispose = register()
|
||||||
}
|
}
|
||||||
const unsubscribe = registry.subscribe(name, () => { tryRegister() })
|
const unsubscribe = registry.subscribe(name, () => { tryRegister() })
|
||||||
tryRegister()
|
try {
|
||||||
|
tryRegister()
|
||||||
|
} catch (error) {
|
||||||
|
// A synchronous registration failure (the declared slot is already
|
||||||
|
// occupied) must not leave the just-installed subscription behind: the
|
||||||
|
// caller receives no handle to dispose it through.
|
||||||
|
unsubscribe()
|
||||||
|
throw error
|
||||||
|
}
|
||||||
return {
|
return {
|
||||||
refresh() {
|
refresh() {
|
||||||
dispose?.()
|
dispose?.()
|
||||||
|
|||||||
44
packages/client/ui-slots/tests/deferred.spec.ts
Normal file
44
packages/client/ui-slots/tests/deferred.spec.ts
Normal file
@@ -0,0 +1,44 @@
|
|||||||
|
// deferRegistration lifecycle: declaration-aware registration, HMR
|
||||||
|
// re-registration, and — the failure contract — no subscription survives a
|
||||||
|
// construction that throws synchronously (an already-occupied single slot).
|
||||||
|
import { describe, expect, it, vi } from 'vitest'
|
||||||
|
import { deferRegistration, SlotCore } from '@deepseek-ai/dsh-client-ui-slots'
|
||||||
|
|
||||||
|
// Shares the merges declared by core.spec.ts (same program); reuse its keys.
|
||||||
|
const HOLE = 'test.single' as const
|
||||||
|
|
||||||
|
function declared(): SlotCore {
|
||||||
|
const core = new SlotCore()
|
||||||
|
core.register({ name: 'root', children: { [HOLE]: { kind: 'single', scope: 'root' } } } as never, (() => null) as never)
|
||||||
|
return core
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('deferRegistration', () => {
|
||||||
|
it('registers immediately under an existing declaration and disposes cleanly', () => {
|
||||||
|
const core = declared()
|
||||||
|
const component = (): null => null
|
||||||
|
const handle = deferRegistration(core, HOLE, component, () =>
|
||||||
|
core.register({ name: HOLE } as never, component as never))
|
||||||
|
expect(core.entries(HOLE)).toHaveLength(1)
|
||||||
|
handle.dispose()
|
||||||
|
expect(core.entries(HOLE)).toHaveLength(0)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('drops its subscription when the immediate registration throws', async () => {
|
||||||
|
const core = declared()
|
||||||
|
const foreign = (): null => null
|
||||||
|
const disposeForeign = core.register({ name: HOLE } as never, foreign as never)
|
||||||
|
const component = (): null => null
|
||||||
|
const register = vi.fn(() => core.register({ name: HOLE } as never, component as never))
|
||||||
|
// The single hole is occupied: the immediate attempt throws out of the
|
||||||
|
// constructor, and the caller never receives a handle to dispose.
|
||||||
|
expect(() => deferRegistration(core, HOLE, component, register)).toThrow(/already has a registration/)
|
||||||
|
expect(register).toHaveBeenCalledOnce()
|
||||||
|
// The subscription rolled back with it: freeing the hole flushes a
|
||||||
|
// notification that must not resurrect the failed registration.
|
||||||
|
disposeForeign()
|
||||||
|
await Promise.resolve()
|
||||||
|
expect(register).toHaveBeenCalledOnce()
|
||||||
|
expect(core.entries(HOLE)).toHaveLength(0)
|
||||||
|
})
|
||||||
|
})
|
||||||
Reference in New Issue
Block a user