Comfy-Org / Comfy-Org/ComfyUI_frontend
Failed node-template load returns an empty list that is then written over the user's saved templates
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 704
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 512
Description
## Summary
`ManageTemplates.load()` in `src/extensions/core/nodeTemplates.ts` cannot tell "you have no saved templates" from "I could not read your saved templates", and the empty list it returns on failure is then written back over the user's template file. A single unreadable response destroys the user's entire saved node-template collection.
## Mechanism
`load()` swallows a `res.json()` failure and returns `[]`:
```ts
const res = await api.getUserData(file)
if (res.status === 200) {
try {
templates = await res.json()
} catch (error) {
reportError(error, { /* ... */ }) // reports, but still falls through
}
}
return templates ?? []
```
The constructor assigns that straight to instance state:
```ts
this.load().then((v) => {
this.templates = v
})
```
and every mutation path calls `store()`, which serializes `this.templates` wholesale:
```ts
async store() {
const templates = JSON.stringify(this.templates, undefined, 4)
await api.storeUserData(file, templates, { stringify: false })
}
```
So after a failed load, the next `store()` replaces the user's file with the empty list plus whatever was just added. Every entry point does this: saving a selection as a template (`manage.templates.push(...); manage.store()`), `importAll()`, rename, delete, and drag-reorder.
## Impact
Silent, total loss of saved node templates. The user sees an empty template list — which reads as "I have no templates yet", not as an error — and the very next save makes the loss permanent.
The load failure does not require the stored file to be corrupt. A truncated response body is enough to fail `res.json()` while the file on the server is still intact, at which point we overwrite a good file with an empty one.
## Repro
1. Save two or three node templates.
2. Make `getUserData` for the templates file return a 200 with a body that is not valid JSON (truncate it, or stub the response).
3. Open Manage Node Templates — the list is empty.
4. Save any new template.
5. Reload with the response restored to normal. The original templates are gone; only the one saved in step 4 remains.
## Suggested fix
Make the failure explicit rather than an empty result, so a read failure cannot be laundered into a write. Return a discriminated result from `load()` (loaded vs unreadable), keep the dialog in a distinct "could not load templates" state on failure, and have `store()` refuse to write while the last load is known to have failed. Offer the user an explicit recovery action rather than silently continuing.
Whatever the shape, the invariant worth testing is: a failed load must never lead to a `storeUserData` call that shrinks the persisted set.
## Notes
Found while reviewing #16797, which added the `reportError()` call in the `catch` above. That PR makes the failure visible in telemetry but deliberately does not change the recovery behavior — this issue is the recovery behavior.
Contributor guide
Research direction
Start in src/extensions/core/nodeTemplates.ts and trace the constructor's load assignment, each mutation path, and store(); inspect the getUserData and storeUserData entry points plus the dialog state described in the issue. Done means a failed JSON read leaves an explicit recovery state, store() does not write, and regression coverage verifies that persisted templates are not silently replaced.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100