search

xamarin / xamarin-macios

.NET for iOS, Mac Catalyst, macOS, and tvOS provide open-source bindings of the Apple SDKs for use with .NET managed languages such as C#
Other
2.44k stars 509 forks source link

[iOS] HttpClient GetAsync does not consider connection loss as an error #5215

Closed samhouts closed 5 years ago

samhouts commented 5 years ago

From @Flheriau on November 13, 2018 15:53

Description

I'm using HttpClient to download files from URLs with GetAsync method. With huge images for example, when I stop internet connection manually in the middle of the download, it is considered as a normal ending for the GetAsync method and the resulting HttpResponse is totally normal (status 200, no error message, no exception). Depending of the moment of the connection loss, the resulting image corresponds to the download progress (starting from the top of the image).

Steps to Reproduce

  1. Download an image from a public URL using HttpClient.GetAsync
  2. Disable your internet connection during the download
  3. Set an Image ImageSource with the response

Expected Behavior

At least an exception for the connection loss or another status for the HttpResponse

Actual Behavior

Nothing.

Basic Information

Sample project

TestHttpClientGet.zip

Simple sample project with a "Download" button. To reproduce, you need to stop internet connection after you clicked on Download.

Copied from original issue: xamarin/Xamarin.Forms#4392

samhouts commented 5 years ago

From @Flheriau on November 15, 2018 10:23

You can now reproduce with the sample project. I should have put it the first time !

samhouts commented 5 years ago

From @kingces95 on November 16, 2018 23:35

Could you please provide a sample app as well as the URL used download an image that reproduces the problem?

samhouts commented 5 years ago

From @Flheriau on November 19, 2018 7:48

You can see a Sample project in the description. The image used is 10 Mo large to have the time to stop the internet connection. (https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg)

samhouts commented 5 years ago

From @Flheriau on November 22, 2018 9:2

Any update on this ? Did you manage to reproduce the problem with the sample ?

VincentDondain commented 5 years ago

Hi,

So I see some useful timeout info in the logs but, for the NSUrlSession http client handler, no exception is being raised.

I see:

2018-12-03 15:37:42.047358-0500 TestHttpClientGet.iOS[38962:9553965] Task <99165598-5C60-4F12-9A63-5B195A83D714>.<1> finished with error - code: -1001
2018-12-03 15:37:42.048418-0500 TestHttpClientGet.iOS[38962:9552315] Task <99165598-5C60-4F12-9A63-5B195A83D714>.<1> load failed with error Error Domain=NSURLErrorDomain Code=-1001 "The request timed out." UserInfo={_kCFStreamErrorCodeKey=-2102, NSUnderlyingError=0x6000036a1dd0 {Error Domain=kCFErrorDomainCFNetwork Code=-1001 "(null)" UserInfo={_kCFStreamErrorCodeKey=-2102, _kCFStreamErrorDomainKey=4}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <99165598-5C60-4F12-9A63-5B195A83D714>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
    "LocalDataTask <99165598-5C60-4F12-9A63-5B195A83D714>.<1>"
), NSLocalizedDescription=The request timed out., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=4} [-1001]
2018-12-03 15:37:42.156921-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding failed with error code -1
2018-12-03 15:37:42.157051-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding: C0 0x0FA00A12 0x0000354A 0x11111100 0x0050C010 147072
2018-12-03 15:37:42.157124-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Options: 1x-1 [00000000,0FA0035C] 0001D060
2018-12-03 15:37:42.158184-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding failed with error code 7
2018-12-03 15:37:42.158247-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding: C0 0x0FA00A12 0x0000354A 0x11111100 0x0050C010 147072
2018-12-03 15:37:42.158309-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Options: 1x-1 [0000035C,0FA0035C] 0001D060
2018-12-03 15:37:42.166405-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding failed with error code -1
2018-12-03 15:37:42.166520-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding: C0 0x0FA00A12 0x0000354A 0x11111100 0x0050C010 147072
2018-12-03 15:37:42.166602-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Options: 1x-1 [00000000,0FA0035C] 0001D060
2018-12-03 15:37:42.167590-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding failed with error code 7
2018-12-03 15:37:42.167659-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Decoding: C0 0x0FA00A12 0x0000354A 0x11111100 0x0050C010 147072
2018-12-03 15:37:42.167734-0500 TestHttpClientGet.iOS[38962:9552057] [0x7f950ba20800] Options: 1x-1 [0000035C,0FA0035C] 0001D060

