Handle errors from parser and resolver

[zbraniecki]

This issue is a super-set of #62. We should unify how we handle errors from parser and resolver in the context.

We currently return Result from add_messages and Option from format and format_message.

I expect that what we really want are:

• Result from add_messages which would return all parser errors collected during parsing and context errors collected during context building (currently, duplicated entry)
• Option<Result<>> from format and format_message where Option handles whether the value was there, and Result would return the formatted message or the best string we can product and errors recorded during production.

[stasm]

I agree with the general direction. Thanks for writing this down.

In our conversation, you mentioned using Option<Result<String, (String, Vec<FluentError>)>> for the return type of format. I have an open question about using Result in our APIs like that. Its purpose is to provide a branching mechanism. But format doesn't really branch on errors, it always returns something which is good to be used.

Using Result can lead to duplicated code:

if let Some(resolved) = bundle.format("key") {
    resolved
         // Handle value.
        .and_then(|value| ...)
        // Handle value again, and possibly errors.
        .or_else(|(value, errs)| ...)
}

If instead format returned Option<(String, Option<Vec<FluentError>>)>>, this code could be simplified to:

if let Some((value, errs)) = bundle.format("key") {
    // Handle errors.
    errs.and_then(|errs| ...);
    // Handle value.
    value
}

I don't know what the established pattern for such scenarios is in Rust. Is it better to have a dedicated branch for the error scenario with fallback (or_else) or to handle errors separately if there are Some?

[stasm]

I also realized that using Option might prove to be limiting when we want to differentiate between bundle-level errors. E.g. there could be a different error for when a message is missing, a different one for when the id starts with a - (because terms are private), and yet another one when the message exists but doesn't have a value!

Taking this into account, maybe we should return one of the following types?

1. Result<Result<String, (String, Vec<ResolveError>)>, BundleError>, or
2. Result<(String, Vec<ResolveError>), BundleError>, or
3. Result<(String, Option<Vec<ResolveError>>), BundleError>

[zbraniecki]

I asked on rust user group - https://users.rust-lang.org/t/l10n-library-api-question/19967

[zbraniecki]

This has been fixed by 21f3a31