Skip to content

Commit

Permalink
Add IsSuccessful property to ApiResponse (#1891)
Browse files Browse the repository at this point in the history 
* Use Error property when throwing exception #1376

* Add Unit test

* Update test assertions

* Add IsSuccess property and EnsureSuccessAsync method

Introduce IsSuccess property to ApiResponse<T> and related interfaces to indicate request success. Add EnsureSuccessAsync method to handle errors, including deserialization issues. Refactor EnsureSuccessStatusCodeAsync to use ThrowsApiExceptionAsync. Update Dispose method for exception handling. Add unit tests to verify new behavior.

* Refactor API response success properties and methods

Renamed properties and methods related to the success status of an API response in the `Refit` library:
- `IsSuccess` to `IsSuccessful`
- `EnsureSuccessAsync` to `EnsureSuccessfulAsync`

* Update IsSuccessful usage and documentation

* Fix a typo

* Update tests

Added a new test method `When_SerializationErrorOnSuccessStatusCode_EnsureSuccesStatusCodeAsync_DoNotThrowApiException` to verify that `EnsureSuccessStatusCodeAsync` does not throw an `ApiException` on deserialization error with a success status code. Included assertions to check response status and error presence. Enhanced existing test method `When_SerializationErrorOnSuccessStatusCode_EnsureSuccessfulAsync_ThrowsApiException` with additional assertions.

---------

Co-authored-by: Chris Pulman <chris.pulman@yahoo.com>
  • Loading branch information
@marcominerva @ChrisPulman
marcominerva and ChrisPulman authored Oct 29, 2024
1 parent 2d2169c commit dc74700
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 21 deletions.
10 changes: 7 additions & 3 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,8 +1099,9 @@ var response = await gitHubApi.GetUser("octocat");
//Getting the status code (returns a value from the System.Net.HttpStatusCode enumeration)
var httpStatus = response.StatusCode;

//Determining if a success status code was received
if(response.IsSuccessStatusCode)
//Determining if a success status code was received and there wasn't any other error
//(for example, during content deserialization)
if(response.IsSuccessful)
{
//YAY! Do the thing...
}
Expand Down Expand Up @@ -1362,7 +1363,7 @@ You can then decide what to do like so:

```csharp
var response = await _myRefitClient.GetSomeStuff();
if(response.IsSuccessStatusCode)
if(response.IsSuccessful)
{
//do your thing
}
Expand All @@ -1372,6 +1373,9 @@ else
}
```

> [!NOTE]
> The `IsSuccessful` property checks whether the response status code is in the range 200-299 and there wasn't any other error (for example, during content deserialization). If you just want to check the HTTP response status code, you can use the `IsSuccessStatusCode` property.
#### When returning `Task<T>`
Refit throws any `ApiException` raised by the `ExceptionFactory` when processing the response and any errors that occur when attempting to deserialize the response to `Task<T>`.

Expand Down
20 changes: 18 additions & 2 deletions Refit.Tests/API/ApiApprovalTests.Refit.DotNet6_0.verified.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,25 @@ namespace Refit
public Refit.ApiException? Error { get; }
public System.Net.Http.Headers.HttpResponseHeaders Headers { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
public bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
public bool IsSuccessStatusCode { get; }
public bool IsSuccessful { get; }
public string? ReasonPhrase { get; }
public System.Net.Http.HttpRequestMessage? RequestMessage { get; }
public Refit.RefitSettings Settings { get; }
public System.Net.HttpStatusCode StatusCode { get; }
public System.Version Version { get; }
public void Dispose() { }
public System.Threading.Tasks.Task<Refit.ApiResponse<T>> EnsureSuccessStatusCodeAsync() { }
public System.Threading.Tasks.Task<Refit.ApiResponse<T>> EnsureSuccessfulAsync() { }
}
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Parameter)]
[System.Obsolete("Use Refit.StreamPart, Refit.ByteArrayPart, Refit.FileInfoPart or if necessary, in" +
Expand Down Expand Up @@ -190,6 +196,11 @@ namespace Refit
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
bool IsSuccessful { get; }
string? ReasonPhrase { get; }
System.Net.Http.HttpRequestMessage? RequestMessage { get; }
System.Net.HttpStatusCode StatusCode { get; }
Expand All @@ -201,12 +212,17 @@ namespace Refit
new System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
new Refit.ApiException? Error { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
new bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
new bool IsSuccessStatusCode { get; }
new bool IsSuccessful { get; }
}
public interface IFormUrlEncodedParameterFormatter
{
Expand Down
20 changes: 18 additions & 2 deletions Refit.Tests/API/ApiApprovalTests.Refit.DotNet8_0.verified.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,25 @@ namespace Refit
public Refit.ApiException? Error { get; }
public System.Net.Http.Headers.HttpResponseHeaders Headers { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
public bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
public bool IsSuccessStatusCode { get; }
public bool IsSuccessful { get; }
public string? ReasonPhrase { get; }
public System.Net.Http.HttpRequestMessage? RequestMessage { get; }
public Refit.RefitSettings Settings { get; }
public System.Net.HttpStatusCode StatusCode { get; }
public System.Version Version { get; }
public void Dispose() { }
public System.Threading.Tasks.Task<Refit.ApiResponse<T>> EnsureSuccessStatusCodeAsync() { }
public System.Threading.Tasks.Task<Refit.ApiResponse<T>> EnsureSuccessfulAsync() { }
}
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Parameter)]
[System.Obsolete("Use Refit.StreamPart, Refit.ByteArrayPart, Refit.FileInfoPart or if necessary, in" +
Expand Down Expand Up @@ -190,6 +196,11 @@ namespace Refit
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
bool IsSuccessful { get; }
string? ReasonPhrase { get; }
System.Net.Http.HttpRequestMessage? RequestMessage { get; }
System.Net.HttpStatusCode StatusCode { get; }
Expand All @@ -201,12 +212,17 @@ namespace Refit
new System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
new Refit.ApiException? Error { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
new bool IsSuccessStatusCode { get; }
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Error")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "ContentHeaders")]
new bool IsSuccessStatusCode { get; }
new bool IsSuccessful { get; }
}
public interface IFormUrlEncodedParameterFormatter
{
Expand Down
71 changes: 71 additions & 0 deletions Refit.Tests/ResponseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,77 @@ public async Task When_BadRequest_EnsureSuccessStatusCodeAsync_ThrowsValidationE
Assert.Equal("type", actualException.Content.Type);
}

/// <summary>
/// Test to verify if IsSuccessful returns false if we have a success status code, but there is a deserialization error
/// </summary>
[Fact]
public async Task When_SerializationErrorOnSuccessStatusCode_IsSuccessful_ShouldReturnFalse()
{
var expectedResponse = new HttpResponseMessage(HttpStatusCode.OK)
{
Content = new StringContent("Invalid JSON")
};

mockHandler
.Expect(HttpMethod.Get, "http://api/GetApiResponseTestObject")
.Respond(req => expectedResponse);

using var response = await fixture.GetApiResponseTestObject();

Assert.True(response.IsSuccessStatusCode);
Assert.False(response.IsSuccessful);
Assert.NotNull(response.Error);
}

/// <summary>
/// Test to verify if EnsureSuccessStatusCodeAsync do not throw an ApiException if we have a success status code, but there is a deserialization error
/// </summary>
[Fact]
public async Task When_SerializationErrorOnSuccessStatusCode_EnsureSuccesStatusCodeAsync_DoNotThrowApiException()
{
var expectedResponse = new HttpResponseMessage(HttpStatusCode.OK)
{
Content = new StringContent("Invalid JSON")
};

mockHandler
.Expect(HttpMethod.Get, "http://api/GetApiResponseTestObject")
.Respond(req => expectedResponse);

using var response = await fixture.GetApiResponseTestObject();
await response.EnsureSuccessStatusCodeAsync();

Assert.True(response.IsSuccessStatusCode);
Assert.False(response.IsSuccessful);
Assert.NotNull(response.Error);
}

/// <summary>
/// Test to verify if EnsureSuccessfulAsync throws an ApiException if we have a success status code, but there is a deserialization error
/// </summary>
[Fact]
public async Task When_SerializationErrorOnSuccessStatusCode_EnsureSuccessfulAsync_ThrowsApiException()
{
var expectedResponse = new HttpResponseMessage(HttpStatusCode.OK)
{
Content = new StringContent("Invalid JSON")
};

mockHandler
.Expect(HttpMethod.Get, "http://api/GetApiResponseTestObject")
.Respond(req => expectedResponse);

using var response = await fixture.GetApiResponseTestObject();
var actualException = await Assert.ThrowsAsync<ApiException>(
() => response.EnsureSuccessfulAsync()
);

Assert.True(response.IsSuccessStatusCode);
Assert.False(response.IsSuccessful);
Assert.NotNull(actualException);
Assert.IsType<System.Text.Json.JsonException>(actualException.InnerException);
}

[Fact]
public async Task WhenProblemDetailsResponseContainsExtensions_ShouldHydrateExtensions()
{
Expand Down
73 changes: 59 additions & 14 deletions Refit/ApiResponse.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,21 @@ public sealed class ApiResponse<T>(
/// Indicates whether the request was successful.
/// </summary>
#if NET6_0_OR_GREATER
[MemberNotNullWhen(true, nameof(Content))]
[MemberNotNullWhen(true, nameof(ContentHeaders))]
[MemberNotNullWhen(false, nameof(Error))]
#endif
public bool IsSuccessStatusCode => response.IsSuccessStatusCode;

/// <summary>
/// Indicates whether the request was successful and there wasn't any other error (for example, during content deserialization).
/// </summary>
#if NET6_0_OR_GREATER
[MemberNotNullWhen(true, nameof(Content))]
[MemberNotNullWhen(true, nameof(ContentHeaders))]
[MemberNotNullWhen(false, nameof(Error))]
#endif
public bool IsSuccessful => IsSuccessStatusCode && Error is null;

/// <summary>
/// The reason phrase which typically is sent by the server together with the status code.
/// </summary>
Expand Down Expand Up @@ -119,20 +128,22 @@ public async Task<ApiResponse<T>> EnsureSuccessStatusCodeAsync()
{
if (!IsSuccessStatusCode)
{
var exception =
Error
?? await ApiException
.Create(
response.RequestMessage!,
response.RequestMessage!.Method,
response,
Settings
)
.ConfigureAwait(false);
await ThrowsApiExceptionAsync().ConfigureAwait(false);
}

Dispose();
return this;
}

throw exception;
/// <summary>
/// Ensures the request was successful and without any other error by throwing an exception in case of failure
/// </summary>
/// <returns>The current <see cref="ApiResponse{T}"/></returns>
/// <exception cref="ApiException"></exception>
public async Task<ApiResponse<T>> EnsureSuccessfulAsync()
{
if (!IsSuccessful)
{
await ThrowsApiExceptionAsync().ConfigureAwait(false);
}

return this;
Expand All @@ -147,6 +158,24 @@ void Dispose(bool disposing)

response.Dispose();
}

private async Task<ApiException> ThrowsApiExceptionAsync()
{
var exception =
Error
?? await ApiException
.Create(
response.RequestMessage!,
response.RequestMessage!.Method,
response,
Settings
)
.ConfigureAwait(false);

Dispose();

throw exception;
}
}

/// <inheritdoc/>
Expand All @@ -171,10 +200,17 @@ public interface IApiResponse<out T> : IApiResponse
/// <summary>
/// Indicates whether the request was successful.
/// </summary>
[MemberNotNullWhen(true, nameof(Content))]
[MemberNotNullWhen(true, nameof(ContentHeaders))]
[MemberNotNullWhen(false, nameof(Error))]
new bool IsSuccessStatusCode { get; }

/// <summary>
/// Indicates whether the request was successful and there wasn't any other error (for example, during content deserialization).
/// </summary>
[MemberNotNullWhen(true, nameof(Content))]
[MemberNotNullWhen(true, nameof(ContentHeaders))]
[MemberNotNullWhen(false, nameof(Error))]
new bool IsSuccessful { get; }
#endif

/// <summary>
Expand Down Expand Up @@ -207,6 +243,15 @@ public interface IApiResponse : IDisposable
#endif
bool IsSuccessStatusCode { get; }

/// <summary>
/// Indicates whether the request was successful and there wasn't any other error (for example, during content deserialization).
/// </summary>
#if NET6_0_OR_GREATER
[MemberNotNullWhen(true, nameof(ContentHeaders))]
[MemberNotNullWhen(false, nameof(Error))]
#endif
bool IsSuccessful { get; }

/// <summary>
/// HTTP response status code.
/// </summary>
Expand Down

0 comments on commit dc74700

Please sign in to comment.