If I'm using the managed HttpClient handler, I'm entering the catch and getting:

2018-12-03 15:57:01.635516-0500 TestHttpClientGet.iOS[39113:9598884] System.Threading.Tasks.TaskCanceledException: A task was canceled.
  at System.Net.Http.HttpClientHandler+<SendAsync>d__64.MoveNext () [0x004ae] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClientHandler.cs:414 
--- End of stack trace from previous location where exception was thrown ---
  at System.Net.Http.HttpClient+<SendAsyncWorker>d__48.MoveNext () [0x00080] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:276 
--- End of stack trace from previous location where exception was thrown ---
  at TestHttpClientGet.MainViewModel+<GetSource>d__14.MoveNext () [0x0004a] in /Users/vidondai/Downloads/TestHttpClientGet/TestHttpClientGet/TestHttpClientGet/MainViewModel.cs:35
spouliot commented 5 years ago

@VincentDondain there's another timeout issue in https://github.com/xamarin/xamarin-macios/issues/5190 You might want to try the workaround (I have a proper fix in progress).

spouliot commented 5 years ago

I tried to duplicate this before checking if the fix for #5190 had an impact. However it does seems to behave like I would expect, i.e. entering the catch.

2018-12-19 09:54:58.199599-0500 TestHttpClientGet.iOS[84132:4039663] Task <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1> HTTP load failed (error code: -1009 [1:50])
2018-12-19 09:54:58.200934-0500 TestHttpClientGet.iOS[84132:4039663] Task <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1> finished with error - code: -1009
2018-12-19 09:54:58.203703-0500 TestHttpClientGet.iOS[84132:4039697] Task <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1> load failed with error Error Domain=NSURLErrorDomain Code=-1009 "The Internet connection appears to be offline." UserInfo={_kCFStreamErrorCodeKey=50, NSUnderlyingError=0x600000af35a0 {Error Domain=kCFErrorDomainCFNetwork Code=-1009 "(null)" UserInfo={_kCFStreamErrorCodeKey=50, _kCFStreamErrorDomainKey=1}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
    "LocalDataTask <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1>"
), NSLocalizedDescription=The Internet connection appears to be offline., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=1} [-1009]
Thread started:  #4
2018-12-19 09:55:08.807948-0500 TestHttpClientGet.iOS[84132:4039652] System.Net.WebException: The Internet connection appears to be offline. ---> Foundation.NSErrorException: Error Domain=NSURLErrorDomain Code=-1009 "The Internet connection appears to be offline." UserInfo={_kCFStreamErrorCodeKey=50, NSUnderlyingError=0x600000af35a0 {Error Domain=kCFErrorDomainCFNetwork Code=-1009 "(null)" UserInfo={_kCFStreamErrorCodeKey=50, _kCFStreamErrorDomainKey=1}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
    "LocalDataTask <353EC60B-CB34-4BCD-8F3C-990160AB226E>.<1>"
), NSLocalizedDescription=The Internet connection appears to be offline., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=1}
   --- End of inner exception stack trace ---
  at System.Net.Http.NSUrlSessionHandler.SendAsync (System.Net.Http.HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) [0x001c3] in /Users/poupou/git/master/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:260 
  at System.Net.Http.HttpClient.SendAsyncWorker (System.Net.Http.HttpRequestMessage request, System.Net.Http.HttpCompletionOption completionOption, System.Threading.CancellationToken cancellationToken) [0x00080] in /Users/poupou/git/master/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:276 
  at TestHttpClientGet.MainViewModel.GetSource () [0x0004a] in /Users/poupou/Downloads/TestHttpClientGet/TestHttpClientGet/TestHttpClientGet/MainViewModel.cs:35

