[BUG] Design of ExceptionFactory doesn't support Task<ApiResponse<T>> programming model

[james-s-tayler]

Describe the bug
I noticed too when working on #1167 that the design of ExceptionFactory is fundamentally broken. It's only designed to work well when interface methods return Task, but it has very unexpected behavior when used with Task<ApiResponse> in that if the exception factory returns null it will attempt to deserialize the response, but that may well fail with a deserialization error. Prior to #1167 it would have actually thrown that exception too.

It seems the main use case of that feature was to override Refit's behavior of throwing exceptions on HTTP 404, which while technically an "error" code has semantics loose enough to only be an error in some cases, and not an error in others. So, being able to chose how that behaves such that it lines up with the semantics of the particular service you're consuming is great, but the docs and developer experience aren't the best for it.

A couple of things wrong with it are as follows:

• Returning null from the ExceptionFactory will attempt to deserialize the response and technically the user has specified that there isn't an error! Except, that just isn't a strong enough guarantee the deserialization can occur successfully. So, the deserialization can fail and populate an unexpected result into ApiException.Error which may have knock-on effects to the calling code which may decide to log any errors present there which could generate a lot of noisy logs.
• The exception factory allows for returning any arbitrary exception class, but ApiResponse.Create() will try to cast it like e as ApiException which surely isn't correct.
• There aren't any unit tests for the feature that make use of Task<ApiResponse>
• The docs for the feature don't show a clear example of how to get HTTP 404 to behave as you need to which is the main reason I can see for the existence of the feature
• The docs show an example which simply never returns an exception ever, which I have a hard time imagining would actually be useful in a real world scenario as there are plenty of transient HTTP errors that happen and you would simple get back null and not really know why.

Steps To Reproduce
Provide an ExceptionFactory that returns null on HTTP 404 and use it in conjunction with a method that returns Task<ApiResponse<T>> and invoke an api returning 404.

Expected behavior
I'm not exactly sure what I expect to happen in this case. I just simply don't expect it to do what it does. I'm leaving this issue as a note to self, so I can have the context to address it later, unless someone else gets to it.

I think probably a good way to deal with the developer experience side of things at least is to provide an additional ExceptionFactory implementation other than just DefaultExceptionFactory that takes in a list of status codes to NOT throw exceptions on, so that consumers of the library can do something easier like:

  var settings = new RefitSettings
        {
            HttpMessageHandlerFactory = () => handler,
            ExceptionFactory = new StatusCodeExceptionSuppressingFactory(HttpStatusCode.NotFound);
        };

You could make the design a little cleaner still by hiding that class behind its own more discoverable static factory method on ExceptionFactory class like

        var settings = new RefitSettings
        {
            HttpMessageHandlerFactory = () => handler,
            ExceptionFactory = ExceptionFactory.SuppressExceptionsOn(HttpStatusCode.NotFound, HttpStatusCode.ImATeapot);
        };

As for how the exception factory behaves with respect to Task<ApiResponse>I'm not sure. It needs some thought.

Environment

• OS: Windows 10
• Device: Desktop
• Version: Refit 6.0.38

Additional context
#1153 is a good example of why/how docs aren't clear enough on this feature and the DX is lacking.

In order to avoid this kind of situation I think the PR template checklist should be updated to include 2 items like:

• does the feature have adequate support for both Task and Task<ApiResponse> (if applicable)
• does the documentation clearly indicate how to use the feature with both the Task and Task<ApiResponse> programming models (if applicable)?

[james-s-tayler]

Perhaps this PR is worth finishing #927 ?