graphql-go-tools icon indicating copy to clipboard operation
graphql-go-tools copied to clipboard

panic occurred when gateway access union types

Open vvakame opened this issue 2 years ago • 8 comments

Hi, We tried to use federation gateway for our PoC project. We met the panic with union types. I tried to investigate why this problems are happens. but I couldn't... 😿

reproduction code. https://github.com/vvakame/graphql-go-tools/tree/feat-panic

If this problem is solved, my use case may work, so I hope it will be fixed. 😸

example schema

schema A

extend type Query {
    me: User
}

type User @key(fields: "id") {
    id: ID!
}

schema B

extend type User @key(fields: "id") {
    id: ID! @external
    histories(
        first: Int = 10
        after: String
    ):HistoryConnection!
}

type HistoryEdge {
    node: History!
    cursor: String
}

type HistoryConnection {
    edges: [HistoryEdge!]!
    nodes: [History!]!
}

union History =
    HistoryEntityA |
    HistoryEntityB

type HistoryEntityA {
    id: String
    textA: String
}

type HistoryEntityB {
    id: String
    textB: String
}
query that cause panic
{
  me {
    id
    histories(first: 10) {
      edges {
        node {
          ... on HistoryEntityB {
            id
            textB
          }
        }
      }
      nodes {
        ... on HistoryEntityB {
          id
          textB
        }
      }
    }
  }
}
2021/08/23 18:14:54 http: panic serving [::1]:54855: runtime error: index out of range [5] with length 5
goroutine 47 [running]:
net/http.(*conn).serve.func1(0xc0012f6140)
	/usr/local/Cellar/go/1.16.6/libexec/src/net/http/server.go:1804 +0x153
panic(0x170d140, 0xc0001553e0)
	/usr/local/Cellar/go/1.16.6/libexec/src/runtime/panic.go:971 +0x499
