Redirect stubbing not working reliably

[nevil]

First of all, thanks for this great tool! Sorry for the disorganized report

I've been having an issue that our redirect unit tests occasionally fail. In the latest release I saw a fix that I was hoping to solve my problem ( #224) but unfortunately not.

Now I have a way to make the redirect stubbing fail reliably. A small modification to OHHTTPStubs.m:

[client URLProtocol:self wasRedirectedToRequest:redirectRequest redirectResponse:urlResponse];
+++ usleep(1000000)

With the above we get the failure 100% of the time. I added a test to NSURLSessionTests.m:

- (void)test_Redirect
{
    if ([NSURLSession class])
    {
        NSURLSession *session = [NSURLSession sharedSession];

        NSDictionary* json = @{@"Success": @"Yes"};
        [self _test_redirect_NSURLSession:session jsonForStub:json completion:^(NSError *errorResponse, NSHTTPURLResponse *redirectResponse, id jsonResponse) {
            XCTAssertNil(errorResponse, @"Unexpected error");
            XCTAssertNil(redirectResponse, @"Unexpected redirect response received");
            XCTAssertEqualObjects(jsonResponse, json, @"Unexpected data received");
        }];
    }
    else
    {
        NSLog(@"/!\\ Test skipped because the NSURLSession class is not available on this OS version. Run the tests a target with a more recent OS.\n");
    }
}

After inserting the sleep the data task completion handler starts receiving the redirect response instead of the response after the redirect.

I can fix it by not calling didReceiveResponse for the redirect (301 code) but based on #175 this does not seem to be a valid solution

I will investigate further but wanted to create a ticket in case anyone already have a clue about the root cause.

[AliSoftware]

🤔 Interesting, thanks for the report. No guarantee I'll have much time to look at it soon, but will keep an eye on it and try to check it some time 👍

[AliSoftware]

Hey @nevil did you manage to investigate this issue further and understand what was causing it?

[nevil]

@AliSoftware Sorry. I didn't have the time yet to find the real cause. I have temporarily changed the requestTime to 0.2 and responseTime to 0.5 (arbitrarily selected) and it prevents the issue from happening for me.

This does slow down my unit test execution a bit (though not more than 10 seconds :-) ) so I hope to have time to continue investigating soon.

[AliSoftware]

Hey @nevil !

Any chance you'll have time to reopen the investigation anytime soon, or maybe at least re-test with a more recent iOS/SDK/Xcode to see if we can close this?

[morrowa]

I've encountered the same issue. Here's a commit adding a unit test that semi-reliably reproduces the issue. I've been able to reproduce the issue in the iOS 11 simulator and directly in macOS. I'm using Xcode 9.3 on macOS 10.13.4. You might have to run the new unit tests several times to see a failure.

When the failure happens, the 302 redirect is not followed. There's no response object, but there's also no error object. The body of the 302 response is returned.

[nevil]

I don't have a full investigation for this issue but It seemed to me like the timing of the call of the completionHandler from URLSession:task:willPerformHTTPRedirection:completionHandler: matters.

For example, if I take your test, add a delegate, and implement it like below then the redirect test never fails. If I shorten or remove the usleep() then testRedirection1000Times fails almost every time.

- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest * _Nullable))completionHandler {
    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
        usleep(1000);
        completionHandler(request);
    });
}

[morrowa]

I've been able to reproduce the issue without any OHHTTPStubs code at all. I suspect there's a race condition in Apple's implementation of NSURLProtocol, NSURLProtocolClient, and/or NSURLSession.

In the attached project (RedirectBugDemo.zip), StubbingProtocol is an extremely pared-down version of the concept of OHHTTPStubs. It intercepts certain HTTP requests and synchronously returns a response. (Although OHHTTPStubs is not synchronous, I've tested modifications to make it synchronous. The change doesn't fix this bug.)

The unit tests run a simple NSURLSession and expect it to follow the 302 redirect. In the first test, there is no NSURLSession delegate, so all redirects are followed immediately. In the second test, the NSURLSessionTaskDelegate calls usleep(500) before instructing the session to follow redirects.

The test without the delay fails on average once per 100,000 runs. The test with the delay has not failed in over 1,000,000 runs.

I've submitted a Radar and cloned it to OpenRadar as well.

[AliSoftware]

Thanks a lot @morrowa for all the investigative work, that's very interesting!