Skip to content

fix: return Result from draw() and save_gexf() (closes #33) - #34

Open
Dani-giron wants to merge 3 commits into
reverseame:apotheosis2from
Dani-giron:l1-gexf-error-handling
Open

fix: return Result from draw() and save_gexf() (closes #33)#34
Dani-giron wants to merge 3 commits into
reverseame:apotheosis2from
Dani-giron:l1-gexf-error-handling

Conversation

@Dani-giron

@Dani-giron Dani-giron commented Aug 17, 2026

Copy link
Copy Markdown

Summary

draw() exports the HNSW graph to GEXF files and had two silent failure modes. gexf.to_string() can fail and was called through .unwrap(), which panics the whole process on a serialization error instead of surfacing it. Separately, draw() called save_gexf() through let _ = ..., discarding its std::io::Result and swallowing I/O errors from fs::write (disk full, no permission, invalid path) while still returning as if the export had succeeded.

Changes

  • src/controllers/apotheosis.rs: both draw() and the private save_gexf() now return Result<(), Box<dyn std::error::Error>>. The to_string() calls use ? instead of .unwrap(), and the save_gexf() call in draw() uses ? instead of let _ =. GexfError from the gexf crate already implements std::error::Error via thiserror, so it converts into Box<dyn std::error::Error> through ? with no extra work.
  • This is a public signature change. No caller in src/ or src/bin/ calls draw(), so nothing there needed updating.
  • tests/api_contract.rs: draw_produces_one_gexf_file_per_layer, the one test calling draw(), now calls .expect() on the result.

Test plan

Verified locally with the same commands CI runs: cargo clippy --all-targets --all-features -- -D warnings clean, cargo fmt --check clean, and all 44 tests passing.

Also reproduced the silent I/O failure live before fixing it: calling draw() with a path whose parent directory does not exist returned Ok with no panic and no file written, confirming the bug was real and not just a static-analysis finding.

Closes #33.

@Dani-giron

Copy link
Copy Markdown
Author

Heads up: this PR will conflict with #25 for the same reason, see the comment there. Whichever merges second needs to either reapply this fix at the new location (src/export/gexf.rs) or rebase around #25's move, depending on order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant