Timeout property for AFHTTPClient

[vocaro]

This commit adds a timeoutInterval property to AFHTTPClient. The typical use case is for servers that are sporadically slow to respond. For example, I send requests to a server that's usually fast but occasionally takes 45 seconds or longer to process a request. I don't want to keep the user hanging, thinking that the app is busy when in fact it's just stalled, waiting for a server that is taking forever to respond.

I know that this issue has been discussed before, such as pull request #133. That request was rejected for two reasons:

1. It is not clear whether iOS honors the timeout

2. A timeout can be set with AFHTTPRequestOperation instead of AFHTTPClient.

For (1), I can say that I tested the timeout behavior (iOS 6) and it works just as expected. When a GET request takes longer than the specified timeout, AFHTTPClient fails with an error: "The request timed out."

As for (2), I would argue that the suggested workaround is unintuitive. It's not at all clear -- at least, not to myself nor the other two users who supported the earlier pull request -- that a timeout on AFHTTPClient can be set via AFHTTPRequestOperation. Furthermore, having to rewrite code to use AFHTTPRequestOperation is an unnecessary inconvenience for this seemingly common use case.

Thanks for reconsidering this issue.

[mattt]

Thanks for creating this patch, @vocaro.

Following up from my previous response to a similar pull request a year ago, my advice now would be to override -requestWithMethod:path:parameters, setting the timeout from the result of super.

I understand the appeal of accessors for NSURLRequest properties like timeout interval, HTTP pipelining, handling cookies, cache policy, but my concern is that by adding all of these helpers would 1) pollute and overburden the API, and 2) create confusion for users not knowing how these work (rather than understanding that they're part of NSURLRequest, and that all requests that are created go through -requestWithMethod:path:parameters:).

Since AFHTTPClient is designed to be subclassed, the pattern of overriding key methods makes a lot of sense to me.

With timeout interval specifically, it's not clear when changing the timeout policy is actually the correct move: current network latency varies wildly depending on the kind of connection. Apple's policy, though heavy-handed at times, has been that we don't really know what timeout policies should be, and that our tendency to make it unreasonably short ends up being detrimental to users in dire circumstances.

This is all to say that I need some time to think through this. Happy to hear any other opinions on the matter.

[vocaro]

I'm okay with rejecting this pull request for reasons of API simplicity. However, my concern is that it's not clear how to get around the problem. In the previous pull request #133, you suggested using AFHTTPRequestOperation as a means of setting the timeout. In this pull request, you're suggesting subclassing AFHTTPClient. Which is the appropriate solution?

[mattt]

@vocaro Either works equally well.

If you already have a subclass, just override the method, and everything happens automatically. For one-off instances of the client, use the aforementioned approach of manually creating and modifying the request, and passing that in to create and enqueue a new operation.

[vocaro]

I think a solution that would satisfy us both is some better documentation on how to handle this problem. I'll work on a pull request for that. In the meantime, could you please confirm that the correct way to rewrite this:

AFHTTPClient *client = [[AFHTTPClient alloc] initWithBaseURL:[NSURL URLWithString:kBaseURL]];
[client registerHTTPOperationClass:[AFJSONRequestOperation class]];
[client setDefaultHeader:@"Accept" value:@"application/json"];
[client setAuthorizationHeaderWithUsername:self.username password:self.password];
[client getPath:@"customers"
     parameters:nil
        success:^(AFHTTPRequestOperation *operation, id response) {
            // Handle success...
        }
        failure:^(AFHTTPRequestOperation *operation, NSError *error) {
            // Handle failure...
        }];

is this? (Note the addition of setTimeoutInterval).

AFHTTPClient *client = [[AFHTTPClient alloc] initWithBaseURL:[NSURL URLWithString:kBaseURL]];
[client registerHTTPOperationClass:[AFJSONRequestOperation class]];
[client setDefaultHeader:@"Accept" value:@"application/json"];
[client setAuthorizationHeaderWithUsername:self.username password:self.password];
NSMutableURLRequest *request = [client requestWithMethod:@"GET" path:@"customers" parameters:nil];
[request setTimeoutInterval:8];
AFHTTPRequestOperation *operation = [client HTTPRequestOperationWithRequest:request
                                                                    success:^(AFHTTPRequestOperation *operation, id response) {
                                                                        // Handle success...
                                                                    }
                                                                    failure:^(AFHTTPRequestOperation *operation, NSError *error) {
                                                                        // Handle failure...
}];
[client enqueueHTTPRequestOperation:operation];

Thanks!

[mattt]

@vocaro Sorry for the belated response. Yes, that is the correct way to do that without a custom AFHTTPClient subclass.

[vocaro]

Okay, thanks. Please see related issue #632.