rust-libxml icon indicating copy to clipboard operation
rust-libxml copied to clipboard

Error handling

Open mhasel opened this issue 1 year ago • 2 comments

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:

  • making breaking API changes due to the changed return-type of the function
  • to deprecate these functions and introduce new versions that support error handling

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!

mhasel avatar Sep 25 '23 09:09 mhasel

@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.

dginev avatar Sep 25 '23 13:09 dginev

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.

mhasel avatar Oct 02 '23 09:10 mhasel