icu4x icon indicating copy to clipboard operation
icu4x copied to clipboard

Create local error types and error hierarchy

Open echeran opened this issue 4 years ago • 3 comments

The idea of creating local error types, defined within a hierarchy, has come up a couple of times recently:

  • When discussing #118, we brought up the idea, summarized as "Have an error type hierarchy: ParseError into LocaleError into IcuError"
  • In review comments for PR #122, the discussion includes a known problem that it could solve, alternatives, and caveats

echeran avatar Jun 25 '20 21:06 echeran

@zbraniecki says that we should put this together when we start putting together the ICU metacrate.

sffc avatar Jul 10 '20 18:07 sffc

To-do: Take a birds-eye view of our errors before 1.0.

sffc avatar Apr 01 '22 17:04 sffc

Discussion:

  • @zbraniecki - Do you mean something like this?
enum ICUError {
    DataProvider(),
    Consistency(),
    InvalidInput(),
    UnexpectedError(),
}

impl Into<ICUError> for DateTimeFormatError {}
...
right?
  • @zbraniecki - I think this is not a 1.0 blocker.
  • @robertbastian - I think it would be nice to have in our API for 1.0.
  • @sffc - Can we leverage the same massive flat error enum like we discussed in #703, and expose it in both Rust and FFI?
  • @robertbastian - There are going to be variants that cannot occur in all setups.
  • @robertbastian - If we did this in 1.0, we could have our APIs return the variants of IcuError, which we should consider.
  • @sffc - I'm not convinced that we should do that, in part because icu::datetime and icu_datetime should be the same.
  • @zbraniecki - For low-level functions like parse, I wouldn't want to return IcuError.
  • @robertbastian - For 1.0 we should at least make sure errors are not overly nested. If there is a data error, there should be only one variant to check.
  • @sffc - I agree. That's something we should track separately.

Consensus: No IcuError in 1.0, but make sure that each individual error type is useful and not overly nested.

sffc avatar May 27 '22 18:05 sffc