Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for issue #174 - NSURLSessionTask completion block not called when not following redirects #175

Merged
merged 1 commit into from
May 14, 2016

Conversation

vfig
Copy link
Contributor

@vfig vfig commented May 10, 2016

The standard behaviour of NSURLSession, when its delegate says not to follow redirects, and when no custom NSURLProtocol is managing things is to call the completion block with the actual redirect response, its data, and a nil error.

These commits fix OHHTTPStubsProtocol to implement the same behaviour. The behaviour when the delegate says to follow redirects is unchanged.

Note that Apple’s CustomHTTPProtocol sample code instead completes the redirect response with an NSURLErrorCancelled error, diverging from the standard behaviour.

@vfig vfig changed the title Fix for #174 - NSURLSessionTask completion block not called when not following redirects Fix for [#174](https://github.com/AliSoftware/OHHTTPStubs/issues/174) - NSURLSessionTask completion block not called when not following redirects May 10, 2016
@vfig vfig changed the title Fix for [#174](https://github.com/AliSoftware/OHHTTPStubs/issues/174) - NSURLSessionTask completion block not called when not following redirects Fix for issue #174 - NSURLSessionTask completion block not called when not following redirects May 10, 2016
@AliSoftware
Copy link
Owner

AliSoftware commented May 11, 2016

Thanks for the PR 👍 Looks ace! 👌

Could you please:

  • pull the new commits from master then rebase your branch on top of my master branch
    (so that your branch includes the latest commits from master, as I've merged other PRs on my master since yours)
  • then add an entry in the CHANGELOG.md to credit yourself 🎉

Thanks!

When `-[NSURLSessionTaskDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]` calls its completion handler with `nil` to indicate that the redirect should not be followed, the completion handler from `-[NSURLSession dataTaskWithRequest:completionHandler:]` is normally called with the actual redirect response itself. But when using OHTTPStubs such that a stub returns a redirect response, the task’s completion handler block is simply never called.

- Ensure `OHHTTPStubsProtocol` finishes redirect responses fully when not following redirects.
- Update `NSURLSessionTests` to exercise this behaviour.
- Fix `AFNetworkingTests` test that had been looking for the incorrect behaviour.
@vfig
Copy link
Contributor Author

vfig commented May 13, 2016

Rebased and changelog updated. I also squashed it all into one commit and rewrote the commit message to describe the issue better.

@AliSoftware
Copy link
Owner

@adurdin In an effort to try and follow Moya's Contributor Guidelines, and to thank your for your recent contribution to OHHTTPStubs, I just added you as contributor to OHHTTPStubs so that you can now merge PRs and manage issues!
(Note: master is locked against direct commits, in order to force us to make PRs pass tests before we can merge any new code to master)

@vfig
Copy link
Contributor Author

vfig commented May 26, 2016

Thank you, but I’ll decline. I’m not using OHHTTPStubs at all regularly just now, so don’t think I’m in a position to usefully contribute for the moment.

@AliSoftware
Copy link
Owner

@adurdin: Noted 😉
But just to be clear, the push access doesn't come with any requirement nor expectation from your part 😉
If you want to use it do it, if you don't no problem, if you don't have time, no problem either: there is no expectation on you contributing on a regular basis or whatnot, it's just for in case you want to, you now have more access, you can do whatever you want with it… including nothing 😜

@vfig
Copy link
Contributor Author

vfig commented May 26, 2016

Ok, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants