Skip to content

Commit 5b4de4d

Browse files
thomhurstclaude
andauthored
fix: Improve error handling in HTTP and analyzer code (#1574, #1550) (#1692)
Issue #1574 - SuccessHttpHandler unconditionally throws: - Add HttpResponseException class with detailed error information - Include status code, reason phrase, response content, and request URI - Create EnsureSuccessStatusCodeWithContentAsync extension method - Replace EnsureSuccessStatusCode() calls in Http.cs and Downloader.cs - Response content is truncated at 2000 chars to avoid excessive messages Issue #1550 - Null dereference in AsyncModuleCodeFixProvider.AddAsync: - Replace First()! with FirstOrDefault() for expression syntax lookup - Add null checks for return statements without expressions - Add null checks for ArgumentList and inner expression extraction - Gracefully skip malformed return statements instead of throwing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 875cee5 commit 5b4de4d

5 files changed

Lines changed: 153 additions & 5 deletions

File tree

src/ModularPipelines.Analyzers/ModularPipelines.Analyzers.CodeFixes/AsyncModuleCodeFixProvider.cs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,26 @@ private async Task<Document> AddAsync(CodeFixContext context, MethodDeclarationS
6666

6767
foreach (var returnStatement in GetReturnStatements(methodDeclarationSyntax))
6868
{
69-
var expressionSyntax = returnStatement.ChildNodes().OfType<ExpressionSyntax>().First()!;
69+
var expressionSyntax = returnStatement.ChildNodes().OfType<ExpressionSyntax>().FirstOrDefault();
70+
71+
// Skip return statements without expressions (e.g., bare "return;")
72+
if (expressionSyntax is null)
73+
{
74+
continue;
75+
}
7076

7177
if (IsTaskFromResult(expressionSyntax, semanticModel)
7278
|| IsAsTaskExtension(expressionSyntax, semanticModel))
7379
{
74-
var firstInnerExpression = expressionSyntax.ChildNodes().OfType<ArgumentListSyntax>().First().Arguments.First().Expression;
80+
var argumentList = expressionSyntax.ChildNodes().OfType<ArgumentListSyntax>().FirstOrDefault();
81+
var firstArgument = argumentList?.Arguments.FirstOrDefault();
82+
var firstInnerExpression = firstArgument?.Expression;
83+
84+
// Skip if we can't find the inner expression to unwrap
85+
if (firstInnerExpression is null)
86+
{
87+
continue;
88+
}
7589

7690
var newReturnStatement = returnStatement.ReplaceNode(expressionSyntax, firstInnerExpression);
7791

src/ModularPipelines/Context/Downloader.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public async Task<HttpResponseMessage> DownloadResponseAsync(DownloadOptions opt
6464
HttpClient = options.HttpClient,
6565
}, cancellationToken).ConfigureAwait(false);
6666

67-
return response.EnsureSuccessStatusCode();
67+
return await response.EnsureSuccessStatusCodeWithContentAsync(cancellationToken).ConfigureAwait(false);
6868
}
6969

7070
/// <summary>
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using System.Net;
2+
3+
namespace ModularPipelines.Exceptions;
4+
5+
/// <summary>
6+
/// Exception thrown when an HTTP request returns a non-success status code.
7+
/// Provides detailed information about the failed response including status code, reason phrase, and response content.
8+
/// </summary>
9+
public class HttpResponseException : PipelineException
10+
{
11+
/// <summary>
12+
/// Gets the HTTP status code of the failed response.
13+
/// </summary>
14+
public HttpStatusCode StatusCode { get; }
15+
16+
/// <summary>
17+
/// Gets the reason phrase from the failed response.
18+
/// </summary>
19+
public string? ReasonPhrase { get; }
20+
21+
/// <summary>
22+
/// Gets the content of the failed response, if available.
23+
/// </summary>
24+
public string? ResponseContent { get; }
25+
26+
/// <summary>
27+
/// Gets the request URI that produced the failed response.
28+
/// </summary>
29+
public Uri? RequestUri { get; }
30+
31+
/// <summary>
32+
/// Initializes a new instance of the <see cref="HttpResponseException"/> class.
33+
/// </summary>
34+
/// <param name="statusCode">The HTTP status code.</param>
35+
/// <param name="reasonPhrase">The reason phrase from the response.</param>
36+
/// <param name="responseContent">The content of the response.</param>
37+
/// <param name="requestUri">The request URI.</param>
38+
public HttpResponseException(HttpStatusCode statusCode, string? reasonPhrase, string? responseContent, Uri? requestUri)
39+
: base(FormatMessage(statusCode, reasonPhrase, responseContent, requestUri))
40+
{
41+
StatusCode = statusCode;
42+
ReasonPhrase = reasonPhrase;
43+
ResponseContent = responseContent;
44+
RequestUri = requestUri;
45+
}
46+
47+
/// <summary>
48+
/// Initializes a new instance of the <see cref="HttpResponseException"/> class with an inner exception.
49+
/// </summary>
50+
/// <param name="statusCode">The HTTP status code.</param>
51+
/// <param name="reasonPhrase">The reason phrase from the response.</param>
52+
/// <param name="responseContent">The content of the response.</param>
53+
/// <param name="requestUri">The request URI.</param>
54+
/// <param name="innerException">The inner exception.</param>
55+
public HttpResponseException(HttpStatusCode statusCode, string? reasonPhrase, string? responseContent, Uri? requestUri, Exception? innerException)
56+
: base(FormatMessage(statusCode, reasonPhrase, responseContent, requestUri), innerException)
57+
{
58+
StatusCode = statusCode;
59+
ReasonPhrase = reasonPhrase;
60+
ResponseContent = responseContent;
61+
RequestUri = requestUri;
62+
}
63+
64+
private static string FormatMessage(HttpStatusCode statusCode, string? reasonPhrase, string? responseContent, Uri? requestUri)
65+
{
66+
var message = $"HTTP request failed with status code {(int)statusCode} ({statusCode})";
67+
68+
if (!string.IsNullOrWhiteSpace(reasonPhrase))
69+
{
70+
message += $": {reasonPhrase}";
71+
}
72+
73+
if (requestUri is not null)
74+
{
75+
message += $"\nRequest URI: {requestUri}";
76+
}
77+
78+
if (!string.IsNullOrWhiteSpace(responseContent))
79+
{
80+
// Truncate very long responses to avoid excessive exception messages
81+
const int maxContentLength = 2000;
82+
var truncatedContent = responseContent.Length > maxContentLength
83+
? responseContent[..maxContentLength] + "... (truncated)"
84+
: responseContent;
85+
message += $"\nResponse content: {truncatedContent}";
86+
}
87+
88+
return message;
89+
}
90+
}

src/ModularPipelines/Http/Http.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpOptions httpOptions, Cancel
4545
return response;
4646
}
4747

48-
return response.EnsureSuccessStatusCode();
48+
return await response.EnsureSuccessStatusCodeWithContentAsync(cancellationToken).ConfigureAwait(false);
4949
}
5050

