RestCore::JsonResponse::ParseError on non-success http status code

[wvengen]

With the following middleware:

  use RC::ErrorHandler  , lambda {|env| RuntimeError.new(env[RC::RESPONSE_BODY]['message']) }
  use RC::ErrorDetector , lambda {|env| env[RC::RESPONSE_BODY]['status'] != 'ok'}
  use RC::ErrorDetectorHttp
  use RC::JsonResponse  , true

and a request that returns a non-success http status code returning an html error page, JsonResponse can't parse the response.

Moving the JsonResponse middleware to before the ErrorHandler (not just ErrorDetectorHttp) solves this problem, but then the ErrorDetector and ErrorHandler can't be used because at that moment the body hasn't been parsed yet.

Any idea how to solve this nicely?

(Somewhat similar to #8.)

[wvengen]

Current universal.rb's order is the above.

[wvengen]

Ideally I'd have the JsonResponse middleware just before ErrorDetectorHttp, so that http errors are handled without parsing, but api errors with a 200 status code handled by ErrorDetector. This does give the JsonResponse parse error though.

[godfat]

Frankly I am seeing this all the time, and personally I blame the API providers for this... I think twitter, and stackexchange and so on so forth did this. Years ago facebook did this, too, but now they fixed it.

For now, I just ignore it.

Of course this is not a good way to do it, and the fact is, there are too many API providers did this. Since you brought it up, I guess we could try to work out some way to fix it...

The solution you mentioned:

Ideally I'd have the JsonResponse middleware just before ErrorDetectorHttp, so that http errors are handled without parsing, but api errors with a 200 status code handled by ErrorDetector. This does give the JsonResponse parse error though.

Might not work for all the API, because some of them would use HTTP status code for API errors, not only server errors. The challenge here is that not all API providers did the same thing, and there's no easy way to tell if they would respond with a properly formatted data, in a general way for all API providers.

So, maybe a general workaround would be not raising an error upon parsing failures. But what should we do from there then? That might be the other question...

[wvengen]

Hmm it's indeed a bit complicated. This is not a very important feature for myself, but I though it would be nice to be able to show meaningful error messages.

After giving it some thought, I was thinking that perhaps JsonResponse could accept a block to specify in which case it needs to be processed. Then the client writer could tell when json needs to be parsed, and when not.

(I guess the ErrorHandler function would then test if the body is a string, and show something meaningful, and if it's not a string it's json and would return the json response. Something like that.)

[godfat]

That could be an option, and I'll think a bit on how this would impact all the clients.

And yes, as you could see I just did something like that in ErrorHandler for StackExchange: https://github.com/godfat/rest-more/blob/rest-more-3.3.3/lib/rest-core/client/stackexchange.rb#L16-L23 I really don't understand why can't we keep consistent... (The problem is that for requesting a access token, upon success, it would return a query string; while upon failure, it would return a JSON. Alas, I could only try to detect it and write a bit comments before I forgot why I did that in the first place.)