Skip to content

Conversation

christian-byrne
Copy link
Contributor

@christian-byrne christian-byrne commented Aug 11, 2025

This commit fixes a critical issue where saving a workflow with complex subgraphs could lead to a corrupted file, causing errors upon loading.

The root cause was twofold:

  1. Unsafe Object Cloning: The LiteGraph.cloneObject function used JSON.stringify, which cannot handle circular references that can exist in a nodes properties, especially in complex graphs with promoted widgets. This has been replaced with _.cloneDeep from lodash for robust and safe deep cloning.

  2. Unsafe Node Configuration: The LGraphNode.configure method used a generic loop that would overwrite the inputs and outputs arrays with plain objects from the serialized data. This created a temporary but dangerous state where class instances were replaced by plain objects, leading to TypeError exceptions when methods or private fields were accessed on them. The configure method has been refactored to handle inputs, outputs, and properties explicitly and safely, ensuring they are always proper class instances.

This fix makes the node loading process more robust and predictable, preventing data corruption and ensuring the stability of workflows with subgraphs.

Summary

Changes

  • What:
  • Breaking:
  • Dependencies:

Review Focus

Screenshots (if applicable)

┆Issue is synchronized with this Notion page by Unito

@christian-byrne christian-byrne requested a review from a team as a code owner August 11, 2025 23:12
Copy link

github-actions bot commented Aug 11, 2025

⚠️ Warnings

⚠️ Warning: E2E Test Coverage Missing

If this PR modifies behavior that can be covered by browser-based E2E tests, those tests are required. PRs lacking applicable test coverage may not be reviewed until added. Please add or update browser tests to ensure code quality and prevent regressions.

⚠️ Warning: Visual Documentation Missing

If this PR changes user-facing behavior, visual proof (screen recording or screenshot) is required. PRs without applicable visual documentation may not be reviewed until provided.
You can add it by:

  • GitHub: Drag & drop media directly into the PR description

  • YouTube: Include a link to a short demo

@christian-byrne christian-byrne added area:subgraph needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch 1.25 labels Aug 11, 2025
@christian-byrne
Copy link
Contributor Author

So, it seems the exact same tests fail as the other fix (#4481)

This commit fixes a critical issue where saving a workflow with complex subgraphs could lead to a corrupted file, causing errors upon loading.

The root cause was twofold:

1.  **Unsafe Object Cloning:** The `LiteGraph.cloneObject` function used `JSON.stringify`, which cannot handle circular references that can exist in a nodes properties, especially in complex graphs with promoted widgets. This has been replaced with `_.cloneDeep` from lodash for robust and safe deep cloning.

2.  **Unsafe Node Configuration:** The `LGraphNode.configure` method used a generic loop that would overwrite the `inputs` and `outputs` arrays with plain objects from the serialized data. This created a temporary but dangerous state where class instances were replaced by plain objects, leading to `TypeError` exceptions when methods or private fields were accessed on them. The `configure` method has been refactored to handle `inputs`, `outputs`, and `properties` explicitly and safely, ensuring they are always proper class instances.

This fix makes the node loading process more robust and predictable, preventing data corruption and ensuring the stability of workflows with subgraphs.
@christian-byrne christian-byrne force-pushed the fix/subgraph-serialization-issue branch from 7bf8234 to a2c8f4b Compare August 12, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.25 area:subgraph needs-backport Fix/change that needs to be cherry-picked to the current feature freeze branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant