icerpc / slicec

The Slice compiler library
Apache License 2.0
13 stars 5 forks source link

Result<TSuccess, TFailure> generator #648

Closed bernardnormier closed 9 months ago

bernardnormier commented 1 year ago

This is a proposal to add a Result<TSuccess, TFailure> as a kind of generator for enum with associated values.

Prerequisite: enum with associated values as described in https://github.com/icerpc/slicec/issues/30#issuecomment-1454872837

Result<TSuccess, TFailure> is an enum with 2 enumerators (Success & Failure). TSuccess can be a single type or a tuple while TFailure must be a single type.

Result<(x: int32, y: string, tag(1) z: string?), MyError> is a short-hand for:

// a checked enum
enum Result<(x: int32, y: string), MyError> { // pseudo Slice since this is not a valid Slice identifier
    Success(x: int32, y: string, tag(1) z: string?) // the success tuple as-is
    Failure(value: MyError) // MyError can be any type or typealias, but not a tuple; can't be optional, like typealias
}

Result<tag(1) string?, int32> is a short-hand for:

// a checked enum
enum Result<tag(1) string?, int32> { // pseudo Slice since this is not a valid Slice identifier
    Success(tag(1) value: string?)
    Failure(value: int32)
}

Then, you can use Result<TSuccess, TFailure> anywhere you can use a type: as a field, as an operation parameter, and more commonly, as a operation return type:

interface Greeter {
    greet(name: string) -> Result<string, GreeterError>
}

struct GreeterError {
    message: string
    errorCode: int32
}

At the IceRPC level, this return value is a StatusCode=Success, even if it holds a GreeterError.

stream parameter

The Result success tuple is fed into the Success enumerator's associated value and can't include a stream. If you want a stream parameter, it has to be separate:

interface Example {
    op() -> (result: Result<string, string>, stream: Sequence<uint8>)
    op() -> stream Result<string, string>
}

Comparison with previous Result proposal

In #3571, Result is a special notation that maps (with IceRPC) to StatusCode=Success and StatusCode=ApplicationError, and takes advantage of the error message carried by the icerpc StatusCode=ApplicationError response.

One advantage of the #3571 proposal is:

op() -> string
op() -> Result<string, SomeError>

are "on-the-wire" compatible. That's no longer the case with the new proposal, where Result is a short-hand for an enum.

Language mapping

In Rust and Swift, we would naturally map Result to the native Result type.

In C#, we could create our own generic struct or class. We need map enum-with-associated-value in any case, so we can create a similar looking Result generic. An alternative would be to reuse a third-party Result type.

pepone commented 1 year ago

For C# there is a https://dotnet.github.io/dotNext/api/DotNext.Result.html

bernardnormier commented 1 year ago

I don't find the DotNext Result appropriate: you can't have custom errors, they are all Exception.

externl commented 1 year ago

Am I misreading it? I don't see any reference to Exception?

public readonly struct Result<T, TError> : 
    IResultMonad<T, TError, Result<T, TError>>, 
    IResultMonad<T, TError>, 
    IOptionMonad<T, Result<T, TError>>, 
    IOptionMonad<T>, ISupplier<object?>, 
    IFunctional<Func<object?>> 
where TError : struct, Enum
InsertCreativityHere commented 9 months ago

I'm going to start implementing this after I finish wrapping up the changes to enums.

I think the current idea is to make Result a special type like Sequence or Dictionary. We'll add full generic support in the future, but since Result is so widely used, and will have a special mapping, I think it makes sense to make it a special type as well.

If anyone has any changes or ideas to this proposal, comment now or forever hold your peace!

InsertCreativityHere commented 9 months ago

The majority of this proposal was implemented in #687.

InsertCreativityHere commented 9 months ago

This still needs to be implemented on the C# side of things: https://github.com/icerpc/icerpc-csharp/issues/3875