github.com/jensneuse/graphql-go-tools/pkg/ast.(*Document).AddSelection(...)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/ast/ast_selection.go:107
github.com/jensneuse/graphql-go-tools/pkg/engine/datasource/graphql_datasource.(*Planner).EnterInlineFragment(0xc0005c6d00, 0x1)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/engine/datasource/graphql_datasource/graphql_datasource.go:254 +0xb8a
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkInlineFragment(0xc001374300, 0x1)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:2004 +0x165
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkSelectionSet(0xc001374300, 0x4)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1719 +0x385
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkField(0xc001374300, 0x7)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1809 +0x645
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkSelectionSet(0xc001374300, 0x5)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1715 +0x407
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkField(0xc001374300, 0x8)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1809 +0x645
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkSelectionSet(0xc001374300, 0x6)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1715 +0x407
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkField(0xc001374300, 0x9)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1809 +0x645
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkSelectionSet(0xc001374300, 0x7)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1715 +0x407
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walkOperationDefinition(0xc001374300, 0x0)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1588 +0x46c
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).walk(0xc001374300)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1475 +0x60b
github.com/jensneuse/graphql-go-tools/pkg/astvisitor.(*Walker).Walk(0xc001374300, 0xc001313938, 0xc0002db430, 0xc000613f20)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/astvisitor/visitor.go:1301 +0xa5
github.com/jensneuse/graphql-go-tools/pkg/engine/plan.(*Planner).Plan(0xc0001ca1c0, 0xc001313938, 0xc0002db430, 0x0, 0x0, 0xc000613f20, 0x0, 0x100000001)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/engine/plan/plan.go:228 +0x97b
github.com/jensneuse/graphql-go-tools/pkg/graphql.(*ExecutionEngineV2).Execute(0xc0005f0160, 0x1b22318, 0xc00140c040, 0xc001313900, 0x1b1d368, 0xc000020f00, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vvakame/work/gopath/pkg/mod/github.com/jensneuse/[email protected]/pkg/graphql/execution_engine_v2.go:229 +0x24a
gist.github.com/vvakame/7a08b7f517b012cca09f82ec9ab4d1fd/gateway/http.(*GraphQLHTTPRequestHandler).handleHTTP(0xc00068ffb0, 0x1b20d40, 0xc00130a540, 0xc0005c6a00)
	/tmp/graphql-go-tools/examples/federation/gateway/http/http.go:46 +0x565
gist.github.com/vvakame/7a08b7f517b012cca09f82ec9ab4d1fd/gateway/http.(*GraphQLHTTPRequestHandler).ServeHTTP(0xc00068ffb0, 0x1b20d40, 0xc00130a540, 0xc0005c6a00)
	/tmp/graphql-go-tools/examples/federation/gateway/http/handler.go:49 +0x245
...
query that cause parse error
{
  me {
    id
    histories(first: 10) {
      edges {
        node {
          ... on HistoryEntityB {
            id
            textB
          }
        }
      }
    }
  }
}
{
  "errors": [
    {
      "message": "Expected {, found }",
      "locations": [
        {
          "line": 1,
          "column": 169
        }
      ]
    }
  ],
  "data": {
    "me": {
      "id": "1234",
      "histories": {
        "edges": []
      }
    }
  }
}

vvakame avatar Aug 23 '21 09:08 vvakame

Hey, thanks for reporting this issue. I'm not sure if this is an error in the library or user error. We haven't seen this so far in our own projects so I'm not sure. I'm sorry but I'm currently not able to look into this. If it's super urgent, we can discuss if you want to buy paid support and I can prioritize it differently. Otherwise, you'd have to be patient or try to find a solution yourself. If you find a bug and can prove it, I'm more than happy to accept a PR if the quality is ok and the code is tested.

jensneuse avatar Aug 23 '21 11:08 jensneuse

thanks for your response. I tried to fix this trouble in this 2days. but I can't... I think this library a bit difficult to debug.

I think github.com/jensneuse/graphql-go-tools/pkg/engine/plan has some troubles. Maybe It related to inline fragment & selection set.

I will think about another solution once. Thank you very much!

vvakame avatar Aug 27 '21 10:08 vvakame

@vvakame Union and Interface have been broken for federation. #283 solves the issue. I cloned your reproducing code repo and it worked with my PR. Could you check your example one more time ? Do not forget to replace graphql-golang-tools in go.mod

replace github.com/jensneuse/graphql-go-tools => github.com/chedom/graphql-go-tools v1.13.5-0.20210828153106-e565ca7a48cd

and add implementation to Histories method:

func (r *userResolver) Histories(ctx context.Context, obj *model.User, first *int, after *string) (*model.HistoryConnection, error) {
	
	histories  := []model.History{
		model.HistoryEntityA{
			ID: strToPtr("1"),
			TextA: strToPtr("some text 1"),
		},
		model.HistoryEntityA{
			ID: strToPtr("2"),
			TextA: strToPtr("some text 2"),
		},
		model.HistoryEntityA{
			ID: strToPtr("3"),
			TextA: strToPtr("some text 3"),
		},
	}
	
	edges := make([]*model.HistoryEdge, len(histories))

	for i := range histories {
		edges[i] = &model.HistoryEdge{
			Node:   histories[i],
			Cursor: histories[i].(model.HistoryEntityA).ID,
		}
	}

	return &model.HistoryConnection{
		Nodes: histories,
		Edges: edges,
	}, nil
}

chedom avatar Aug 28 '21 16:08 chedom

@chedom what a great work! 👍 I am very interested in how you were able to identify and fix the problem!

BTW, I tried my PoC code with your branch. I got another error (not panic! 🎉 ) likes below.

{
  "errors": [
    {
      "message": "unable to resolve",
      "locations": [
        {
          "line": 9,
          "column": 7
        }
      ],
      "path": [
        "me",
        "histories"
      ]
    }
  ],
  "data": {
    "me": null
  }
}

I tried to make reproduction code. But I couldn't get the exact same result... but unexpected result are reproduced.

https://github.com/vvakame/graphql-go-tools/tree/feat-panic-r2 https://github.com/vvakame/graphql-go-tools/commit/cb78825b60eebcdcf031c68f40daf83f9b8f2936

My PoC code doesn't have union types on top of the HistoryConnection. History type has detail: HistoryDetail. It's nullable. and HistoryDetail is union types.

Response has blank object unexpectedly.

actual

        "nodes": [
          {
            "id": "1",
            "detail": {
              "id": "1a",
              "textA": "some text 1"
            }
          },
          {
            "id": "2",
            "detail": {
              "id": "2b",
              "textB": "some text 2"
            }
          },
          {}
        ]

expected

        "nodes": [
          {
            "id": "1",
            "detail": {
              "id": "1a",
              "textA": "some text 1"
            }
          },
          {
            "id": "2",
            "detail": {
              "id": "2b",
              "textB": "some text 2"
            }
          },
          {
            "id": "3",
            "detail": null
          },
        ]
query of nullable issue & response

Query.

{
  me {
    id
    histories(first: 10) {
      nodes {
        id
        detail {
          ... on HistoryEntityA {
            id
            textA
          }
          ... on HistoryEntityB {
            id
            textB
          }
        }
      }
    }
  }
}

Response.

{
  "data": {
    "me": {
      "id": "1234",
      "histories": {
        "nodes": [
          {
            "id": "1",
            "detail": {
              "id": "1a",
              "textA": "some text 1"
            }
          },
          {
            "id": "2",
            "detail": {
              "id": "2b",
              "textB": "some text 2"
            }
          },
          {}
        ]
      }
    }
  }
}

vvakame avatar Aug 30 '21 03:08 vvakame

ah, I found. when edges or nodes data length is 0. response message becomes unable to resolve.

vvakame avatar Aug 30 '21 04:08 vvakame

@vvakame

Response has blank object unexpectedly.

I found the problem. Added fix.

ah, I found. when edges or nodes data length is 0. response message becomes unable to resolve.

Also fixed

Now response is expected (I have tested in your PoC, change in your go.mod replace github.com/jensneuse/graphql-go-tools => github.com/chedom/graphql-go-tools v1.13.5-0.20210830085101-aa10d4ca518c). Can your check one more time ?

chedom avatar Aug 30 '21 08:08 chedom

@spetrunin you should update the tyk branch with this fix as well I think. @chedom we're about to merge the Union/Interface fixes into master today. Batching will take some more time.

jensneuse avatar Aug 30 '21 09:08 jensneuse

@chedom what a great! I tried your new commit with my PoC. It looks work fine now 👍

vvakame avatar Aug 30 '21 09:08 vvakame