-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Convert cpprestsdk to WIL exception for better handling #5188
base: master
Are you sure you want to change the base?
Conversation
@@ -233,4 +241,9 @@ namespace AppInstaller::Http | |||
|
|||
return response.extract_json().get(); | |||
} | |||
|
|||
[[noreturn]] void HttpClientHelper::RethrowAsWilException(web::http::http_exception& exception) |
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 it be "const &" ?
@@ -115,6 +115,10 @@ namespace AppInstaller::Http | |||
|
|||
return ValidateAndExtractResponse(httpResponse); | |||
} | |||
catch (web::http::http_exception& exception) |
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.
nit: the 2 catches here and below could be const too
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.
Putting const on the catch is like putting const on a by-value parameter. It is only there to stop you from changing something you have decided you don't want to change accidentally. I will leave this one.
Change
To better expose the underlying result code, convert the
http_exception
to aResultException
so that the HRESULT can be properly extracted.Validation
Using the PowerShell module and interfering with my connection, I was able to see that the connect result's extended error was originally
8007023E
, but changed to the appropriate80072EE2
with the change.Microsoft Reviewers: Open in CodeFlow