[Bug]: IsSuccessStatusCode is true, but Error is non-null

[hartmark]

Describe the bug 🐞

We noticed that if you get any serialization errors IsSuccessStatusCode is still true for ApiResponse, but Error is not-null because of error in serialization.

We have this reproducing code. Using xunit and RichardSzalay.MockHttp.

Step to reproduce

Create a class using this code and run the unit-test:

using System;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using Refit;
using RichardSzalay.MockHttp;
using Xunit;
using Xunit.Abstractions;

namespace Tests.UnitTests;

public class DeserializeRefitTest
{
    private readonly ITestOutputHelper testOutputHelper;
    private readonly MockHttpMessageHandler mockHttp;
    private readonly IRefitClient refitClient;

    public DeserializeRefitTest(ITestOutputHelper testOutputHelper)
    {
        this.testOutputHelper = testOutputHelper;
        mockHttp = new MockHttpMessageHandler();
        var client = new HttpClient(mockHttp);
        client.BaseAddress = new Uri("https://example.com");

        refitClient = RestService.For<IRefitClient>(client);
    }

    [Fact]
    public async Task DeserializeResponse()
    {
        // If we change to use this JSON it all works
        // const string jsonString =
        //     """
        //     {
        //       "value": "bar"
        //     }
        //     """;

        const string jsonString = "broken json";

        const HttpStatusCode httpStatusCode = HttpStatusCode.OK;
        mockHttp.Fallback.Respond(httpStatusCode, "application/json", jsonString);

        IApiResponse<FooResponse> response =
            await refitClient.GetFoo();

        Assert.NotNull(response);

        if (!response.IsSuccessStatusCode)
        {
            testOutputHelper.WriteLine(response.Error!.Content);
        }

        Assert.Equal(HttpStatusCode.OK, response.StatusCode);

        Assert.Null(response.Error);

        Assert.NotNull(response.Content);
        Assert.Equal("bar", response.Content.Value);
    }
}

public interface IRefitClient
{
    [Get("/api/foo")]
    Task<IApiResponse<FooResponse>> GetFoo();
}

public class FooResponse
{
    public string Value { get; set; }
}

Reproduction repository

No response

Expected behavior

response.Error should not be null when IsSuccessStatusCode is true

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

Refit Version

7.0.0

Additional information ℹ️

No response

[hartmark]

We added this workaround extension method so we catch all errors when checking IApiResponse:

public static class ApiResponseExtensions
{
    public static bool IsReallySuccessful<T>(this IApiResponse<T> response)
    {
        return response.IsSuccessStatusCode && response.Error == null;
    }

    public static bool IsReallySuccessful(this IApiResponse response)
    {
        return response.IsSuccessStatusCode && response.Error == null;
    }
}

[dtcopcp]

found the same too https://github.com/reactiveui/refit/pull/1303