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

GraphiQL expects 200 status for errors #88

Closed
sporto opened this issue Feb 4, 2016 · 8 comments
Closed

GraphiQL expects 200 status for errors #88

sporto opened this issue Feb 4, 2016 · 8 comments

Comments

@sporto
Copy link

sporto commented Feb 4, 2016

In order to render an error on the right panel, GraphiQL expects the response to have a 200 status. In case of a bad query, the server needs to respond with 200 in order the see the error in the UI.

We had a case where bad queries were send to the server e.g. asking for something that doesn't exists. Looking at the server logs wouldn't show the issue as everything was 200. So we changed our graphql server to return 422 on bad queries.

But now GraphiQL doesn't show anything on the right, as it expects 200.

So I am not sure about this, is this a graphql spec issue or should GraphiQL handle some errors and try to render the response?

@asiandrummer
Copy link
Contributor

Hi @sporto - sorry it took a while to get to this. I'm not sure if I understand the problem; when you say "bad queries", do you mean a invalid query with a correct syntax (e.g. a field that isn't defined is queried), or a query that returns null because the result was empty?

GraphiQL does not know about the network status/http status code/etc - instead it handles errors if the defined fetcher rejects the query and/or an error is thrown during the execution (defined here). So it is possible to render the error logs without a specific status code - express-graphql implementation is an example of this.

@sporto
Copy link
Author

sporto commented Mar 6, 2016

hi @asiandrummer using graphiql 0.5 this is the behaviour I see.

Consider a query where one attribute doesn't exist e.g. unknown in the screenshot below

qraphiql-200

In this case I have setup my graphql server to return a 200 with the errors. GraphiQL shows the error on the right panel.

But returned 200 when an error happens is not what we think should happen.

qraphiql-422

So in the screenshot above I have set up the server to return 422 with the error. GraphiQL just shows [object Object] on the right.

The expected behaviour would be to have GraphiQL show the error on the right if there is an error on the response body, even if the status is 422.

@asiandrummer
Copy link
Contributor

I see. Just checking in case - is your server throwing an error whenever it comes across any responses other than 200 status? The query execution workflow is set up so that the response (in your case maybe a JSON.parsed object?) gets passed into the callback defined in _runEditorQuery function in GraphiQL component, and if there's any error thrown from the fetcher, _fetchQuery will catch the error and try to find a stack trace, or cast the error into String format, which is what I think might be happening with your 422 status code:

  _fetchQuery(query, variables, cb) {
    this.props.fetcher({ query, variables }).then(cb).catch(error => {
      this.setState({
        isWaitingForResponse: false,
        response: error && (error.stack || String(error))
      });
    });
  }

FYI, our example fetcher definition does this from example/index.html:

try {
  return JSON.parse(responseBody);
} catch (error) {
  return responseBody;
}

@sporto
Copy link
Author

sporto commented Mar 7, 2016

@asiandrummer I don't follow your question, can you please rephrase it?
As far as I can tell my server is returning exactly the same (headers and body) but the only thing that is different is the status code.

@asiandrummer
Copy link
Contributor

is your server throwing an error whenever it comes across any responses other than 200 status?

Sorry; what I meant was a "client-side fetcher code", not a "server". I was just curious how your fetcher is defined.

@sporto
Copy link
Author

sporto commented Mar 7, 2016

I see this error:

graphiql-error

I see what you mean about the fetcher, I'm using axios, so you are saying that the error is most likely happening there? I will investigate this.

This is how it is defined at the moment:

import 'graphiql/graphiql.css'

import React from 'react'
import ReactDOM from 'react-dom'
import GraphiQL from 'graphiql'
import axios from 'axios'

function graphQLFetcher(graphQLParams) {
  return axios({
    url: '/graphql',
    method: 'POST',
    data: graphQLParams,
  })
}

ReactDOM.render(<GraphiQL fetcher={graphQLFetcher} />, document.body)

@sporto
Copy link
Author

sporto commented Mar 7, 2016

Ok, that is the problem, thanks so much for helping debug this. Closing this issue.

@sporto sporto closed this as completed Mar 7, 2016
@asiandrummer
Copy link
Contributor

:D glad you were able to resolve the issue - awesome!

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