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

invalid json response body at .. for empty response (as in OPTIONS) request #432

Closed
marcin-wosinek opened this issue Jan 29, 2018 · 4 comments

Comments

@marcin-wosinek
Copy link
Collaborator

Same issue as in #409 happens if the response body is empty - an expected behavior, for OPTIONS request

@H1Gdev
Copy link
Collaborator

H1Gdev commented Jan 30, 2018

@marcin-wosinek
@vlucas

https://stackoverflow.com/questions/29784398/should-content-type-header-be-present-when-the-message-body-is-empty

So, I do not check body length.(error will be occurred.)

What do you think about this ?

@marcin-wosinek
Copy link
Collaborator Author

To what I've seen in the specs, the content-type is not forbidden for OPTIONS request.

What I thought about as a solution, after proposing #433 is something like:

try {
  body = JSON.parse(response.body);
} catch (error) {
  body = error;
}

In case of my tests, I was not interested in body content - so uncaught exception was a problem. Returning error instead of content, should be clear enough for anybody interested in the body itself.

@H1Gdev
Copy link
Collaborator

H1Gdev commented Feb 1, 2018

OPTIONS request is right.

It is also possible to check each HTTP method.

        // Auto-parse JSON
        if (response.headers.has('Content-Type') && response.headers.get('Content-Type').includes('json') && response.status !== 204 && (this._request.method.toUpperCase() === 'OPTIONS' && responseBody.length > 0)) {
          try {
            response._json = JSON.parse(responseBody);
          } catch(e) {
            return Promise.reject(new TypeError(`Invalid json response body: '${responseBody}' at ${this._request.url} reason: '${e.message}'`));
          }
        }

@koooge
Copy link
Collaborator

koooge commented May 24, 2021

Let me close this issue because it would be solved by #521 .

@koooge koooge closed this as completed May 24, 2021
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

3 participants