Comfy-Org / Comfy-Org/ComfyUI_frontend
A configure() that throws still dispatches 'configured', so listeners cannot detect failed loads
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 704
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 512
Description
`LGraph.configure()` dispatches `configured` in a `finally` block, so a configure that throws still fires the event. No listener (canvas, Vue layer, workflow store, extensions) can distinguish a failed load from a successful one, and at that point the graph is half-built.
```ts
} finally {
endNamedValuesShadowDiffLoad()
this.events.dispatch('configured')
}
```
Pre-existing, not a branch regression: the block is byte-identical on `main` `296fc5cd07` (`src/lib/litegraph/src/LGraph.ts:2734-2736`) and on `feature/ecs-migration` `f1bfb313d6` (`LGraph.ts:2836-2839`).
Why it matters more on the ECS branch: `configure` now registers entity state into external Pinia stores as it builds (`nodeDataStore` via `registerNodeState`, `linkStore`, `widgetValueStore`, `rerouteStore`, `previewExposureStore`, plus `layoutStore` via `attachNodeLayout`). The half-built state a `configured` listener observes after a throw is materially larger than on main, and `configure` is not transactional (best-effort rollback for the additive path only; recovery via the next good load is covered by `LGraph.interruptedConfigure.test.ts` on #15729, which pins this dispatch-on-throw behavior as characterisation rather than fixing it).
Decision needed before changing anything: `configuring`/`configured` are extension-facing (ADR 0008 lists entity callback changes as affecting 40+ custom node repos). Options:
1. Dispatch a distinct event on failure (e.g. `configure-failed`) and keep `configured` success-only. Breaking for any extension that relies on `configured` always firing.
2. Add a success/error field to the `configured` payload. Additive, non-breaking, but every listener must opt in to checking it.
3. Keep the behavior and document that `configured` means "configure finished", not "configure succeeded".
Routing to @DrJKL for the call since it constrains the extension-facing event contract.
Contributor guide
Research direction
Start in src/lib/litegraph/src/LGraph.ts at LGraph.configure() and run the LGraph.interruptedConfigure.test.ts coverage referenced in #15729. Read ADR 0008 and inspect the extension-facing configuring/configured listeners before proposing the event contract. Done means the failure semantics are decided, documented, and covered by tests without leaving listeners unable to distinguish success from failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100