Fix panic in dagre_rust remove_edge_label_proxies on edge labels#18
Open
cetex wants to merge 1 commit into1jehuang:masterfrom
Open
Fix panic in dagre_rust remove_edge_label_proxies on edge labels#18cetex wants to merge 1 commit into1jehuang:masterfrom
cetex wants to merge 1 commit into1jehuang:masterfrom
Conversation
remove_edge_label_proxies() unconditionally unwraps node.e, which is None for some edge-proxy dummy nodes by the time the function runs. The layout pipeline calls nesting_graph::cleanup() between inject and remove, which can invalidate the edge reference. Replace the unwrap with `if let Some(...)` so missing edge refs are silently skipped instead of panicking. This fixes crashes on any diagram that uses edge labels: flowcharts with -->|label| or diamond nodes, state diagrams with transition labels, and ER diagrams. Adds regression tests for all three crash cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix panic in dagre_rust remove_edge_label_proxies on edge labels
remove_edge_label_proxies() in dagre_rust unconditionally unwraps node.e, which can be None by the time the function runs. The layout pipeline calls nesting_graph::cleanup() between inject_edge_label_proxies and remove_edge_label_proxies, which removes the nesting root node and its incident edges -- this can invalidate the edge reference stored on dummy proxy nodes.
This causes a panic on any diagram that produces edge label proxies during layout:
The fix replaces the unwrap() with if let Some(...), so a missing edge ref is skipped rather than crashing. The node is still removed from the graph regardless.
Includes regression tests for all three crash cases.
(I didn't do any of this, just forked the repo and asked my AI-agent do fix the bug I / we found while I worked on other stuff)