0

Last time I used I created a REST API client in .NET I used exceptions to represent status codes which don't indicate success. (404 was returning null). It's been quite some time since then and my ideas have evolved a bit. Now I am using Blazor for part of my project and I am faced with the problem of API client design. I am specifically interested in the response object design. C# 8.0 provides pattern matching which opens some new options for this design.

My API returns the object serialized as JSON if the call is executed correctly, 500 if internal server error, 404 if the object is not found, 400 if the validation fails, 401 and 403 in the corresponding authentication and authorization cases and the interesting part 409 Conflict when the error is one that should be displayed to the user. An example of such an error might be a registration call where the email is found to exist in the database or trying to post a comment without confirming the email. These are basically errors that would require additional API calls to validate on the client. So my question is how do I design a statically typed API client in C# 8.0 which allows me to express as much intent in the method signature and is easy to call and handle. 404 become nullable types but what about the rest? As I see it I have a few options

  • Do it the old school way and make a bunch of exceptions for the other cases. This means that exceptions will be effectively used for flow control as I will have to use try/catch to branch. Even 500 errors are handled in an app with UI. For example if you get a 500 you display something like "Unexpected error" to the user and need to unlock the form that was locked during the post. This is different from server side programming where you would just bubble the exception to some global handler, craft an error response and terminate the request. You can't just kill the client app if one request fails (or maybe you can?). I can think of some workarounds here like a message bus that handles the exception and displays the message in an universal way but what about unlocking the form and what about these 409 responses which should result in the message from the server being displayed to the user?

  • Craft response objects. The ASP.NET Identity has examples of this like the IdentityResult class which has a bool Succeeded property and an Errors property. You are supposed to if-check the Succeeded property and use the Errors property if it is false. This pattern can be extended to have a Result property which can be examined if the request needs to return some object. What I don't like about this approach is that you can forget the check. You can get this result even if you didn't check the property.

  • My final idea is to create Result types designed to be used with pattern matching. For example like this

public abstract class Result
{

}

public sealed class Success<T> : Result
{
    public T Value { get; }

    public Success(T value)
    {
        Value = value;
    }
}

public sealed class Error : Result
{
    public string Message { get; }

    public Error(string message)
    {
        Message = message;
    }
}

public sealed class ExpectedError : Result
{
    public string Message { get; }

    public ExpectedError(string message)
    {
        Message = message;
    }
}

They would be used like this

Result userResult = await apiClient.RegisterUserAsync(something);

if (userResult is Success<User> success)
{
   User user = success.Value;
   //do something
}
else if (userResult is ExpectedError expectedError)
{
   DisplayError(ExpectedError.Message);
}
else
{
    DisplayError("Something went wrong");
}

This will at least force the client code to check if the request was executed successfully although it will not force handling the errors. I've also seen libraries that do similar thing with methods like OnSuccess, anyone has experience with these?

So which approach would you recommend? How can these approaches be improved? Any other options I have not considered? Maybe exceptions are fine and I can add some form of global handlers on the Blazor side that will keep the form's state?

13
  • If you use exceptions, is their sole purpose to stop work and notify the user of an error, or does useful work (other than user notification) occur after any of the exceptions? – Robert Harvey Apr 29 '20 at 16:07
  • If you were not restrained by the maxim "don't use exceptions for flow control," would exceptions be your preferred mechanism here? – Robert Harvey Apr 29 '20 at 16:09
  • I can't think of a practical case where I do anything but display the error to the user. However I don't want to "stop" work. I want to enable the form so he can try again or click something else. I don't want the app to crash (in the Blazor's case it would display the Reload button) – Stilgar Apr 29 '20 at 16:10
  • 1
    I would put that kind of activity under the heading "handling an error." Which is exactly what exceptions are for. – Robert Harvey Apr 29 '20 at 16:26
  • Maybe but then what? Just wrap every call in try/catch and add the handling in each catch? I'd rather have ifs in this case. It seems to me that the exceptions approach would be good if I find a way to let them bubble up and use a common handler that works in all forms. Maybe a message bus and let the forms subscribe to messages and force them to have Unlock method which enables the controls? – Stilgar Apr 29 '20 at 16:42
1

I happen to be really interested in both Union Types and Result<T> instead of Exceptions in C#. Here's an implementation of Union Types that I've started messing around with.

public class UnionType<T1, T2> 
{
    private readonly SelectedType selectedType;

    public T1 Type1 { get; }
    public UnionType(T1 t1) { Type1 = t1; selectedType = SelectedType.t1; }

    public T2 Type2 { get; }
    public UnionType(T2 t1) { Type2 = t1; selectedType = SelectedType.t2; }

