search

KWARC / rust-libxml

Rust wrapper for libxml2
https://crates.io/crates/libxml
MIT License
76 stars 38 forks source link

Error handling #120

Open mhasel opened 1 year ago

mhasel commented 1 year ago

Hi!

First of all, thanks for your work on this project. I've recently started using this crate for XML validation and when running tests with syntactically incorrect XML files (which might be the result from bad merges) I noticed that internal errors from libxml2 result in a panic.

I'm happy to contribute on this matter myself, I was just wondering which route you might want to take here. I've had a look at all the TODOs regarding error handling - turning the null-pointer checks in https://github.com/KWARC/rust-libxml/blob/da4369a035557667650a221bc89d5e59d0efb247/src/schemas/parser.rs#L24C1-L26C6 from panics to results would necessitate either:

All the other occurrences of panics due to missing error-handling already return a Result<Self, Vec<StructuredError>>, so implementing these should only be a matter of creating new StructuredErrors, i.e. StructuredError::null_ptr() and StructuredError::internal().

Any and all feedback is appreciated!

dginev commented 1 year ago

@mhasel thank you for looking into this.

As you may suspect, the panics in question were added there to forcefully remind us we should be fleshing out an error interface at some point. Maybe your newly opened issue is that occasion, and especially if you are willing to do a contribution yourself - it would be welcome!

Your plan for creating more StructuredError variants sounds workable, though whether you need the wrapping Vec or not is up to you for the final Result<Self, StructuredError> type here.

Note to self: This will likely move us to a version 0.4.0 to signal we've broken an interface, and maybe that is a good occasion to double-check if any of the other issues can benefit of getting batched in.

mhasel commented 1 year ago

Hey, sorry for the radio silence. Been under the weather these past couple of days. I should have a PR ready soon, just need to sort out tests and the error-domain codes.