Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GET requests to API endpoints can't have a body. Find something better than the workaround. #576

Closed
LefterisJP opened this issue Dec 22, 2019 · 3 comments

Comments

@LefterisJP
Copy link
Member

LefterisJP commented Dec 22, 2019

Problem Definition

Since the front-end uses AXIOS the GET requests can't have a body and as such can't get parameters.

The work-around we went with so far is to allow query params for the GET endpoints that need them. This is suboptimal due to the incosistency between those endpoints and all the rest which accept json params on the body.

GET Requests should accept body parameters. It is true that in the past a GET request with a body was undefined, but that has changed. Reference here

Specifically:

The RFC2616 referenced as "HTTP/1.1 spec" is now obsolete. In 2014 it was replaced by RFCs 7230-7237. Quote "the message-body SHOULD be ignored when handling the request" has been deleted. It's now just "Request message framing is independent of method semantics, even if the method doesn't define any use for a message body" The 2nd quote "The GET method means retrieve whatever information ... is identified by the Request-URI" was deleted.

Task

Figure out if any of the following, or something else, is better than the work-around of using query parameters.

  • Wait until the javascript http api implementation of browsers gets up to date with the new RFC allowing body to GET requests.
  • Modify the endpoints in a way so that the GET request parameters go away. For example turn them into part of the URL. For example /api/(version)/exchanges/trades/from/0/to/9999999 to filter the timestamp
  • Turn all those requests to PUT or to POST.
@kelsos
Copy link
Member

kelsos commented Dec 22, 2019

@LefterisJP I am adding this as a clarification (since we already discussed this).

Unfortunately, this doesn't seem to be an axios problem. The problem seems to lie on the http client implementation in the browser javascript engine.

According to the documentation and the spec XMLHttpRequest ignores the body of the request in case the method is GET.
If you perform a request in Chrome/Electron with XMLHttpRequest and you try to put a json body in the send method this just gets ignored.

Using fetch which is the modern replacement for XMLHtppRequest also seems to fail in Chrome/Electron.

The following message is the error you get when you attempt to use a body in GET:

TypeError: Failed to execute 'fetch' on 'Window': Request with GET/HEAD method cannot have body.
    at VueComponent.mounted (App.vue:130)
    at invokeWithErrorHandling (vue.runtime.esm.js:1854)
    at callHook (vue.runtime.esm.js:4219)
    at Object.insert (vue.runtime.esm.js:3139)
    at invokeInsertHook (vue.runtime.esm.js:6346)
    at Vue.patch [as _patch_] (vue.runtime.esm.js:6565)
    at Vue._update (vue.runtime.esm.js:3945)
    at Vue.updateComponent (vue.runtime.esm.js:4066)
    at Watcher.get (vue.runtime.esm.js:4479)
    at new Watcher (vue.runtime.esm.js:4468)

Basically it seems that the browsers are not implementing the latest RFC.

LefterisJP added a commit that referenced this issue Dec 26, 2019
Workaround for the problem described in
#576

Which was that many browser http implementations remove a request's
body if it's a GET request and as such JSON data bodies don't work for
GET requests.
@LefterisJP
Copy link
Member Author

Or ... we can just switch all GET endpoints that are supposed to have a body to POST ... as we just did for one: #2250

@LefterisJP
Copy link
Member Author

Closing this as there is no way around this other than changing them to POST

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants