Expose server-side error description for non-fulfilled requests

[t0rst]

The extra information that server-side often sends back when a request could not be fulfilled can save a lot of time. (Read: I wasted a lot of time and inconvenienced some server-side folks.)

Normally this information has already been received by STHTTPRequest when it creates an NSError, but is not used, so I added code to include it in a readable way and give the option for some heavier debugging; this is via new function -errorDescribingRequestNonfulfillment which is used by -URLSession:task:didCompleteWithError: and -startSynchronousWithError:.

The simple use-case that I catered for is for REST servers that return a dictionary in JSON with an error key, and I just append the value description to the status code description, hence, you now see something like "HTTP Status 401: Unauthorized (OBP-20004: Invalid login credentials. Check username/password.)" - enormously valuable for the less trivial cases. I just did the simplest of checks, so there may be small variations that get missed. And the logic for local bool serverSideDescriptionPossible is not clever enough to eliminate the case where quite some data was normally received before an error caused the request to fail; however, better to try all than falsely eliminate.

Also, if you know of other common use-cases, please add, or make extensible. The fallback for the other cases is to include the header and data in the error userInfo, but as this can get a bit verbose when logged to the console, I made this conditional on a new defaults key STHTTPRequestIncludeHeaderAndDataInError; maybe the default should be on and the key should work with opposite sense? (Its useful to see this information, but for trivial cases, it looks like spam, so maybe its best as an opt-in.)

Hope its to your liking. If not, please adjust.

Cheers, Torsten

[t0rst]

...also forgot to add: if you accept the pull request, please could you also bump the version and add a new tag so that Carthage and CocoaPods can pull it. Cheers, Torsten

[nst]

So, if I get your PR correctly, it basically adds "headers" and "data" into NSError userInfo when HTTP status is >= 400.

I totally agree that STHTTPRequest lacks access to headers and data in case of error.

However, I'm not very comfortable with the fact the code assumes that the error format is

{ "error" = "my error" }

since it's not defined in any standard. I regulary meet APIs like:

{ "error_string" = "my error", "error_code" = 123 }

or like:

{ "errors" = ["bad id", "unknown stuff"] }

So, maybe we could simply update the errorBlock to return headers and data, so that we could write:

r.errorBlock = ^(NSDictionary *headers, NSData *data, NSError *error) {
    // ...
};

since completion block is pretty similar:

r.completionDataBlock = ^(NSDictionary *headers, NSData *data) {
    // ...
};

This alternative solution would allow any user to extract the errors the way he wants.

What do you think?

[t0rst]

So, if I get your PR correctly, it basically adds "headers" and "data" 
into NSError userInfo when HTTP status is >= 400.

...that's the fallback option (and on reflection I would have it enabled by default, with an option to suppress it), but my main preference was to identify the server's message and get it into the error description if possible.

I've often seen REST servers return {"error" : "some added-value description"}, which is not a standard, but just a fairly common occurrence, as are other formats I would expect. One server I am working with is sending back error messages in HTML, which is even more tricky to extract. Rather than attempting to be comprehensive, I was just trying to catch one of the cases, which while imperfect, would save a lot of hassle from unnoticed server-side help.

I'm not very keen on adding header and data to the error block because it would break a lot of usage, and in any case, the responseData and responseHeaders can be accessed via the original request instance captured in the error handling block. However, it took me the lost time of server help going under my radar, wasting the time of server side guys, then a bit of investigation to find out how I could access this server help, that made me think other people shouldn't also have to pay this high price.

So...

• quickest solution would be to leave the code unchanged, but somewhere in the header, describe the technique of accessing and inspecting headers and data in the error handler block and why, or
• have imperfect handling of common cases ("error", "error_string", ...), together with an invitation for people to notify if there are more common cases to be added, or

• add an extensible, configurable mechanism to extract server-side error descriptions, e.g.

  typedef NSString*(^serverErrorDescriberBlock_t)(NSDictionary *headers, NSData *data)
  @property (copy) serverErrorDescriberBlock_t serverErrorDescriberBlock;

  ...looks a bit klunky. Alternatively, expose -errorDescribingRequestNonfulfillment so it can be overriden. Needs more thinking about. A generalised solution could easily cause ugly code bloat. The imperfect solutions are pragmatic - if they cover 60%, that's a real benefit, and the other cases are caught by the fallback.

From your comments, I think the solution ideas need time to evolve and mature a bit. Maybe you could try it your side and see what direction comes of it.

I can continue to use my own solution in a bottleneck error handler in the library I'm working on, but it would be good to eventually share something with other users.

[nst]

I agree that adding more info into NSError may save debugging time, but I would prefer the solution to cover all the cases. So, I think I will merge your PR and change it so that, in case of HTTP error status, the userInfo is added a "JSON" key with the JSON contents inside, or a "text" key with the text as a fallback. Let me know what you think.

[t0rst]

I think that would be fine. Visibility is the main thing, as in, once users can see that there is extra info to look at while debugging an error, they can proceed from there in whatever way suits them best.

[t0rst]

Hi Nicolas, any chance of a new podspec version to expose this (and your latest) to pod users? Cheers & Bests, Torsten

[nst]

done