Found while writing the repros for #15577 and #15581 (PR #15592). Those two are instances of a class, and this is the third and worst instance. Branch feature/ecs-migration, head 9f6ab9adda.
The class
Every updateEndpoints call site on the branch swallows a rejected topology update into console.error and continues with degraded state. There are three, and each degrades differently:
| Site |
On rejection |
Result |
linkDeduplication.ts:158 |
logs, break |
abandons the node's remaining moves (#15581) |
LLink.ts:115 |
logs, falls through |
caller proceeds as if the endpoint moved |
node/slotLinks.ts:192 |
logs, return [] |
node keeps its old input layout; [] is indistinguishable from "nothing to replace" |
None marks has_errors. None uses the repo's unified reportError. Raw console.error is auto-captured by Datadog RUM but untagged, so a load that silently re-wires a graph produces no searchable signal.
Why slotLinks.ts:192 is the worst of the three
const result = store.updateEndpoints(graphScopeOf(node.graph), updates, removals.map(...))
if (!result.ok) {
console.error('Failed to replace node inputs', result.error)
return []
}
node.inputs.splice(0, node.inputs.length, ...finalInputs)
The early return happens before the splice. So on a rejected batch the node keeps its previous input layout while the caller believes replaceNodeInputs ran, and the empty return value is the same value a successful no-op replacement produces. There is no way for a caller to tell the two apart.
Why this may already be a known break
replaceNodeInputs is the indexed-slot-replacement path. comfyui-promptchain (14,814 installs) does exactly node.inputs[i] = {...slot}, and it is the confirmed pack break on this branch. If promptchain's failure is a rejected replaceNodeInputs returning [], then the shim currently being scoped for it is being designed against a symptom rather than the cause.
Worth testing that before the shim is specified.
Asks
- Decide what a rejected topology update should do. One decision for the class, not three patches.
- At minimum, make
replaceNodeInputs distinguishable: return a result type, or throw, rather than an empty array that means two different things.
- Route these through
reportError so they are searchable, and mark the node has_errors where a graph has actually been left mis-wired.
Related
Found while writing the repros for #15577 and #15581 (PR #15592). Those two are instances of a class, and this is the third and worst instance. Branch
feature/ecs-migration, head9f6ab9adda.The class
Every
updateEndpointscall site on the branch swallows a rejected topology update intoconsole.errorand continues with degraded state. There are three, and each degrades differently:linkDeduplication.ts:158breakLLink.ts:115node/slotLinks.ts:192return [][]is indistinguishable from "nothing to replace"None marks
has_errors. None uses the repo's unifiedreportError. Rawconsole.erroris auto-captured by Datadog RUM but untagged, so a load that silently re-wires a graph produces no searchable signal.Why
slotLinks.ts:192is the worst of the threeThe early return happens before the splice. So on a rejected batch the node keeps its previous input layout while the caller believes
replaceNodeInputsran, and the empty return value is the same value a successful no-op replacement produces. There is no way for a caller to tell the two apart.Why this may already be a known break
replaceNodeInputsis the indexed-slot-replacement path. comfyui-promptchain (14,814 installs) does exactlynode.inputs[i] = {...slot}, and it is the confirmed pack break on this branch. If promptchain's failure is a rejectedreplaceNodeInputsreturning[], then the shim currently being scoped for it is being designed against a symptom rather than the cause.Worth testing that before the shim is specified.
Asks
replaceNodeInputsdistinguishable: return a result type, or throw, rather than an empty array that means two different things.reportErrorso they are searchable, and mark the nodehas_errorswhere a graph has actually been left mis-wired.Related