Matt Basta [:basta] Reporter
	Description • 11 years ago

Apr  4 22:18:44 dev1.addons.phx1.mozilla.com: [][] django.request.tastypie:ERROR /api/v1/account/feedback/ Internal Server Error: /api/v1/account/feedback/ :/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py:245#012Traceback (most recent call last):#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py", line 192, in wrapper#012    response = callback(request, *args, **kwargs)#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py", line 397, in dispatch_list#012    return self.dispatch('list', request, **kwargs)#012  File "/data/www/addons-dev.allizom.org/zamboni/mkt/api/base.py", line 46, in dispatch#012    .dispatch(request_type, request, **kwargs))#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py", line 427, in dispatch#012    response = method(request, **kwargs)#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py", line 1161, in post_list#012    deserialized = self.deserialize(request, request.raw_post_data, format=request.META.get('CONTENT_TYPE', 'application/json'))#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/resources.py", line 346, in deserialize#012    deserialized = self._meta.serializer.deserialize(data, format=request.META.get('CONTENT_TYPE', 'application/json'))#012  File "/data/www/addons-dev.allizom.org/venv/lib/python2.6/site-packages/tastypie/serializers.py", line 191, in deserialize#012    raise UnsupportedFormat("The format indicated '%s' had no available deserialization method. Please check your ``formats`` and ``content_types`` on your Serializer." % format)#012UnsupportedFormat: The format indicated 'application/x-www-form-urlencoded' had no available deserialization method. Please check your ``formats`` and ``content_types`` on your Serializer.

It seems jQuery gives a Content-Type of "application/x-www-form-urlencoded" on POSTs. The API should handle this gracefully.

	Chuck Harmston [:chuck]
	Comment 1 • 11 years ago

I'm not sure that I agree that the API should handle that gracefully. You're POSTing JSON in the content body, so you should format that header correctly.

Can you not do this?

$.ajax({
    type: 'POST',
    contentType: "application/json"
});

	Matt Basta [:basta] Reporter
	Comment 2 • 11 years ago

Where are you seeing a JSON POST body? We're using jQuery's default form serialization to post values, which uses form encoding.

	Chuck Harmston [:chuck]
	Comment 3 • 11 years ago

There should be a JSON post body:
"A POST, PUT and PATCH accept parameters as a JSON document in the body of the request."

http://zamboni.readthedocs.org/en/latest/topics/api.html#verbs

Additionally:
"All requests should be made with the header: Content-type: application/json"

http://zamboni.readthedocs.org/en/latest/topics/api.html#requests

	Matt Basta [:basta] Reporter
	Comment 4 • 11 years ago

Why can't we just accept form encoding, as the web has done since the dawn of time? We have multiple layers of abstraction over our AJAX, which would make this frustratingly difficult to implement.

	Andy McKay Assignee
	Comment 5 • 11 years ago

We chose to implement using JSON because its simple, everything comes in the body as JSON, everything goes back out as JSON, errors too. This is the same for solitude, elastic search, github api and so on. To me there's a simple simplicity that everything is JSON and JSON out.

If you really want form-encoding there's no real reason we can't do that. I'd like to keep all the docs and so on in JSON. 

We should also check the Accept header to be sure that the client will accept JSON back.

	Matt Basta [:basta] Reporter
	Comment 6 • 11 years ago

Form encoding is the standard for XHR-based communication, and sending anything else requires jumping through hoops. Let's add application/x-www-form-urlencoded, even if it's undocumented.

	Christopher Van Wiemeersch [:cvan]
	Comment 7 • 11 years ago

(In reply to Chuck Harmston [:chuck] from comment #1)
> I'm not sure that I agree that the API should handle that gracefully. You're
> POSTing JSON in the content body, so you should format that header correctly.

Whether we should accept the Content-Type is a debatable question. Can we agree though that we should never be 500'ing?

	Chuck Harmston [:chuck]
	Comment 8 • 11 years ago

Agree that we should never 500, but that's a bug orthogonal to this ticket, I think.

	Christopher Van Wiemeersch [:cvan]
	Comment 9 • 11 years ago

(In reply to Chuck Harmston [:chuck] from comment #8)
> Agree that we should never 500, but that's a bug orthogonal to this ticket,
> I think.

"API returns 500 for feedback and maybe more" - LOL is it?

	Matt Basta [:basta] Reporter
	Comment 10 • 11 years ago

I think if the cause of the tracebacks is unaccepted input, we should fix the cause rather than the symptom.

My biggest argument for adding form encoding is that the default for exactly zero HTTP libraries is JSON. Any consumer of our API would need to write a custom wrapper. You could never simply do a requests.post('...', {}) or $.post('...', {}), you'd have to have some intermediary layer that wraps all API posts.

I've yet to see any endpoints that require non-key-value POST bodies, but I'd imagine we could require JSON POST bodies for those ones. I don't believe there's anything in the consumer APIs that would necessitate that.

	Chuck Harmston [:chuck]
	Comment 11 • 11 years ago

Here you go, cvan: 

https://github.com/mozilla/zamboni/commit/859a1b036ddb3de4003ed4569976795dc53bc60f

Now returns a 400 when an unsupported serialization format is sent.

	Matt Basta [:basta] Reporter
	Comment 12 • 11 years ago

Blocks Fireplace

	Andy McKay Assignee
	Comment 13 • 11 years ago

https://github.com/mozilla/zamboni/commit/c75f23

Hopefully one day we can remove this :(