Fix the race condition when user cancel the network loading will not trigger the completion block #3142
Conversation
…trigger the completion block. Now we do not relay on URLSession delegate timing, directly callback when cancelled.
| self.dataTask = nil; | ||
| // Provide the same userInfo as URLSession if network is cancelled | ||
| // Don't relay on `URLSession:task:didCompleteWithError:` delegate callback because it may delay | ||
| [self callCompletionBlocksWithError:[NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorCancelled userInfo:@{NSLocalizedDescriptionKey: @"cancelled", NSURLErrorFailingURLErrorKey: self.request.URL, NSURLErrorFailingURLStringErrorKey: self.request.URL.absoluteString}]]; |
There was a problem hiding this comment.
Should we just return the same SDWebImageErrorDomain.SDWebImageErrorCancelled as other cancel timing cases ?
I don't knwo whether this change is suitable for most cases. @Insofan
There was a problem hiding this comment.
I change this into the SDWebImageErrorDomain.SDWebImageErrorCancelled, which pass the unit test.
This design is the same as Kingfisher.
Codecov Report
@@ Coverage Diff @@
## master #3142 +/- ##
==========================================
+ Coverage 83.38% 83.41% +0.02%
==========================================
Files 70 70
Lines 7674 7675 +1
==========================================
+ Hits 6399 6402 +3
+ Misses 1275 1273 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)This merge request fixes / refers to the following issues: #3141
Pull Request Description
This close #3141
Now we do not relay on URLSession delegate timing, directly callback when cancelled.