Add StatusCode to HttpRequestException - #32455
#32455Add StatusCode to HttpRequestException#32455scalablecory merged 12 commits intodotnet:masterdotnet/runtime:masterfrom yaakov-h:httprequestexception-statuscodeyaakov-h/dotnet-runtime:httprequestexception-statuscodeCopy head branch name to clipboard
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Don't worry about it as part of this PR. If/when someone makes this exception serializable, it'll be up to them to do it in a way that is appropriately compatible with .NET Framework. |
|
ok, I've added some xmldoc comments by pretty much lifting text from similar members elsewhere and tweaking to suit. |
The reason why I didn't make that exception serializable is because on .NET Framework it is being serialized with a different mechanism that we don't use on Core: http://index/?query=HttpRequestException (internal link only visible for MSFT employees). We could still make that Exception serializable with the standard mechanism but it wouldn't be exchangeable with .NET Framework. |
scalablecory
left a comment
There was a problem hiding this comment.
Looks good. Are there any other tests that could have StatusCode asserted in? Testing the convenience methods e.g. GetAsync would be good.
…Message.cs Co-Authored-By: Cory Nelson <phrosty@gmail.com>
|
@scalablecory Testing the basic convenince methods could be useful. Would I also need to test the convenience methods in |
The unit tests for this PR should focus only on existing APIs in HttpClient. |
|
@yaakov-h Can you clarify if you've addressed all PR feedback? There are still some unresolved comments in the PR. Thanks. |
|
/azp list |
|
/azp run runtime-libraries outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@davidsh unless there are any further comments to be had on the xmldoc, I think that's everything. |
|
It looks good to me, my "accept" check still stands. Thanks for the PR. |
HttpRequestException.StatusCode property gets the HTTP status code associated with the exception if any. It was introduced in dotnet/runtime#32455
Implements #911.
TODO:
Code:
Add tests that the property is persisted when the exception is serialized.Docs:
The exception doesn't seem to be marked as Serializable in any way, so I'm not sure if/how I should deal with exception serialization.