hasura / go-graphql-client

Package graphql provides a GraphQL client implementation.
MIT License
394 stars 90 forks source link

Unexpected, intermittent (!) deserialization error #139

Open peterldowns opened 3 months ago

peterldowns commented 3 months ago

I'm seeing an intermittent failure right now where sometimes (inconsistently!) a Github Graphql API response fails to decode, resulting in an error like this:

Message: slice doesn't exist in any of 1 places to unmarshal, Locations: []

This error is thrown in the deserialization code whensomeSliceExist == false. I'm curious if it's related to the changes in #102 — a PR that has no description, no tests, and changes something suspicious. If you look at that PR, it makes a change so that when there is a slice, but it's of zero length, someSliceExist == false, whereas before this change, someSliceExist would be set to true. I'm trying to understand why I'm seeing this error intermittently, which is always a red flag that something is horribly wrong, especially considering that the API response is valid JSON and seems to match what I am deserializing into (because it succeeds intermittently!)

An example of the failure happening is when the API responds with:

{
  "data": {
    "search": {
      "pageInfo": {
        "endCursor": "Y3Vyc29yOjE=",
        "hasNextPage": false
      },
      "nodes": [
        {
          "number": 32,
          "id": "I_kwDOJBmYg86J6maD",
          "createdAt": "2024-05-23T21:03:09Z",
          "updatedAt": "2024-05-28T15:42:02Z",
          "state": "OPEN",
          "stateReason": null,
          "closedAt": null,
          "author": {
            "login": "drewAdorno"
          },
          "authorAssociation": "NONE",
          "title": "subdomains not redirecting",
          "body": "Hi,\r\nI was able to configure and successfully use localias in was wsl env (ubuntu 22.04). Everything works fine when i use my SLD (xxxx.test) but if I add my subdomain to my yaml file (admin.xxxx.test), it doesn't redirect properly\r\n\r\nIf you have any recommendations to get subdomains working, I'd greatly appreciate it\r\n\r\nDrew"
        }
      ]
    }
  }
}

and I'm trying to deserialize into a struct like this:

type queryIssues struct {
    // arguments
    repoSlug string               `json:"-"`
    ctx      context.Context      `json:"-"`
    exec     boil.ContextExecutor `json:"-"`
    // results
    Search struct {
        PageInfo PageInfo
        Nodes    []issue
    }
}

type issue struct {
    Number            int64
    ID                string
    CreatedAt         time.Time
    UpdatedAt         time.Time
    State             string
    StateReason       *string
    ClosedAt          *time.Time
    Author            struct {
        Login string
    }
    AuthorAssociation string
    Title             string
    Body              string
}

The actual call to graphql looks like this:

query := `query($cursor0: String, $searchQuery: String!) {
    search(type: ISSUE, query: $searchQuery, first: 100, after: $cursor0) {
        pageInfo {
            endCursor
            hasNextPage
        }
        nodes {
            ... on Issue {
                number
                id
                createdAt
                updatedAt
                state
                stateReason
                closedAt
                author {
                    login
                }
                authorAssociation
                title
                body
            }
        }
    }
}`
result := queryIssues{}
variables := map[string]any{"searchQuery": "...", "cursor0": nil}
githubClient := graphql.NewClient("/graphql", authClient).WithDebug(true)
data, err := githubClient.Exec(ctx, query, &result, variables)

Any ideas what's going on? I've worked around it for now by literally using the exact same query and struct, but using ExecRaw and decoding the JSON response myself. Doing this succeeds every time.

data, err := githubClient.ExecRaw(ctx, query, variables)
if err != nil {
    return err
}
if err := json.Unmarshal(data, &result); err != nil {
    return err
}

Finally, I'll add that I was unable to reproduce the issue ever in a testcase calling into jsonutil.UnmarshalGraphQL. Here's my testcase that always passes (which is weird, because I'd expect this to fail):


func TestExpectedToFail(t *testing.T) {
    t.Parallel()
    rawData := `{"search":{"pageInfo":{"endCursor":"Y3Vyc29yOjE=","hasNextPage":false},"nodes":[{"number":32,"id":"I_kwDOJBmYg86J6maD","createdAt":"2024-05-23T21:03:09Z","updatedAt":"2024-05-28T15:42:02Z","state":"OPEN","stateReason":null,"closedAt":null,"author":{"login":"drewAdorno"},"authorAssociation":"NONE","title":"subdomains not redirecting","body":"Hi,\r\nI was able to configure and successfully use localias in was wsl env (ubuntu 22.04). Everything works fine when i use my SLD (xxxx.test) but if I add my subdomain to my yaml file (admin.xxxx.test), it doesn't redirect properly\r\n\r\nIf you have any recommendations to get subdomains working, I'd greatly appreciate it\r\n\r\nDrew"}]}}`
    x := queryIssues{}
    err := jsonutil.UnmarshalGraphQL([]byte(rawData), &x)
    assert.Nil(t, err)
}
hgiasac commented 3 months ago

So the check to ensure that the slice is not empty. If not the library will panic at this line.

copied, err := copyTemplate(v.Index(0))

But you're right, someSliceExist should be outside the not-empty check.

peterldowns commented 3 months ago

@hgiasac ok, that makes sense, and it clarifies that my problem is probably due to something else. Thanks.

Do you have any idea why I would be getting this failure intermittently? Could it be because I'm re-using the result object? I'm iterating through responses that have nested cursors, and I've been just re-using the same result instance each time. I didn't think that it would matter, since deserialization usually totally overwrites the response struct.

hgiasac commented 3 months ago

Do you have any idea why I would be getting this failure intermittently? Could it be because I'm re-using the result object? I'm iterating through responses that have nested cursors, and I've been just re-using the same result instance each time. I didn't think that it would matter, since deserialization usually totally overwrites the response struct.

We may need the graphql schema on the server to know.

I reproduced your use case and didn't see anything wrong with the response example that you sent. However, can you try this commit to see if it works https://github.com/hasura/go-graphql-client/pull/141