This should handle any errors that can occur while obtaining and reading the response stream, such as invalid argument and IOException exceptions.

wow, you've just found one more issue: IOException is not handled even if the response was ok (200), I thought it was handled somewhere in WebRequestPSCmdlet. I'm going to fix it as well.

..and it is also not handled in fullclr powershell if the response status code was 200 (non-200 handling is here). From another side, it seems legit since it reports the user that an I/O exception occurred and it is pretty clear what happened (e.g. "The connection with the server was terminated abnormally"). Do you consider to wrap some exceptions as inner one of WebException?

Thanks, I think it is Ok in this case. My original concern is hiding the WebRequest exception with an IOException caused while processing the WebRequest exception.

thx, now it handles this situation almost the same way as the full clr implementation does.

Why is the DateTime format specifier {0:d} used here?

d is for decimal:) According to this doc, the format string can only contain the "G" or "g", "D" or "d", "X" or "x", and "F" or "f", i.e. it can't be unambiguous "N0" or "#". As an alternative, I can cast the enum to int explicitly without all these hacks)

I believe "d" in String.Format is reserved for DateTime formatting. I was using this as reference: http://www.csharp-examples.net/string-format-datetime/

I don't think creating a sub type named 'WebException' is the right solution. System.Net.WebException is exposed in .NET Core, in system.net.requests.dll. Adding a stub type with the same name just adds confusion.
All stubs in this file are for types that no longer exist in .NET Core, and eventually we want to clean up the code to remove most of those stubs.

new WebException(msg) { Response = response }

It's great to get the detailed message for the error record to keep a consistent behavior. But I don't think we should use a stub type to mimic the exception in full PS.
Since the content of the response has been extracted and passed to the error record, is there a real scenario where the response object is necessary?

Dongbo Wang (@daxian-dbw) Well, I agree that the Response property looks kind of weird here. However, it helps keep backward compatibility. For example, given an http request, I need to get the status code

Invoke-RestMethod http://httpbin.org/404 
$e = $Error[0]
$e | gm
$e.Exception | gm

In the case of non-200 response, is there another straight way to get the status code in the FullClr implementation except for $e.Exception.Response.StatusCode?

I'm thinking aloud here: how about we add the response object to "Exception.Data" for core PS? Users can get the status code by $e.Exception.Data.Response.
Yes, this is not backward compatible, but we are sort of not backward compatible anyways, for example, members/methods exposed by (invoke-webrequest www.bing.com).BaseResponse are different.

Travis Plunk (@TravisEz13) Jason Shirk (@lzybkr) sergei (@vors) can you please take a look and share your opinions?

Exception.Data was a neat idea, but it's not discoverable and a poor api. Because you typically derive from System.Exception, it's easier to add your data as properties - and more reliable - your property can be readonly.

There are real cases where the response data is needed. Some sites send details of the error in the body, etc. I agree that putting the response under data is less discoverable. The exception message from the base exception should not be altered, though.

Dongbo Wang (@daxian-dbw) in both cases we considered we would have trade-offs. However, in the case of Exception.Data, we would have a breaking change, that could lead to code structures like this $error[0].Exception.Response, $error[0].Exception.Data.StatusCode | select -f 1 (another "great" example). Moreover, it would be hard to guess without taking a look at the source code or scrutinizing MSDN carefully that the status code was in the Data property, all the previous blog posts, kb articles would be outdated.

From another side, in the case of the Response field, oh yes, we would have that a little bit tricky exception logic in the CoreClr implementation, but all that logic would be encapsulated in that method and with tests we could get along with that. Plus, if we find a third solution later, we can refactor it whereas with that breaking change it could be far more complex.

Does anybody have the third solution for this problem:))?

I agree Exception.Data is not discoverable, and also breaks script portability. How about we have an exception type (not defined in the stub namespace) derived from HttpRequestException that exposes a Response property and use that exception when status code is not success? I would like to get the real exception thrown from CoreCLR, and our exception type should have a constructor like public XXXException(Exception e) : base (e.Message, e), so that we can pass in the real HttpRequestException as an inner exception. try { } catch [HttpRequestException] { } would also work this way.

One problem here is that EnsureSuccessStatusCode would dispose HttpResponseMessage.Content, and that means user won't be able to read the body streams anymore. I guess that's not acceptable given Travis Plunk (@TravisEz13)'s comments. More thought is needed.

Could response.Content be null and raise NullReferenceException in GetResponseStream? Maybe we should check response.Content != null && response.Content.Headers.ContentLength > 0 before getting its stream.
And also, could response.Content.ReadAsStreamAsync raise any exception when the response is not in success status? Maybe the code dealing with stream should be put in a try/catch-all block, just like how it does in full PS.

detailMsg = System.Text.RegularExpressions.Regex.Replace(detailMsg, "<[^>]*>", "");

In full PS, a simple regex replace is used to remove tags from the content. We probably should do the same.