Khan / genqlient

a truly type-safe Go GraphQL client
MIT License
1.07k stars 107 forks source link

Running genqlient on large schemas fails #340

Closed benmai closed 3 months ago

benmai commented 3 months ago

Describe the bug In the last day running go run github.com/Khan/genqlient started failing with:

$ go run github.com/Khan/genqlient
invalid schema: exceeded token limit of 15000
exit status 1

To Reproduce Run genqlient with a large schema.

Expected behavior I would expect generation to succeed.

genqlient version v0.7.0, with github.com/vektah/gqlparser/v2 v2.5.13 or v2.5.14

Additional context I am fairly certain this is related to https://github.com/vektah/gqlparser/pull/291, which I see @benjaminjkraft has already taken a look at and commented on the schema size limit. My workaround for now is to use a replace directive (since we're using Dependabot) to pin it to the old version:

replace github.com/vektah/gqlparser/v2 => github.com/vektah/gqlparser/v2 v2.5.12

If gqlparser is going to have this limitation I would like to also have that available for configuration via genqlient as well, although TBH I don't think schema size is a valid threat vector -- we always load it from disk.

benjaminjkraft commented 3 months ago

In theory you shouldn't need a replace? (Go picks the lowest version possible, so unless some other dep of yours has updated, and you've pulled that update, you should be fine.) But yeah we should just turn this off for genqlient (or better set gqlparser to actually default to disabling it, which I think was the intention but not actually what happened (per my comment on the PR). I see no need for genqlient to even expose this, unless someone can explain a plausible threat model where you'd want it. PRs welcome in either case!

benjaminjkraft commented 3 months ago

Fixed by #341

benmai commented 3 months ago

FWIW the latest gqlparser v2.5.16 works just fine for my case with genqlient! Thanks for the response and for being on top of genqlient's dependencies.

About not needing a replace: I tried removing the replace, manually setting gqlparser to an earlier version, and then updating genqlient with go get -u, and it does update the transitive dependency:

$ go get github.com/vektah/gqlparser/v2@v2.5.12
go: downgraded github.com/vektah/gqlparser/v2 v2.5.13 => v2.5.12
$ go get -u github.com/Khan/genqlient
go: upgraded github.com/vektah/gqlparser/v2 v2.5.12 => v2.5.16

I'm probably missing something but without an explicit replace directive I've not found a way to keep dependencies at a version other than the latest minor. Either way, in this case I'm grateful for the quick responses all around.

benjaminjkraft commented 3 months ago

Yeah, go get -u tends to update everything. I think it's intended behavior, although I'm too lazy to dig up the issue thread. If I want to update a specific package I just do it manually by editing go.mod. (rsc's post on the subject explains how and why the version-selection works.)