Interacting with non-standard mime types lead to confusing results #4

Closed
eric opened this Issue Sep 2, 2011 · 5 comments

Comments

Projects
None yet
2 participants

eric commented Sep 2, 2011

When interacting with the Tender API it returns a Content-Type of application/vnd.tender-v1+json which causes faraday-stack to return the body as a string.

Unfortunately there isn't any indication for the user why it was the body wasn't parsed or what exactly to do to fix it.

I wanted to bring this up more as a discussion of the over-arching problem of automatic parsing based on mime type and how that is for a new user of the library more than just a quick fix for this specific case.

A reasonable amount of time was spent by a coworker trying to figure out why things weren't parsed before he found out he could pass :content_type to the JSON middleware to get it to parse this content-type.

I don't have any specific actions I would advocate at the moment, but the idea does come to mind that when making a request, the user generally knows what format he is expecting the results to be in. Having a single stack that can parse XML and JSON and whatever else may not be as obvious to a new user as having separate stacks that are intended to parse a specific type.

/cc @technoweenie

Owner

mislav commented Sep 2, 2011

I feel your coworker's pain. But the JSON middleware, when initialized without the :content_type option, should parse every response as JSON. It only does content-type checking when a string or regexp is explicitly given.

However, I'm aware that the default stack does initialize JSON middleware with :content_type => /(application|text)\/json/. Maybe this default regexp should be make more permissible, like the XML regex currently is:

use ResponseXML,  :content_type => /[+\/]xml$/

This would hopefully avoid scenarios such as you've described in the future when people are using the default stack.

eric commented Sep 2, 2011

Yes, I agree about the default for the JSON middleware. I was worried about the pitfalls of the idea of the default stack.

I've seen people server JSON as text/plain or application/javascript (so it is displayed instead of downloaded by browsers) or application/octet-stream (due to server misconfiguration).

Maybe it would make sense to have a JSON and an XML stack defined and promote those in the README above an überstack that does both...

I'm sure there are cases for a stack that can parse absolutely everything (though I'm having trouble coming up with one off the top of my head), but I bet in general people are going to be pretty sure they'll be consuming JSON or XML before they start...

Owner

mislav commented Sep 2, 2011

I agree about usually knowing what type of API you're communicating with: it's either JSON or XML, not a mixture.

However as a developer I do not wish to use different Ruby classes (e.g. FaradayStack::JSON, FaradayStack::XML) depending on the API I'm consuming. I don't want to think about this. The whole idea of the uber stack was "one stack to rule them all". And content-types make this possible for the Web.

I don't want to declare that we're surrendering just because of some server misconfigurations. Quite the contrary, I want the developers to suffer if they're consuming a crappy API. They should suffer, they should configure their own stack or write middleware that conditionally fixes the content-type (I've done so myself). Or they can start off with the default stack, remove the XML middleware and configure JSON middleware without content-type matching (or the other way around, if they plan to consume XML).

Your coworker didn't need to suffer, however, because he/she didn't interface with some crappy API. I approve of advanced mime-types such as "application/vnd.tender-v1+json". I want the default stack to be a little more permissive about what JSON is, but that's as far as I want to go in this issue.

Documentation should make this clear and give advice to people what to do when they encounter bad APIs.

eric commented Sep 3, 2011

The problem I see with this approach is getting back a String does not make it clear to the casual user that this is actually an error condition (from what they are expecting).

I've seen the general next step that people will be doing is just passing the result into JSON.parse() instead of investigating what the underlying issue is.

So, while it is a noble goal of trying to encouraging users to complain about crappy APIs, what I've actually seen this do is discourage people from properly using the tools that Faraday gives you.

If, for instance, you threw an exception if none of the parser had been fired, that would do a better job of helping you achieve your goal of punishing bad APIs.

Owner

mislav commented Sep 4, 2011

On Sep 4, 2011, at 12:58 AM, eric wrote:

If, for instance, you threw an exception if none of the parser had been fired, that would do a better job of helping you achieve your goal of punishing bad APIs.

That's a very interesting idea. There could be a middleware that does this.

mislav closed this in 4fbb7aa Jan 10, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment