Pass data in error object for failed responses (2.0 regression) #1390

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
8 participants
@gfiumara
Contributor

gfiumara commented Sep 29, 2013

In AFNetworking 1.x, the NSError parameter of the failure block of AFHTTPClient convenience methods (getPath:, etc.) would report the body of the failed response in its userInfo property's NSLocalizedRecoverySuggestionErrorKey key. It was set in AFHTTPRequestOperation -error:. This no longer happens in 2.0's AFHTTPSessionManager convenience method failure callbacks (ultimately, it's missing from NSURLResponseSerialization -validateResponse:data:error:).

If this is by design, the Migration Guide should be updated to reflect that failure block NSError handling code will need to be overhauled.

Note that in 1.x, the body was reported as a string, but as far as I can tell, the serializer does not have access to a string version of the body. If the string cannot be reported back, then there could be a crash when developers interact with the contents of NSLocalizedRecoverySuggestionErrorKey.

Possible regression of #83?

@tspecht

This comment has been minimized.

Show comment
Hide comment

tspecht commented Oct 10, 2013

+1

@mattt

This comment has been minimized.

Show comment
Hide comment
@mattt

mattt Oct 11, 2013

Contributor

According to the NSError documentation:

NSLocalizedRecoverySuggestionErrorKey
The corresponding value is a string containing the localized recovery suggestion for the error.
This string is suitable for displaying as the secondary message in an alert panel.

As such, it seems inappropriate to pass a data value for NSLocalizedRecoverySuggestionErrorKey, as proposed.

5bcac2f adds a responseObject property to AFHTTPRequestOperation. For sessions, the recommended approach is to create a custom response serializer that translates response data in failing cases into NSError objects as appropriate.

Contributor

mattt commented Oct 11, 2013

According to the NSError documentation:

NSLocalizedRecoverySuggestionErrorKey
The corresponding value is a string containing the localized recovery suggestion for the error.
This string is suitable for displaying as the secondary message in an alert panel.

As such, it seems inappropriate to pass a data value for NSLocalizedRecoverySuggestionErrorKey, as proposed.

5bcac2f adds a responseObject property to AFHTTPRequestOperation. For sessions, the recommended approach is to create a custom response serializer that translates response data in failing cases into NSError objects as appropriate.

@mattt mattt closed this Oct 11, 2013

@Zyphrax

This comment has been minimized.

Show comment
Hide comment
@Zyphrax

Zyphrax Jan 26, 2014

Contributor

Our app relies quite heavily on JSON API calls to our Ruby platform. If the request fails (entity validation error or object not found) it is difficult in AFNetworking 2.0 to get to the responseObject that in that case contains a JSON object describing the validation errors and other useful information.

Would you please consider adding responseObject to the failure completion handler?
Or perhaps store it in the userInfo of the NSError object under a custom key?

Creating a custom response serializer for this seems a bit overkill.

Contributor

Zyphrax commented Jan 26, 2014

Our app relies quite heavily on JSON API calls to our Ruby platform. If the request fails (entity validation error or object not found) it is difficult in AFNetworking 2.0 to get to the responseObject that in that case contains a JSON object describing the validation errors and other useful information.

Would you please consider adding responseObject to the failure completion handler?
Or perhaps store it in the userInfo of the NSError object under a custom key?

Creating a custom response serializer for this seems a bit overkill.

@ahknight

This comment has been minimized.

Show comment
Hide comment
@ahknight

ahknight May 27, 2014

@mattt I've loved AFNetworking for years, but I have to say that's an ugly solution for Session. We really need something that lets us easily at JSON error responses in the default kit. This is how APIs are used in the wild these days and not having support for it makes this kit just that much less beautiful. :(

If it's code help, I can step in. Let's just decide how it should look (a new method can work) and then fun can be had.

@mattt I've loved AFNetworking for years, but I have to say that's an ugly solution for Session. We really need something that lets us easily at JSON error responses in the default kit. This is how APIs are used in the wild these days and not having support for it makes this kit just that much less beautiful. :(

If it's code help, I can step in. Let's just decide how it should look (a new method can work) and then fun can be had.

@Dschee

This comment has been minimized.

Show comment
Hide comment
@cwagdev

This comment has been minimized.

Show comment
Hide comment
@ahknight

This comment has been minimized.

Show comment
Hide comment
@ahknight

ahknight Aug 13, 2014

FWIW, I wound up creating a custom session manager with methods similar to GET:params:completion: and following NSURLSession's completion block format. This lets me trap all calls, handle errors the way my API returns them (and hand them to the caller in a proper NSError along with the responseObject), and then use the default response serializer for the responses.

I imagine there's room for an error handling block in here so that this can be genericised. I'll look at that and maybe post a Pod someday with that.

I also had luck with setting the acceptableStatusCodes to 0-999 but that felt even more hacky to me and moved the error checking work to the wrong place.

At any rate, this is probably a dead bug and is what it is for now. But when 3.0 (or even 2.5) comes, I hope this is considered as a "real world" issue for a lot of us.

FWIW, I wound up creating a custom session manager with methods similar to GET:params:completion: and following NSURLSession's completion block format. This lets me trap all calls, handle errors the way my API returns them (and hand them to the caller in a proper NSError along with the responseObject), and then use the default response serializer for the responses.

I imagine there's room for an error handling block in here so that this can be genericised. I'll look at that and maybe post a Pod someday with that.

I also had luck with setting the acceptableStatusCodes to 0-999 but that felt even more hacky to me and moved the error checking work to the wrong place.

At any rate, this is probably a dead bug and is what it is for now. But when 3.0 (or even 2.5) comes, I hope this is considered as a "real world" issue for a lot of us.

@cwagdev

This comment has been minimized.

Show comment
Hide comment
@cwagdev

cwagdev Aug 13, 2014

I decided just to not use the session manager variant for this case. In another case I ended up adding adding to the acceptableStatusCodes like you did, totally feels hacky. I need to look at https://github.com/Alamofire/Alamofire to see if it handles this differently.

EDIT: At a quick glance it appears Alamofire does resolve this issue by not separating out failure from success for the completion block, which I ultimately like better anyhow. :)

cwagdev commented Aug 13, 2014

I decided just to not use the session manager variant for this case. In another case I ended up adding adding to the acceptableStatusCodes like you did, totally feels hacky. I need to look at https://github.com/Alamofire/Alamofire to see if it handles this differently.

EDIT: At a quick glance it appears Alamofire does resolve this issue by not separating out failure from success for the completion block, which I ultimately like better anyhow. :)

@chrishulbert

This comment has been minimized.

Show comment
Hide comment
@chrishulbert

chrishulbert Sep 9, 2014

Every second project I work on, i come up against this limitation, it's a pity. So I've come up with a slightly hacky solution which works well. Basically, it swizzles out dataTaskWithRequest:completionHandler:, and wraps the handler with a shim which puts the response into the error's userInfo dictionary.

https://gist.github.com/chrishulbert/35ecbec4b37d36b0d608

Using it is quite straightforwards:

[mySessionManager POST:@"some-api-call" parameters:params success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) {
    ...
} failure:^(NSURLSessionDataTask *task, NSError *error) {
    id responseObject = error.userInfo[kErrorResponseObjectKey];
    ...
}];

Every second project I work on, i come up against this limitation, it's a pity. So I've come up with a slightly hacky solution which works well. Basically, it swizzles out dataTaskWithRequest:completionHandler:, and wraps the handler with a shim which puts the response into the error's userInfo dictionary.

https://gist.github.com/chrishulbert/35ecbec4b37d36b0d608

Using it is quite straightforwards:

[mySessionManager POST:@"some-api-call" parameters:params success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) {
    ...
} failure:^(NSURLSessionDataTask *task, NSError *error) {
    id responseObject = error.userInfo[kErrorResponseObjectKey];
    ...
}];
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment