Skip to content

Conversation

psychedelicious
Copy link
Collaborator

Summary

Fixes an error when loading workflows. Looks like this:
image

It occurs when a workflow has invalid edges. For example, between nodes that don't exist on the Invoke installation.

Technical explanation

Previously, reactflow appears to have handled an edge case when using its applyChanges utility. If a change was provided without an item, it would skip that change. For example, an "add edge" change that somehow passed null as the edge, instead of a valid edge.

In our workflow loading and validation logic, invalid edges were removed from the array using delete edges[i]. This left "holes" in the array of edges. We then asked reactflow to add these edges to state. When it encountered one of the "holes", it skipped over it.

In a recent release (unsure which, somewhere between the latest v11 and ~v12.4) this seems to have changed. It no longer skips over the "holes" and instead trusts the data. This can cause a couple issues:

  • Error when loading the workflow if reactflow attempt to do anything with the nonexistent edge.
  • If somehow the workflow makes it into state with "holes" in the array of edges, all sorts of other stuff breaks when our code does anything with the nonexistent edge.

Two-part fix:

  • Update the invalid edge handling to not use delete edges[i]. Instead, as we check each edge, we add invalid ones to a set. Then, after all the checks are finished, filter out the invalid edges. The resultant edges array has no holes.
  • Simplify the logic around setting nodes and edges in redux. Previously we were using reactflow's applyChanges utils, but this does literally nothing except take extra CPU cycles. We can simply set the loaded nodes and edges directly in redux. Perhaps we were using applyChanges because it addressed the "holes" issue? Not sure. But we don't need it now.

Related Issues / Discussions

Closes #7868

QA Instructions

Try loading some workflows that reference nodes which are not installed.

Merge Plan

n/a

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

Previously, reactflow appears to have handled an edge case when using its `applyChanges` utility. If a change was provided without an item, it would skip that change. For example, an "add edge" change that somehow passed `null` as the edge, instead of a valid edge.

In our workflow loading and validation logic, invalid edges were removed from the array using `delete edges[i]`. This left "holes" in the array of edges. We then asked `reactflow` to add these edges to state. When it encountered one of the "holes", it skipped over it.

In a recent release (unsure which, somewhere between the latest v11 and ~v12.4) this seems to have changed. It no longer skips over the "holes" and instead trusts the data. This can cause a couple issues:
- Error when loading the workflow if `reactflow` attempt to do anything with the nonexistent edge.
- If somehow the workflow makes it into state with "holes" in the array of edges, all sorts of other stuff breaks when our code does anything with the nonexistent edge.

Two-part fix:
- Update the invalid edge handling to not use `delete edges[i]`. Instead, as we check each edge, we add invalid ones to a set. Then, after all the checks are finished, filter out the invalid edges. The resultant edges array has no holes.
- Simplify the logic around setting nodes and edges in redux. Previously we were using `reactflow`'s `applyChanges` utils, but this does literally nothing except take extra CPU cycles. We can simply set the loaded nodes and edges directly in redux. Perhaps we were using `applyChanges` because it addressed the "holes" issue? Not sure. But we don't need it now.

Closes #7868
@github-actions github-actions bot added frontend-deps PRs that change frontend dependencies frontend PRs that change frontend files labels Apr 2, 2025
@psychedelicious psychedelicious merged commit eed5d02 into main Apr 2, 2025
11 checks passed
@psychedelicious psychedelicious deleted the psyche/fix/ui/load-workflow-error branch April 2, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files frontend-deps PRs that change frontend dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug]: unknown error validating workflow
2 participants