    public void MatchWith(Action<T1> t1Func, Action<T2> t2Func)
    {
        if (selectedType == SelectedType.t1)
            t1Func(Type1);
        else
            t2Func(Type2);
    }

    private enum SelectedType { t1, t2 }
}

(I've got a UnionType<T1, T2, T3> and UnionType<T1, T2, T3, T4> as well)

Here's a sample function that generates a Union type composed of two different classes:

public static UnionType<PersonEntry, AgeCategory> DecideThing()
{
    PersonEntry person = new PersonEntry { Name = "Steve" };
    AgeCategory ageCat = new AgeCategory() { Age = 34 };

    // decide which to return, in this example case, return the PersonEntry.
    return new UnionType<PersonEntry, AgeCategory>(person);
}

And finally here's the code where the method is called and handled, forcing the callsite to specify a flow for all Types in the Union:

public static void Run()
{
    DecideThing().MatchWith(
        (person) => Console.WriteLine(person.Name + "!"),
        (ageCat) => Console.WriteLine(ageCat.Age - 1000)
    );
}
2
  • I considered this approach but in reality the continuation is often async. This means that I should have MatchWithAsync which can be awaited and accept async lambdas. I thought that approach too heavy on syntax in this async context. Otherwise it is quite good. – Stilgar May 8 '20 at 18:31
  • Its also very possible that your sentiment about this just being too clunky in C# is correct. – Graham May 8 '20 at 20:06
0

I decided that for now C# doesn't have the necessary tools to make using result objects better than exceptions.

  • C# doesn't have a way to show that a Result object has specific subtypes (Success and Error) and thus the dev does not know what types they should pattern match on. By contrast F# has this
  • Related to the previous one C# can't show the possible result types in the method signature (TS and F# can do that)
  • C# requires introducing a new variable when pattern matching on type. TypeScript doesn't need this which makes the code shorter
  • C# can't pattern directly extract a property (or at least I didn't find a good way) when pattern matching. For example if I have a Result and want to match Success and extract the value from it I can't do result is Success<User>.Value user I need to first match Success and then in a separate statement access the value which makes the code longer
  • C# is fine with simply dropping return values. If we have a POST method we might not care about the return value. With exceptions if the call fails and we forget to catch the exception the program will crash which is what we want. If we use a result object and we don't care about the object returned we might forget about it and not check for errors. F# is better here as it is an error to ignore the value returned by a function.
  • The switch expression cannot be used for handling errors (because handling them is a statement) and the switch statement syntax is too verbose. We can work around this by wrapping the handling the cases in a method that returns some dummy value but it feels dirty and in some situations introduces unneeded verbosity. This is not an issue with F#
  • Familiarity of C# developers with the exceptions approach

I considered using a result type with methods like MatchSuccess(value => ...), MatchError(error => ...). They help the user see all possible cases to match but doesn't solve the issue with methods where you don't care about the value and the code becomes very verbose if the callback code is async which is often the case.

None of these issues is a deal breaker on its own but I felt the combination of them all makes the exception approach the lesser evil. This also convinced me that C# does need some form of sum types, discriminated unions or similar mechanism

0

This means that exceptions will be effectively used for flow control as I will have to use try/catch to branch.

I want to disambiguate an important point here, are you're likely going to get (technically correct) advice that might answer a different question than what you're asking.

Exceptions are intended to be used for flow control (i.e. throwing and catching). There's no other way to correctly use exceptions to do what they were designed to do.

The bad practice would using exceptions to control a non-exceptional flow of your application, i.e. misusing the flow control to direct your application for anything other than a situation that cannot be resolved.

You can't just kill the client app if one request fails (or maybe you can?). I can think of some workarounds here like a message bus that handles the exception and displays the message in an universal way but what about unlocking the form and what about these 409 responses [..]?

You're getting confused between exceptions and bad results. When an exception is caught and you then return a bad result in your request (i.e. a non-200 status code), it ceases to be an exception, and instead is now functionally speaking a message to the consumer (i.e. the client app).

What you do with that result is up to your client application. Maybe you show it to the user. Maybe you silently log it. Maybe you swallow it intentionally (if you have a good reason to). Maybe you terminate your client application.

What your client app should do isn't answerable in this question, since we don't know the specs or user expectations.

409 Conflict when the error is one that should be displayed to the user

That is not how a 409 http status is defined. Are you maybe using 409s in a homebrew approach?

All responses from the server (status code and response object), no matter the status code, are intended to be viewed by the consumer, since the consumer is the client app (not the person using the app!).

The entire purpose of the returned information (status code and response object) is so the server can relay information to its consumer. If the consumer wouldn't be allowed to see it, that'd defeat the purpose entirely.

Whether your client app then relays that information to its consumer (i.e. the end user), is contextual, as I already mentioned in the previous section. Whether your client app reuses the message it receives from the server, or it instead created its own message, is completely up to the client app to decide.

It seems like you are trying to use the 409 status to let the server decide whether the client app should or should not expose a certain server message to the user. That's not how you should be approaching this.

The server returns a result to a consumer (whether that's a client app, Postman, or an actual person firing off web requests), and it shouldn't care about what the consumer then does with the data it receives.

My final idea is to create Result types designed to be used with pattern matching. For example like this

This is one of those "just because you can do it doesn't mean you should do it" situations.

  • Empty base classes are a misuse of OOP principles. There is nothing to gain by having this base type since the base type does not define any reusable implementation (nor contract).

  • The functional difference between Error and ExpectedError can and should be expressed using a boolean property, not a new derived type.

  • userResult is Success<User> success is polymorphism abuse. This too can be implemented using a simple scalar property, i.e. userResult.IsSuccess

The result object can be condensed into a single type and used accordingly. There is no reason to separate these types. However, I do understand where your intention comes from. This can be achieved using a different approach, e.g. static methods that define success/error states:

public class Result<T>
{
    public bool IsSuccess { get; private set; }
    public T Value { get; private set }
    public string Message { get; private set; }
    public bool IsExpectedError { get; private set; }


    private Result() { }

    public static Result<T> Success(T value)
    {
        return new Result<T>()
        {
            IsSuccess = true,
            Value = value
        };
    }

    public static Result<T> Error(string message, bool isExpectedError = false)
    {
        return new Result<T>()
        {
            IsSuccess = false,
            Message = message,
            IsExpectedError = isExpectedError 
        };
    }
}

The private constructor ensures that you can only create Result<> objects using the defined static methods, and the static methods themselves specifically force the developer to fill in particular values for a particular result state (success/error). Usage examples:

var goodResult = Result<User>.Success(myUser);
var errorResult = Result<User>.Error("this is a message");

As a sidenote, I don't quite agree with the name/usage of an "expected" error, but I already addressed this in and earlier section. I retained its usage here to show how your code can be changed to avoid OOP abuse.

  • Craft response objects.
  • My final idea is to create Result types designed to be used with pattern matching

The response object (considering the improvements I just suggested) are really good result objects for the web response, i.e. the communication between backend and frontend.

However, these are not appropriate to replace exceptions in the internal logic of the backend itself. The .NET ecosystem has been built on exceptions, and you'll always have to deal with the possibility of exceptions being thrown.

Maybe you don't throw exceptions yourself, but the libraries and framework tools you rely on will be throwing exceptions and you should be catching them (at the very least on the top level just before it would otherwise be returned to the consumer of your backend).

If you're already going to be stuck handling exceptions anyway, there's no point to then also rolling your own result object for a subset of cases. If the error states of your result object are functionally equivalent to what otherwise would've been a thrown exception, you can just throw an exception.

10
  • 1. The 409 code is not supposed to be used like this but I found that it works quite well in practice. We had an app where the code was properly used (to identify invalid relations or situations where the user wanted to delete something that has attached items which should be deleted first). Then we found out it works well for all cases where the user should be presented with the message for the server. I am quite happy with this design and am not looking to change that. – Stilgar May 8 '20 at 19:06
  • 2) The result object is arguably a misuse of OOP but it is not misuse of subtyping. I am trying to emulate the functional approach here which I quite like but C# lacks the tools to represent it and inheritance is a kind of work-around – Stilgar May 8 '20 at 19:07
  • 3) I don't like the approach with properties because cases can be ignored, especially in situations where you don't want to retrieve the result. When using ASP.NET Identity I have on more than one occasion shipped code in production where I do something like create user where I don't care about the result and I forget to check if the result is successful. It is quite weird I'd rather get an exception and see a crash. – Stilgar May 8 '20 at 19:09
  • @Stilgar: 1. I'm not saying you have to conform to convention but the odds of finding an out-of-the-box answer for your homebrew solution are going to be low. 2. It is a misuse of inheritance since your derived classes aren't inheriting anything. You're signing yourself up for LSP violations. 3. So how would you handle not passing a value in your suggested approach, since your example similarly requires you to fill in the T Value? – Flater May 8 '20 at 22:35
  • 2) "You're signing yourself up for LSP violations." - pretty sure I am not. LSP is about subtyping not about inheritance. 3) My suggested pattern matching version doesn't handle not passing a value either. However the exception approach does - you can't ignore exception. F# also does handle this case as you cannot ignore a return value – Stilgar May 9 '20 at 9:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.