-
Notifications
You must be signed in to change notification settings - Fork 614
Include response body in HttpRequestException for transport client errors #1193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include response body in HttpRequestException for transport client errors #1193
Conversation
…ents Replace EnsureSuccessStatusCode with new extension method that reads response body and includes it in the exception message to help diagnose server errors. The HttpRequestException type and StatusCode are preserved. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Address code review feedback to not swallow cancellation exceptions when reading the response body. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Prevents hanging in edge cases where server sends error response but doesn't end the request. Linked CancellationTokenSource ensures the original cancellation token is still honored. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Catch all exceptions including cancellation when reading error response body, since the important thing is throwing the HttpRequestException with the status code. Getting the body is just a "nice to have". Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
halter73
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we trim the body if it's too large for readability in the normal case where the error body isn't that interesting and just represents an outage or something like that?
- Remove comment about timeout as suggested by @halter73 - Trim response body to 8KB max for readability, appending "..." if truncated Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Added trimming - response bodies are now limited to 8KB max, with "..." appended if truncated. (1d7fe0c) |
1KB is sufficient to capture typical error messages while improving readability. Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Head branch was pushed to by a user without write access
EnsureSuccessStatusCodethrows without the response body, making server error diagnostics difficult when the body contains error details (e.g., 400 Bad Request with validation messages).Changes
New
HttpResponseMessageExtensionsinsrc/Common/:EnsureSuccessStatusCodeWithResponseBodyAsync()- reads body before throwing (with 5-second timeout to prevent hangs)CreateHttpRequestException()- constructs exception with body in messageUpdated transport clients to use the new method:
SseClientSessionTransport(connect + send)StreamableHttpClientSessionTransport(send)ClientOAuthProvider(token exchange + metadata fetch)Result
Before:
After:
HttpRequestException.StatusCodeis preserved on .NET targets.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.