Powershell: Original Response Body Unattainable from Web Cmdltes on Non-Success Status Codes
When a non-success status code is returned the web cmdlets provide a convenient error message and attaches the HttpResponseMessage object to the Response property of the exception. In 5.1 cmdltes which wrap Invoke-RestMethod and Invoke-WebRequest could run the following to retrieve the raw response body:
try {
Invoke-WebRequest 'http://httpbin.org/status/418' -ErrorAction Stop
}
Catch {
$err = $_
}
$Stream = $err.Exception.Response.GetResponseStream()
$Stream.Position = 0
$Reader = [System.IO.StreamReader]::new($Stream)
$Reader.ReadToEnd()
This is needed because sometimes the response body for error codes contain useful information about why something failed.
In 6.0.0 the equivalent code is below in the "Steps to Reproduce", This would work except that the web cmdltes are disposing the stream after creating the error message:
This makes it impossible to get the original stream. This would probably not be a problem except that there are encoding issues to consider and that the string in the error message is doctored to make it more human readable:
I recommend not disposing the streamreader and let it naturally fall out of scope as there does not appear to be a way to dispose of a streamreader without closing the underlying stream.
Steps to reproduce
try {
Invoke-WebRequest 'http://httpbin.org/status/418' -ErrorAction Stop
}
Catch {
$err = $_
}
$Stream = $err.Exception.Response.Content.ReadAsStreamAsync().GetAwaiter().GetResult()
$Stream.Position = 0
$Reader = [System.IO.StreamReader]::new($Stream)
$Reader.ReadToEnd()
Expected behavior
-=[ teapot ]=-
_...._
.' _ _ `.
| ."` ^ `". _,
\_;`"---"`|//
| ;/
\_ _/
`"""`
Actual behavior
Exception calling ".ctor" with "1" argument(s): "Stream was not readable."
At line:1 char:1
+ $Reader = [System.IO.StreamReader]::new($Stream)
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : NotSpecified: (:) [], MethodInvocationException
+ FullyQualifiedErrorId : ArgumentException
You cannot call a method on a null-valued expression.
At line:1 char:1
+ $Reader.ReadToEnd()
+ ~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (:) [], RuntimeException
+ FullyQualifiedErrorId : InvokeMethodOnNull
Environment data
Name Value
---- -----
PSVersion 6.0.0-rc
PSEdition Core
GitCommitId v6.0.0-rc
OS Microsoft Windows 10.0.15063
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0
markekraus
All 27 comments
/cc @lipkau
markekraus
on 27 Nov 2017
Any reason why this has to be a Stream and not just parsed automatically into Exception.RawMessage or maybe Exception.Response.RawMessage? Memory usage? Maybe do both if backwards compatibility is the only concern?
Why is it stripping out the formatting?
I hate the idea of having to do extra code to read the stream if the common scenario is to want that detail.
dragonwolf83
on 29 Nov 2017
Any reason why this has to be a Stream
Encoding issues as well as the fact that it could be binary data in the stream.
parsed automatically into Exception.RawMessage
This is already done on the ErrorDetails.Message with minor changes. But if you want this, please open a separate issue.
Memory usage?
~That, and the fact that any extra processing is wasted processing time. It will slow down errors when most users will not care about this data. Fail-fast!~ I just realized the raw string is already there so that's not really a concern.
So, the reason for this is to not destroy the stream that is already available in the HttpResponseMessage so that advanced users can do with that as they please. This has no bearing on any other error related issues, just keeping the stream available instead of closing it and leaving the user with no way to access it.
backwards compatibility is the only concern?
Backwards compatibility is not even a concern here at all as this would absolutely not work the same in 5.1 and 6.0.0.
Why is it stripping out the formatting?
The code comment states it's for readability. It is stripping HTML tags. Compare the following on 6.0.0 and 5.1:
iwr https://google.com/noloexisto
iwr https://httpbin.org/status/418
IMO, this is an improvement (though imperfect) over 5.1
I hate the idea of having to do extra code to read the stream if the common scenario is to want that detail.
It's not common. Most users will find ErrorDetails.Message sufficient. This scenario addresses advanced and less common needs such as wrapping and proxying the cmdlets. This is an area where it is better to leave it up to the user to decide what they want to do with it. As far as PowerShell is concerned, an error occurred. This at least provides the user the opportunity to do something useful with the error, should they choose to and most will not.
markekraus
on 29 Nov 2017
Do we get resource leaks? Seems it is an edge case - maybe add a new parameter to get extended error message?
iSazonov
on 8 May 2018
@iSazonov it is definitely not an edge case. I run into this quite a bit when implementing error handling. The truth is that error bodies contain useful information and we currently mangle it with no way to retrieve the original data. While it is true that _most_ users will not run into issues with this, some _major projects_ (such as those working with Atlassian products) do. It is a problem that presents itself quite clearly to those working closely with APIs. That makes it a step above edge case.
There are several ways to attack this.
I have a plan floating in my head and across several issues here to add support for treating all status codes or a provided list of status code as successful. This would mean that object creation from IRM would proceed as normal and IWR would return its normal response object. This will allow users to interact with non-success messages (such as 3XX, 4XX, and 5XX responses) as if they are normal. In that scenario a module author could indicate the known error messages for the endpoint are success codes, manually check status code, and parse the output. This presents a minor problem with IRM where it is currently not possible to obtain the response code (but this could be remediated).
A second option is to provide a new parameter that results in the response not being disposed when an error occurs. This would make it possible to retrieve the stream on the response object that is attached to the exception.
Another option I've considered is adding a delayed dispose where the stream is available for a period of time and then the object is disposed. I haven't looked deeply into this.. and I kind of think it is a bad idea for a ton of reasons.
Yet another option I have considered is allowing to a -ErrorStreamVariable that accepts a string. this string is used as a variable name in the calling scope (similar to -SessionVariable) for a Stream object. CoptToAsync() is used to copy from the content to the stream, the stream is used to generate the error, and then the stream position is reset to 0. The user could then consume the stream and dispose of it at their leisure in their own code.
markekraus
on 8 May 2018
@markekraus Thanks for great review!
As a user I would prefer to have an optional parameter that will return the contents of the error stream in a variable.
iSazonov
on 8 May 2018
Current behavior makes it impossible to use Invoke-WebRequest with APIs which indicate errors with both HTTP status code and the response body. E.g. if a server returns HTTP status 400 and the following response: {"code":1,"message":"<description>"} then the contents of "message" field will be formatted away and, with the response stream closed, it is impossible to restore the value returned by the server.
MaxKot
on 20 Feb 2019
This coupled with the automatic throw on non-200 status codes makes proper error handling from APIs impossible in PowerShell
felixfbecker
on 6 May 2019
Interesting, will we get anything related (useful) in .Net Core 3.0?
iSazonov
on 6 May 2019
@iSazonov Not really. This is pretty much a mess of our own making. We could redo a huge chunk of code to take advantage of some of the baked in stream decoding, but none of that would affect this current situation.
markekraus
on 7 May 2019
I have thought deeply about this for months now. I think the correct path forward is to add a -SkipErrorCheck parameter which treats all responses as successful.
Since this only affects error handling when the author wants to manually handle errors, they will likely be looking for errors in the responses themselves.
$Uri = 'http://urlecho.appspot.com/echo?status=404&Content-Type=application%2Fjson&body=%7B%22error%22%3A%22Does%20not%20exist%22%7D'
$Result = Invoke-RestMethod -Uri $Uri -SkipErrorCheck
$Result.error
'--------------'
$Result = Invoke-WebRequest -Uri $Uri -SkipErrorCheck
if(!$Result.BaseResponse.IsSuccessStatusCode) {
$ErrorMessage = 'Request failed: {0}' -f $Result.Content
Write-Error $ErrorMessage
}
Result
Does not exist
--------------
Request failed: {"error":"Does not exist"}
+ CategoryInfo : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException
The understanding is that since the user wants to manually handle errors in their own code, we can pass that responsibility to them and let them do what they want.
Incidentally, this also resolved the issues surrounding not being able to retrieve the redirect uri for an end point.
$Result = Invoke-WebRequest https://httpbin.org/absolute-redirect/3 -MaximumRedirection 1 -SkipErrorCheck
$Result.Headers.Location
Result:
http://httpbin.org/absolute-redirect/2
Can we re-use -ErrorAction parameter?
If no I'd suggest more explicit parameter name SkipHttpErrorCheck.
iSazonov
on 7 May 2019
I wouldn't reuse the -ErrorAction here; there are other sources of potential error here than just the responding web server.
vexx32
on 7 May 2019
there are other sources of potential error here than just the responding web server.
It depends on how more we want generalize "treats all responses as successful".
iSazonov
on 7 May 2019
No. We should not reuse the ErrorAction. There are issues which will result in the cmdlets failing that are not related to the response from the web server as @vexx32 points out. Those errors would never produce any error message from the web server and this is targeted specifically at handling error response from the web server which have useful data in them.
I'm good with -SkipHttpErrorCheck.
markekraus
on 7 May 2019
I think -SkipHttpErrorCheck is not the right solution. Invoke-RestMethod only returns the parsed content of the response. With -SkipHttpErrorCheck we would be able to inspect the error response, but we would have no way to know it is an error response, we have no way to check the status code. One could always use Invoke-WebRequest, but then you can鈥檛 use features like -FollowRelLinks, have to content-switch and parse yourself, etc.
Imo especially for a high-level cmdlet like Invoke-RestMethod, it should abstract HTTP as it does already, meaning it should convert HTTP error responses into PowerShell errors.
Why can鈥檛 we expose the response content on the exception object?
felixfbecker
on 7 May 2019
@felixfbecker To address that we could add a StatusCodeVariable parameter which would create a variable in the calling scope with the status code.
$Uri = 'http://urlecho.appspot.com/echo?status=404&Content-Type=application%2Fjson&body=%7B%22error%22%3A%22Does%20not%20exist%22%7D'
$Result = Invoke-RestMethod -Uri $Uri -SkipErrorCheck -StatusCodeVariable StatusCode
if($StatusCode -ne 200) {
Write-Error $Result.Error
}
In a situation where you are doing this, you are already planning to handle HTTP errors yourself so you would be inspecting the response content for errors instead of the exception. Often times those are structured errors, meaning JSON or XML. With what you are proposing by putting th content on the exception, we then have to still translate the content to string then the string to structure objects.
We have discussed the issues with making the content available on the exception earlier in the thread. There are other issue surrounding our simplified errors.
markekraus
on 7 May 2019
@markekraus is there a specific reason we wouldn't expose the status code just as a member of the returned object?
vexx32
on 7 May 2019
To address that we could add a StatusCodeVariable parameter which would create a variable in the calling scope with the status code.
That doesn't sound like a good solution. It will mean you cannot pipe the result of the function anymore into a pipeline to further process it, instead you will have to save it in a variable, then check the magically created variable first, then continue with the result variable. Also, no IntelliSense on magic variables.
In a situation where you are doing this, you are already planning to handle HTTP errors yourself so you would be inspecting the response content for errors instead of the exception.
You cannot (and should not in a REST API, because that's what status codes are for) reliably determine just from the response body whether it is an error. For example, GitHub's REST API's error response look like {"message": "Something went wrong"}. GitHub's REST endpoint for commits also returns an object with a message property. How would you know whether it is an error or a commit? Checking for other properties is really hairy, and the function that does the API request often doesn't know what returned schema for that particular resource is, only the caller knows.
With what you are proposing by putting th content on the exception, we then have to still translate the content to string then the string to structure objects.
What is the problem with that? Invoke-RestMethod does that in the success case, why should it not do it in the error case?
We have discussed the issues with making the content available on the exception earlier in the thread. There are other issue surrounding our simplified errors.
The only real argument I saw was not destroying the stream. But that can't be a problem because ErrorDetails already contains the response as a string, so somewhere the stream is already read anyway. Why can't that string also be exposed on a different property, but without the HTML stripping, and in the same form that the result would be in (i.e. parse it depending on Content-Type just like Invoke-RestMethod does for response bodies of success responses).
felixfbecker
on 7 May 2019
@vexx32 because that would mess with the source object. These are structure objects, adding our own members to those objects could result in a clash or confusion. also.. would be awkward on string result.
markekraus
on 7 May 2019
That doesn't sound like a good solution. It will mean you cannot pipe the result of the function anymore into a pipeline
When you are handling your own errors... you are almost always using the assignment paradigm and not the pipeline paradigm.
How would you know whether it is an error or a commit?
From the status code. That's the same way it is handled in every other language besides powershell.
What is the problem with that? Invoke-RestMethod does that in the success case, why should it not do it in the error case?
By we.. I meant the module authors, not the powershell authors. Again. please review the complication pointed out earlier in the thread.
markekraus
on 7 May 2019
When you are handling your own errors... you are almost always using the assignment paradigm and not the pipeline paradigm.
I don't know why you'd make that assumption. I almost _always_ combine Invoke-RestMethod with a pipeline. E.g.:
try {
Invoke-RestMethod /myapi/$Something |
ForEach-Object { $_.items } |
Where-Object { $_.id -eq $Id }
} catch [WebException] {
if ($_.Exception.StatusCode -eq 401 -and $_.Exception.Body.code -eq 'token_expired') {
Write-Error 'The token is expired. Generate a new one with New-MyApiToken'
return
} else {
Write-Error $_
}
}
From the status code.
Yes, but without a magic variable.
That's the same way it is handled in every other language besides powershell.
If by this you mean throwing an Exception for non-200 status codes, that couldn't be further from true. HTTP libraries in NodeJS all do this, .NET/C# does this (PowerShell is literally using the same Exception type), Java has HttpException too. Exceptions are really convenient and the proper representation of HTTP errors in any language that has them.
By we.. I meant the module authors, not the powershell authors.
Why can't we expose the parsed contents?
Again. please review the complication pointed out earlier in the thread.
I read the thread twice and still don't know what you're referring to. Could you be more specific? What is blocking PowerShell from exposing the parsed contents on the Exception object?
felixfbecker
on 7 May 2019
I don't know why you'd make that assumption.
I spend a lot of time writing, consulting on, and reviewing REST wrappers created in PowerShell. Based on experience, when someone wants to perform error handling manually they almost always use the assignment paradigm. But let's not argue on that. Neither of us have time to pull stats from github and other places for that.
.NET/C#
We are using HttpClient. Both HttpClient and WebRequest APIs do not throw on a non-200 status. If you look at our code, we are creating the HttpReponseException in the PowerShell code.
Why can't we expose the parsed contents? Could you be more specific? What is blocking PowerShell from exposing the parsed contents on the Exception object?
This has to do with the simplified error messages. Essentially, we can have one or the other, but not both without performance sacrifices. Also, the error handling is done in the base cmdlet class, which means it needs to behave similarly for both Invoke-WebRequest and Invoke-RestMethod. Factor in binary responses, no way to ensure disposal, and that these errors accumulate in the $Error variable without falling into the garbage collector and you have a recipe for unintended performance impacts.
In 5.1 the WebRequest API provided a stream that could be reused. In 6.0 with the move to HttpClient the stream cannot be reused and additionally we dispose it because we have to basically duplicate it in memory to perform string processing on it to give a meaningful error.
markekraus
on 8 May 2019
:tada:This issue was addressed in #10466, which has now been successfully released as v7.0.0-preview.6.:tada:
Handy links:
![msftbot[bot] picture](https://avatars2.githubusercontent.com/in/26612?v=4&s=40)
msftbot[bot]
on 21 Nov 2019
This has to do with the simplified error messages.
Are you talking about this?
Invoke-WebRequest:
AccessDeniedAccess DeniedC99953C5A28DBED1IexDfeKyKtSfznqe611PPDBGkbsH7jbvf5H/cZNN7K2pTM9e2SIAX7tFIpGgWem3/YKlUqFTfqA=
This is not simplified, this is plain old garbage. Here is the error message I would expect to appear:
Invoke-WebRequest: Forbidden
Error:
Code : AccessDenied
Message : Access Denied
RequestId : FCD8C82E32E026FD
HostId : Qh5CRv7KHUv4ro3pwWh2F1dXTTl/qOQLK88/Klle4Jut5dvDufpVzxLVpYMio4DZKdqgKq/JoTM=
yecril71pl
on 23 Jul 2020
@yecril71pl Please open new issue if you want to improve the error formatting.
iSazonov
on 23 Jul 2020
yecril71pl
on 27 Jul 2020
Related issues
Michal-Ziemba
路
3Comments
concentrateddon
路
3Comments
rudolfvesely
路
3Comments
mklement0
路
3Comments
alx9r
路
3Comments
Most helpful comment
This coupled with the automatic throw on non-200 status codes makes proper error handling from APIs impossible in PowerShell