Introduce Failure type for scheduler replies

[lukeyeh]

This PR addresses the issue of handling both Trap and Error within the same try-block in the scheduler reply, resolving #463. Currently, the scheduler reply has the signature Result<Result<T, Error>, Trap>, which does not work well with try-blocks that only support one level of Result.

To resolve this, we introduce a new Failure enum that can represent either a Trap or an Error. The Failure type is defined in failure.rs, along with the Trap function. We also implement From traits for both Trap and Error to allow seamless conversion into the Failure type.

By updating the scheduler reply signature to Result<T, Failure>, we can now use the ? operator for both Trap and Error within the same try-block, making the code more concise and easier to read.

[ia0]

Sorry, I probably used a misleading title in #463. This PR is not fixing the core idea of #463 but just introducing an unused private type, which is not useful on its own. Instead, for this PR to fix #463, it should:

• Introduce the Failure type and its 2 implementations at top-level (the enabler).
• Change the SchedulerCall::reply() function to take a Result<_, Failure> instead (the actual change).
• Fix any compilation error when calling reply() (the consequences).
• Improve the try-blocks that currently have to juggle to return an Error (the benefit). This juggling can probably be identified by explicit use of Ok and Err within or right after the try-block .

[ia0]

I don't think we need a new file just for those 3 items. The Failure type and its implementations can simply be in the lib.rs next to the Trap type. In particular we don't need tests. Those are just normal usage of the type within the rest of the crate, so the tests are redundant.

[lukeyeh]

moved Trap back into lib.rs and added the Failure enum there too.

[ia0]

This is not a minor change. This is purely an implementation detail, so a patch change. And the change should be something along the lines of "Simplify try-blocks for API call replies with a common Failure type".

[lukeyeh]

Moved to patch, but kept the description the same as I plan to do the other parts in following PRs. Please let me know if you would like me to do all of the steps in a single PR.

[ia0]

Yes, I think it should all be in a single PR, since the changes independently don't make sense.

[lukeyeh]

Hey @ia0, I changed Scheduler::reply() to take the new Result<_, Failure> type as an argument and fixed all the compiler errors.

[lukeyeh]

Sorry, I probably used a misleading title in #463. This PR is not fixing the core idea of #463 but just introducing an unused private type, which is not useful on its own. Instead, for this PR to fix #463, it should:

* Introduce the `Failure` type and its 2 implementations at top-level (the enabler).

* Change the `SchedulerCall::reply()` function to take a `Result<_, Failure>` instead (the actual change).

* Fix any compilation error when calling `reply()` (the consequences).

* Improve the try-blocks that currently have to juggle to return an `Error` (the benefit). This juggling can probably be identified by explicit use of `Ok` and `Err` within or right after the try-block .

I mainly wanted to add tests because I wanted to have this PR just add the failure type and following PRs to change ScheduelrCall::reply's signature and improve the try blocks. I didn't want to have to add a #[allow(dead_code)] marker, so added some tests. I moved the Trap and Failure back to lib.rs and left a TODO instead.

[ia0]

Thanks! This looks good. Most of the comments are the same and I did not mention it in every file, so you'll have to search for them.

[lukeyeh]

Addressed comments and applied changes to equivalent areas that you didn't leave comments PTAL

[ia0]

Thanks! There's one more pattern I missed: foo().map(|x| x.bar())? can be foo()?.bar().

[lukeyeh]

went through the new pass of comments with the new pattern and other various nits

[ia0]

Thanks! There's 2 occurrences of the map()? that seems to have been missed, otherwise everything else is good to merge now.

[ia0]

Thanks!