shurcooL/graphql

GraphQL Errors not be returned due to bad unmarshal of Response Data

mwilli31 opened this issue · 5 comments

Minor bug where the response from the server is not compliant with the Query Struct and provides valid graphql Errors, but returns too early so error supplied is from Unmarshal and not the actual server Error.

Fix could be in graphql.go (line 85) as such:

if err != nil {
	if err.Error() == "unexpected end of JSON input" {
		//If you return here, then you will not return the actual Errors supplied by the server
		fmt.Println("There was a problem Unmarshalling the response into the provided Model Struct")
	} else {
		return err
	}
}

From:

if err != nil {
	return err
}

Hi, thanks for reporting.

Minor bug where the response from the server is not compliant with the Query Struct and provides valid graphql Errors

I'd like to better understand when this can happen (and why):

  • Can it happen with a valid GraphQL server, or only if the GraphQL server sends an response that isn't correct according to the GraphQL spec?

  • Do you know what are the conditions for this to happen?

  • How can I reproduce it?

Thanks.

If an error was encountered before execution begins, the data entry should not be present in the result.

If an error was encountered during the execution that prevented a valid response, the data entry in the response should be null.

This is happening with a valid GraphQL server and fails while in compliance with the above spec. Essentially, the has to do with the UnmarshalGraphQL function:

func UnmarshalGraphQL(data []byte, v interface{}) error {
	dec := json.NewDecoder(bytes.NewReader(data))
	dec.UseNumber()
	err := (&decoder{tokenizer: dec}).Decode(v)
	if err != nil {
		//ERROR IS THROWN HERE HERE
		return err
	}

To reproduce, send a query that has an error that returns a NULL entity back (like an ID lookup that has no resource found).

Example return from GraphiQL:

{
  "data": {
    "clinicalCodeType": null
  },
  "errors": [
    {
      "message": "Resource Not Found",
      "path": [
        "clinicalCodeType"
      ]
    }
  ]
}

Also, it fails when the query doesn't match a valid query and fails before execution begins on the server (ie. missing required fields).

Example return from Graphiql:

{
  "errors": [
    {
      "message": "Field \"clinicalCodeType\" argument \"id\" of type \"String!\" is required but not provided.",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ]
    }
  ]
}

Tracing it into the UnmarshalGraphQL function, it fails because the "data" element returned by the GraphQL Server is either NULL or non-existent and thus its trying to unmarshal an object that doesn't exist and throws an error.

Thanks a lot for the information, it's very helpful.

I will work on a fix.

I suspect the right solution is that UnmarshalGraphQL should not return an error on that input. It should populate partial results (if any) and return nil error.

Just checking, but are you using the latest version of graphql? I remember fixing an issue related to this somewhat recently (edit: see 8d3d310).

@mwilli31 Can you share the GraphQL query and/or the Go code to execute that query that you used? I can't reproduce the problem yet.

Edit: Okay, I managed to reproduce when "data" field is completely absent from the response.

@mwilli31 I've sent PR #23 which should fix this issue. Could you give it a try and confirm it fixes it for you?