It might already have been fixed inside the mono 2018 (since I'm using master)

spouliot commented 5 years ago

I get similar results with current stable (12.2.1.12).

2018-12-19 10:42:37.449480-0500 TestHttpClientGet.iOS[85314:4089879] Task <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1> HTTP load failed (error code: -1009 [1:50])
2018-12-19 10:42:37.450532-0500 TestHttpClientGet.iOS[85314:4090285] Task <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1> finished with error - code: -1009
2018-12-19 10:42:37.456014-0500 TestHttpClientGet.iOS[85314:4089879] Task <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1> load failed with error Error Domain=NSURLErrorDomain Code=-1009 "The Internet connection appears to be offline." UserInfo={_kCFStreamErrorCodeKey=50, NSUnderlyingError=0x600001aadfb0 {Error Domain=kCFErrorDomainCFNetwork Code=-1009 "(null)" UserInfo={_kCFStreamErrorCodeKey=50, _kCFStreamErrorDomainKey=1}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
    "LocalDataTask <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1>"
), NSLocalizedDescription=The Internet connection appears to be offline., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=1} [-1009]
Thread started:  #4
2018-12-19 10:42:41.018184-0500 TestHttpClientGet.iOS[85314:4089868] System.Net.WebException: The Internet connection appears to be offline. ---> Foundation.NSErrorException: Error Domain=NSURLErrorDomain Code=-1009 "The Internet connection appears to be offline." UserInfo={_kCFStreamErrorCodeKey=50, NSUnderlyingError=0x600001aadfb0 {Error Domain=kCFErrorDomainCFNetwork Code=-1009 "(null)" UserInfo={_kCFStreamErrorCodeKey=50, _kCFStreamErrorDomainKey=1}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
    "LocalDataTask <410654FF-AB7E-4FF1-AD87-57FAA701D9AB>.<1>"
), NSLocalizedDescription=The Internet connection appears to be offline., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=1}
   --- End of inner exception stack trace ---
  at System.Net.Http.NSUrlSessionHandler+<SendAsync>d__29.MoveNext () [0x001c3] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:202 
--- End of stack trace from previous location where exception was thrown ---
  at System.Net.Http.HttpClient+<SendAsyncWorker>d__48.MoveNext () [0x00080] in /Users/builder/jenkins/workspace/xamarin-macios/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:276 
--- End of stack trace from previous location where exception was thrown ---
  at TestHttpClientGet.MainViewModel+<GetSource>d__14.MoveNext () [0x0004a] in /Users/poupou/Downloads/TestHttpClientGet/TestHttpClientGet/TestHttpClientGet/MainViewModel.cs:35

@VincentDondain it's not clear which XI you used. Can you try again ? thanks!

VincentDondain commented 5 years ago

So with XI stable (a.k.a 12.2.1.12) I still get exactly the same behavior as I described here https://github.com/xamarin/xamarin-macios/issues/5215#issuecomment-443883288. I do not enter the catch but I am getting Code=-1001 "The request timed out." in the application output.

I tried with master 12.6.0.8 (built locally) and same thing, not entering the catch but I have the timeout in the logs.

@spouliot your iOS project is set to NSUrlSession for the HttpClient implementation right? Cause the catch is hit just fine with "Managed".

Note: we don't hit the same exception. I tried with 2 different techniques: network conditioner 100% loss and turn off wifi (with not ethernet cable obviously). I'm killing the connection as soon as I hit the download button.

spouliot commented 5 years ago

your iOS project is set to NSUrlSession for the HttpClient implementation right?

@VincentDondain yes :) it's part of the stack trace (showing both the handler and that I'm getting the exception)

walterlv commented 5 years ago

@VincentDondain I actually met totally the same StackTrace of yours and the same 0x001c3 error code.

mandel-macaque commented 5 years ago

