Skip to content

Commit 54a8e62

Browse files
glennawatsonglennawatson
authored
fix: ValidationApiException propagates ContentHeaders and uses configured serializer (#2146)
ValidationApiException dropped the ContentHeaders of the originating ApiException and deserialized ProblemDetails with a hardcoded System.Text.Json instance, ignoring the IHttpContentSerializer configured in RefitSettings. - Copy ContentHeaders from the source ApiException, and widen the ApiException.ContentHeaders setter to protected so derived exceptions can set it. - Deserialize problem details through the configured content serializer so it honors the user's serializer (for example Newtonsoft) and its settings. The default System.Text.Json serializer keeps the same case-insensitive, camelCase behavior as before. - The public synchronous Create overload is unchanged for back compatibility; the automatic problem+json path now uses an async factory. - Add tests for header propagation and for serializer selection. Closes #1945 Closes #1197
1 parent d694baf commit 54a8e62

8 files changed

Lines changed: 125 additions & 20 deletions

‎src/Refit.Tests/API/ApiApprovalTests.Refit.DotNet8_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Refit
1717
protected ApiException(System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1818
protected ApiException(string exceptionMessage, System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1919
public string? Content { get; }
20-
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
20+
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; protected set; }
2121
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2222
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2323
public bool HasContent { get; }

‎src/Refit.Tests/API/ApiApprovalTests.Refit.DotNet9_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Refit
1717
protected ApiException(System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1818
protected ApiException(string exceptionMessage, System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1919
public string? Content { get; }
20-
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
20+
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; protected set; }
2121
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2222
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2323
public bool HasContent { get; }

‎src/Refit.Tests/API/_snapshots/ApiApprovalTests.Refit.DotNet10_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace Refit
1818
protected ApiException(System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1919
protected ApiException(string exceptionMessage, System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
2020
public string? Content { get; }
21-
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
21+
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; protected set; }
2222
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2323
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2424
public bool HasContent { get; }

‎src/Refit.Tests/API/_snapshots/ApiApprovalTests.Refit.DotNet8_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Refit
1717
protected ApiException(System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1818
protected ApiException(string exceptionMessage, System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1919
public string? Content { get; }
20-
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
20+
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; protected set; }
2121
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2222
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2323
public bool HasContent { get; }

‎src/Refit.Tests/API/_snapshots/ApiApprovalTests.Refit.DotNet9_0.verified.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace Refit
1717
protected ApiException(System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1818
protected ApiException(string exceptionMessage, System.Net.Http.HttpRequestMessage message, System.Net.Http.HttpMethod httpMethod, string? content, System.Net.HttpStatusCode statusCode, string? reasonPhrase, System.Net.Http.Headers.HttpResponseHeaders headers, Refit.RefitSettings refitSettings, System.Exception? innerException = null) { }
1919
public string? Content { get; }
20-
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; }
20+
public System.Net.Http.Headers.HttpContentHeaders? ContentHeaders { get; protected set; }
2121
[System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2222
[get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Content")]
2323
public bool HasContent { get; }

‎src/Refit.Tests/ResponseTests.cs‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,76 @@ public async Task ThrowsValidationException()
142142
Assert.Equal("type", actualException.Content.Type);
143143
}
144144

145+
/// <summary>
146+
/// #1945: ValidationApiException should expose the ContentHeaders of the
147+
/// originating ApiException rather than leaving them null.
148+
/// </summary>
149+
[Test]
150+
public async Task ValidationApiExceptionPropagatesContentHeaders()
151+
{
152+
var expectedContent = new ProblemDetails { Detail = "detail", Title = "title" };
153+
var expectedResponse = new HttpResponseMessage(HttpStatusCode.BadRequest)
154+
{
155+
Content = new StringContent(JsonConvert.SerializeObject(expectedContent))
156+
};
157+
expectedResponse.Content.Headers.ContentType = new MediaTypeHeaderValue(
158+
"application/problem+json"
159+
);
160+
mockHandler.Expect(HttpMethod.Get, "http://api/aliasTest").Respond(req => expectedResponse);
161+
162+
var actualException = await Assert.ThrowsAsync<ValidationApiException>(
163+
() => fixture.GetTestObject()
164+
);
165+
166+
Assert.NotNull(actualException.ContentHeaders);
167+
Assert.Equal(
168+
"application/problem+json",
169+
actualException.ContentHeaders.ContentType?.MediaType
170+
);
171+
}
172+
173+
/// <summary>
174+
/// #1197: ValidationApiException must deserialize ProblemDetails with the configured
175+
/// IHttpContentSerializer instead of a hardcoded System.Text.Json instance. A
176+
/// case-sensitive serializer is used here, so the camelCase "detail" key must not map
177+
/// to the PascalCase Detail property the way the old camelCase/case-insensitive
178+
/// hardcoded options would have.
179+
/// </summary>
180+
[Test]
181+
public async Task ValidationApiExceptionUsesConfiguredContentSerializer()
182+
{
183+
var localHandler = new MockHttpMessageHandler();
184+
var settings = new RefitSettings(
185+
new SystemTextJsonContentSerializer(
186+
new JsonSerializerOptions { PropertyNameCaseInsensitive = false }
187+
)
188+
)
189+
{
190+
HttpMessageHandlerFactory = () => localHandler
191+
};
192+
193+
var expectedResponse = new HttpResponseMessage(HttpStatusCode.BadRequest)
194+
{
195+
Content = new StringContent("{\"Title\":\"mapped\",\"detail\":\"unmapped\"}")
196+
};
197+
expectedResponse.Content.Headers.ContentType = new MediaTypeHeaderValue(
198+
"application/problem+json"
199+
);
200+
localHandler
201+
.Expect(HttpMethod.Get, "http://api/aliasTest")
202+
.Respond(req => expectedResponse);
203+
204+
var localFixture = RestService.For<IMyAliasService>("http://api", settings);
205+
206+
var actualException = await Assert.ThrowsAsync<ValidationApiException>(
207+
() => localFixture.GetTestObject()
208+
);
209+
210+
Assert.NotNull(actualException.Content);
211+
Assert.Equal("mapped", actualException.Content.Title);
212+
Assert.Null(actualException.Content.Detail);
213+
}
214+
145215
/// <summary>
146216
/// Test to verify if EnsureSuccessStatusCodeAsync throws a ValidationApiException for a Bad Request in terms of RFC 7807
147217
/// </summary>

‎src/Refit/ApiException.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public class ApiException : ApiExceptionBase
3131
/// <summary>
3232
/// HTTP response content headers as defined in RFC 2616.
3333
/// </summary>
34-
public HttpContentHeaders? ContentHeaders { get; private set; }
34+
public HttpContentHeaders? ContentHeaders { get; protected set; }
3535

3636
/// <summary>
3737
/// HTTP Response content as string.
@@ -214,7 +214,9 @@ public static async Task<ApiException> Create(
214214
?.Equals("application/problem+json") ?? false
215215
)
216216
{
217-
exception = ValidationApiException.Create(exception);
217+
exception = await ValidationApiException
218+
.CreateAsync(exception)
219+
.ConfigureAwait(false);
218220
}
219221

220222
response.Content.Dispose();

‎src/Refit/ValidationApiException.cs‎

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
using System.Text.Json;
1+
using System.Net.Http;
2+
using System.Text;
3+
using System.Text.Json;
24

35
namespace Refit
46
{
@@ -26,14 +28,55 @@ static ValidationApiException()
2628
apiException.ReasonPhrase,
2729
apiException.Headers,
2830
apiException.RefitSettings
29-
) { }
31+
)
32+
{
33+
// Carry over the content headers from the originating ApiException so callers
34+
// can inspect them on the validation exception too (#1945).
35+
ContentHeaders = apiException.ContentHeaders;
36+
}
3037

3138
/// <summary>
3239
/// Creates a new instance of a ValidationException from an existing ApiException.
3340
/// </summary>
3441
/// <param name="exception">An instance of an ApiException to use to build a ValidationException.</param>
3542
/// <returns>ValidationApiException</returns>
3643
public static ValidationApiException Create(ApiException exception)
44+
{
45+
var ex = CreateCore(exception);
46+
ex.Content = JsonSerializer.Deserialize<ProblemDetails>(
47+
exception.Content!,
48+
SerializerOptions
49+
);
50+
return ex;
51+
}
52+
53+
/// <summary>
54+
/// Creates a new instance of a ValidationException from an existing ApiException,
55+
/// deserializing the problem details with the serializer configured in
56+
/// <see cref="RefitSettings"/>.
57+
/// </summary>
58+
/// <param name="exception">An instance of an ApiException to use to build a ValidationException.</param>
59+
/// <returns>ValidationApiException</returns>
60+
internal static async Task<ValidationApiException> CreateAsync(ApiException exception)
61+
{
62+
var ex = CreateCore(exception);
63+
64+
// Deserialize through the configured IHttpContentSerializer rather than a
65+
// hardcoded System.Text.Json instance, so problem details honor the user's
66+
// serializer (e.g. Newtonsoft) and its settings (#1197).
67+
using var content = new StringContent(
68+
exception.Content!,
69+
Encoding.UTF8,
70+
"application/problem+json"
71+
);
72+
ex.Content = await exception
73+
.RefitSettings.ContentSerializer.FromHttpContentAsync<ProblemDetails>(content)
74+
.ConfigureAwait(false);
75+
76+
return ex;
77+
}
78+
79+
static ValidationApiException CreateCore(ApiException exception)
3780
{
3881
if (exception is null)
3982
throw new ArgumentNullException(nameof(exception));
@@ -42,17 +85,7 @@ public static ValidationApiException Create(ApiException exception)
4285
"Content must be an 'application/problem+json' compliant json string."
4386
);
4487

45-
var ex = new ValidationApiException(exception);
46-
47-
if (!string.IsNullOrWhiteSpace(exception.Content))
48-
{
49-
ex.Content = JsonSerializer.Deserialize<ProblemDetails>(
50-
exception.Content!,
51-
SerializerOptions
52-
);
53-
}
54-
55-
return ex;
88+
return new ValidationApiException(exception);
5689
}
5790

5891
/// <summary>

0 commit comments

Comments
 (0)