Avoid dependencies on (external) error types

[matzemathics]

Anyhow provides an idiomatic way for passing around errors without the need to keep every single one of them in a combined error enum, which should reduce inter-dependencies without limiting our ability to inspect errors later on.

[mmarx]

We're building a library, though, not an application, so thiserror is the appropriate choice:

Use thiserror if you care about designing your own dedicated error type(s) so that the caller receives exactly the information that you choose in the event of failure. This most often applies to library-like code. Use Anyhow if you don't care what error type your functions return, you just want it to be easy. This is common in application-like code.

[mkroetzsch]

We need to support generic error types anyhow, since we provide an extension point that can return errors (that we do not know about yet). I don't think this is a library vs. application issue. Specific error messages are always nice, even in an application, but they require full knowledge of the possible errors upfront.

[mkroetzsch]

It would be possible to mix the two approaches by having enum errors to distinguish "rough" types of potential issues, which internally wrap Boxed dyn errors as their causes. But it should not be so specific as to have different enums for every format that RIO supports. RIO should not be a dependency of nemo-physical.

[matzemathics]

On that note, I am not sure it is ideal to have our "public api" depend on types of external crates, if we do consider our code "library code".

[matzemathics]

Maybe it suffices to make the ReadingError dynamically typed?

[matzemathics]

We're building a library, though, not an application, so thiserror is the appropriate choice

I am also not sure what potential users would want to deal with the huge amount of cases we currently provide. I believe to make the API actually useful, we'd need to split the error type based on what part of the API produces what error, and then return specific Error types instead of one catch-all.

[mkroetzsch]

I also think that a catch-all enum is not the way to go. I would think that error-generalisation (more possible cases) should only occur along the error propagation path (as errors from several sources come together in one function that needs to pass them on). What we currently have is a design where error types are passed "sideways" and "downwards" so that they occur on paths that never deal with that type of error.

I know of several possible designs for error handling in rust:

1. Error enums with each possible case included (our current design, easy with thiserror, error types known statically and easy to catch/match, not extensible)
2. Box<dyn Error> and anyhow::Error (fully dynamic errors, harder to catch, fully extensible)
3. A combination of 1 and 2: Some enum cases wrap Box causes (extensible, still allows for some matching, some design considerations as for 1 regarding the enums provided in each reporting path)
4. Generic caller-defined errors: fn library_function<E: From<Self::Error>>(...) -> Result<(), E>; (outside caller must provide struct to handle errors from the library, library can restrict Self::Error to cases that matter locally)

Pattern 4 is quite elegant, but only really works if the caller provides the most generic error (for the whole propagation path). This is easier when we have a single API function that has an "internal" Self::Error that one can extend, but hard if an extension point works as a call-back deep down in a propagation stack (as our file loading does).

Dynamic errors are much easier in terms of coding, but lack the static information and might be difficult to match later on, which can possibly be alleviated by utility libraries like anyhow or combined approaches like case 3 above.

Are there any other options, or is this it?

[mmarx]

Ah, now I understand what this is really about. Still, I think that the idea that a caller must handle all variants of the error enum is fundamentally a misconception: a caller should handle exactly the errors they can actually recover from.

Maybe it suffices to make the ReadingError dynamically typed?

We already have ExternalReadingError and a Box<dyn ExternalReadingError> variant. If this is mostly about eliminating rio variants from ReadingError, those should then simply use ExternalReadingError as well.