5151
public HttpClient GetLoggingHttpClient(HttpLoggingType loggingType)
@@ -141,7 +141,7 @@ private async Task<HttpResponseMessage> SendAndWrapLogging(HttpOptions httpOptio
141141
return response;
142142
}
143143

144-
return response.EnsureSuccessStatusCode();
144+
return await response.EnsureSuccessStatusCodeWithContentAsync(cancellationToken).ConfigureAwait(false);
145145
}
146146

147147
private HttpLoggingOptions GetEffectiveLoggingOptions(HttpOptions httpOptions)
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
using ModularPipelines.Exceptions;
2+
3+
namespace ModularPipelines.Http;
4+
5+
/// <summary>
6+
/// Extension methods for <see cref="HttpResponseMessage"/> handling.
7+
/// </summary>
8+
internal static class HttpResponseExtensions
9+
{
10+
/// <summary>
11+
/// Ensures the response has a success status code, throwing a detailed exception if not.
12+
/// Unlike <see cref="HttpResponseMessage.EnsureSuccessStatusCode"/>, this method includes
13+
/// the response content in the exception for easier debugging.
14+
/// </summary>
15+
/// <param name="response">The HTTP response to check.</param>
16+
/// <param name="cancellationToken">Cancellation token for reading response content.</param>
17+
/// <returns>The original response if successful.</returns>
18+
/// <exception cref="HttpResponseException">Thrown when the response status code indicates failure.</exception>
19+
public static async Task<HttpResponseMessage> EnsureSuccessStatusCodeWithContentAsync(
20+
this HttpResponseMessage response,
21+
CancellationToken cancellationToken = default)
22+
{
23+
if (response.IsSuccessStatusCode)
24+
{
25+
return response;
26+
}
27+
28+
string? responseContent = null;
29+
try
30+
{
31+
responseContent = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
32+
}
33+
catch
34+
{
35+
// If we can't read the content, continue with null
36+
}
37+
38+
throw new HttpResponseException(
39+
response.StatusCode,
40+
response.ReasonPhrase,
41+
responseContent,
42+
response.RequestMessage?.RequestUri);
43+
}
44+
}

0 commit comments

Comments
 (0)