Let it throw on download #1962
Let it throw on download #1962
Conversation
|
@kendallb please check if it meets your expectation. |
|
Yes, that will work. I would still argue that using ThrowOnError in the download function makes no sense because there is no way to inspect the result for an exception if you have it set to false (the default)? So I still think the download function should throw and exception at all times if there is a problem, not just if ThrowOnError is set. I know it would be a breaking change in that it is not what it did before, but I suspect it's a good breaking change because otherwise you just get silently downloaded strange content, which is exactly what we ran across with a site that decided to throw up a fancy text/html 404 page when jpg files didn't exist. Took us a while to sort out it was a 404 which would have been a whole lot easier if it had throw an exception from the start. I am now using ThrowOnError as I prefer exceptions as we have always wrapped our REST calls in exception handlers to handle things like retries, but I can see a situation where someone wants to get an exception when the streaming or download functions fail, yet do not want to have it enabled globally? Using ThrowOnError implicitly assumes there is some other way to consume the failure other than via the exception, which there is not? |
|
I can merge it to dev now and release it as the next minor, and remove the check in 109, where it will always throw. Your point about being unable to inspect the result is totally valid. |
|
Sounds good. I will look for it in dev. |
Fixes #1958