@samhouts We have made some updates in the handler, I'll recheck the status of this issue.

dotMorten commented 5 years ago

@mandel-macaque Do you have a link to the PR for those handler updates? We're being blocked by this issue as well.

mandel-macaque commented 5 years ago

I have checked the current status of the given project, I tested the application with the following environment: https://gist.github.com/mandel-macaque/f9ce8bc4348f1d4e9dde000f677e7ead

It looks like there is an issue with the Handler, which I used with an iPhon 8+ (iOS 12.1.4) resulting in no timeout exception, but with a partial image beings used:

IMG_8112

@dotMorten I will work on a fix. If the network is lost (which I'm using via airplane mode) we should be getting an exception. The PRs I talked was https://github.com/xamarin/xamarin-macios/issues/5190 but it is not throwing an exception. Will link here ASAP.

eric906 commented 5 years ago

FYI... We are completely blocked by this, as are 1000+ of our users at the moment. Thank you for your attention to this!

mandel-macaque commented 5 years ago

@spouliot @VincentDondain it looks like we had an issue when setting the exception on both the response and the stream, which kind of explains why you could get different results.

I have done a fair amount of testing to find out when this is happening and I believe the issue is as follows:

  1. We receive data and is stored in the buffer.
  2. Receive the Error from the OS.
  3. Set the exception in the response (correct).
  4. Stream continues reading in a diff thread, ergo everything seems ok.
  5. Set the exception in the stream, this is done in two steps: a. Set the inner exception. b. Set stream as completed.
  6. App received read data and that the stream is completed because a thread got paused between a and b in step 5, ergo everything points that we are ok.

I believe that the fix would be to:

  1. Block the reading when setting the exceptions in both the stream and the response.
  2. Before we return in the read async, check again for an exception, if it happened, disregard the read data.

I've got a fix for it. I have tested it on device and on the simulator by using airplane mode on device or disconnecting the sim completely from the network. With the above reasoning, we always get the exception when disconnected even when using caching.

With the fix we get an exception similar to the following:

{System.Net.WebException: The network connection was lost. ---> Foundation.NSErrorException: Error Domain=NSURLErrorDomain Code=-1005 "The network connection was lost." UserInfo={_kCFStreamErrorCodeKey=57, NSUnderlyingError=0x60000344ba20 {Error Domain=kCFErrorDomainCFNetwork Code=-1005 "(null)" UserInfo={NSErrorPeerAddressKey=<CFData 0x6000019200f0 [0x10970dae8]>{length = 16, capacity = 16, bytes = 0x100201bb86ab4b210000000000000000}, _kCFStreamErrorCodeKey=57, _kCFStreamErrorDomainKey=1}}, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDataTask <2CC92646-DD65-40D6-951A-B6D0D60F54BC>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=( "LocalDataTask <2CC92646-DD65-40D6-951A-B6D0D60F54BC>.<1>" ), NSLocalizedDescription=The network connection was lost., NSErrorFailingURLStringKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, NSErrorFailingURLKey=https://www.spacetelescope.org/static/archives/images/publicationjpg/heic0711a.jpg, _kCFStreamErrorDomainKey=1} --- End of inner exception stack trace --- at System.Net.Http.NSUrlSessionHandler+NSUrlSessionDataTaskStream.ThrowIfNeeded (System.Threading.CancellationToken cancellationToken) [0x00008] in /Users/mandel/Xamarin/xamarin-macios/master/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:655 at System.Net.Http.NSUrlSessionHandler+NSUrlSessionDataTaskStream.ReadAsync (System.Byte[] buffer, System.Int32 offset, System.Int32 count, System.Threading.CancellationToken cancellationToken) [0x0001b] in /Users/mandel/Xamarin/xamarin-macios/master/xamarin-macios/src/Foundation/NSUrlSessionHandler.cs:668 at System.IO.Stream.CopyToAsyncInternal (System.IO.Stream destination, System.Int32 bufferSize, System.Threading.CancellationToken cancellationToken) [0x000cf] in /Users/mandel/Xamarin/xamarin-macios/master/xamarin-macios/external/mono/mcs/class/referencesource/mscorlib/system/io/stream.cs:171 at System.Net.Http.HttpContent.LoadIntoBufferAsync (System.Int64 maxBufferSize) [0x00064] in /Users/mandel/Xamarin/xamarin-macios/master/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpContent.cs:146 at System.Net.Http.HttpClient.SendAsyncWorker (System.Net.Http.HttpRequestMessage request, System.Net.Http.HttpCompletionOption completionOption, System.Threading.CancellationToken cancellationToken) [0x0012a] in /Users/mandel/Xamarin/xamarin-macios/master/xamarin-macios/external/mono/mcs/class/System.Net.Http/System.Net.Http/HttpClient.cs:286 at TestHttpClientGet.MainViewModel.GetSource () [0x00050] in /Users/mandel/Downloads/TestHttpClientGet/TestHttpClientGet/TestHttpClientGet/MainViewModel.cs:36 }

mandel-macaque commented 5 years ago

The proposed change in commit https://github.com/xamarin/xamarin-macios/pull/5769/commits/4e6241b4348b6bd1df66838f71f7fd759700a826 should be fixing the issue and will ensure that you do a get TimeoutException (with the correct inner exception) in a consistent manner.

The underlaying issue is the one I explained in my previous comment, changing the CancellationToken that is passed to the HttpCotent is a much safer bet than locking on the data read, which could still show some issue (less common, but I'm sure someone would hit it and reopen the issue) and a less async implementation.

mandel-macaque commented 5 years ago

Oh! if TimeoutException is not the one wanted, please comment in the PR :)

dotMorten commented 5 years ago

Well technically that doesn't sound like the right exception, but the main issue is an exception wasn't thrown and you assume the download is valid, so any exception is a million times better than no exception. Thank you for jumping on this!

mandel-macaque commented 5 years ago

@dotMorten Please do comment in the PR about it. There are several options we can do about the exception, I was reading the CoreFx issues and they had a similar "discussion". In their case, users complained because they had a CancelationTaskException that was hard to identify (was the task cancelled via the CancellationToken in the SendAsync or was it due to another reason, ie Timeout).

On our side, we have a diff problem but a similar results, after @spouliot PR we are getting a task cancelled when we reach the timeout, the reason is that there is a mismatch between the default timeout of the NSUrlSession and the HttpClient timeout. Due to that, we have a timeout BEFORE we get the correct NSError (stating that the connection was lost). @spouliot is correct in the sense that we want to respect the timeout, and technically, there is no real way I can propagate the NSError to the Stream of the HttpContent :/

Nevertheless, because we can control the exception raised, we could add a different one if it makes more sense. But as you said, we are getting the exception raised, which is what was truly needed. (I need to find the area of our docs in which I could write a small explanation for developers).

mandel-macaque commented 5 years ago

In order to ensure that we will back port this from master to d16-1 could we please get some feedback from the recent build? Packages can be found in the following links.

Your feedback is very welcome.

Flheriau commented 5 years ago

Hi, Thank you for your work, you have been so effective ! 🥇 I've tested my sample project with the custom Xamarin.iOS package and it works like a charm ! If the network is lost (airplane mode in my case), we are getting a TimeoutException (InnerException : TaskCanceled) so it's perfect for me 😃

eric906 commented 5 years ago

Thanks for this! Huge. Is there an ETA on when it will be made available to general public?

mandel-macaque commented 5 years ago

@eric906 the bar to back port the fix to d16-0 is very high but I will be doing the port to d16-1. I really cannot give an exact time for it since it depends on the release managers. @chamons might be able to give you an estimate.

eric906 commented 5 years ago

@mandel-macaque Thank you for the quick response! Greatly appreciated. Understood. I'll stand by.

chamons commented 5 years ago

So a few things:

eric906 commented 5 years ago

@chamons Sounds good. Thanks!

abdullahdev2015 commented 4 years ago

Replace NSUrl to Managed kfws5q8isqgc