None type passed instead of Empty Array

[peterbraden]

Apologies if this is user error, but I cannot find anything relating to this in the docs or existing issues.

With a schema:

input Foo {
    bar: [Bar]!
}

bar should never be a null type, but always passed as an empty list.

Using the query generated from:

var b []query.Bar

query.Foo {
  bar: b
}

and:

 query GetFooBar($q: Foo!) {
   getFooBar(query: $q) {
      data 
}

We are instead getting {"bar": null }

We should instead get {"bar": [] }

This happens at runtime.

genqlient version
v0.3.1-0.20220113190133-9fbb6b87aa40

[benjaminjkraft]

Ah, interesting, thanks for raising this. The workaround, if it isn't clear, is to use b := []query.Bar{}, i.e. an empty list rather than null; that will jsonify to [] rather than null, which the server will accept.

The question is, should genqlient do that for you, should it complain explicitly, or should it just pass through to the server (the current behavior). Thinking out loud, I have to admit I don't like any of the options: doing it automatically seems a bit too magical as you did after all pass [], not nil; complaining explicitly seems like not genqlient's job; and just passing it through seems not the most helpful as you've pointed out. The closest parallel I can think of is integers (see #39 and the FAQ), where we went for convenience and just pass whatever int you give us to the server even though the spec says only 32-bit integers are allowed, which I think has indeed proven to be the least-bad option. The reason it feels parallel is that this is a case where the best equivalent Go type is one with values that aren't valid on the GraphQL side (although in this case there is no alternative like int32 that's theoretically correct). The difference here is that arguably there is a natural mapping from the invalid value (null) to a valid one ([]), unlike a too-large int where there's not much to do, but that mapping still seems too magical to me since those are distinct Go values with distinct GraphQL mappings, one of which is simply not valid for this type. (Although on the third hand it's hard to imagine a case where you would intend anything else; if the server intends for the list to be of length at least one it can validate for that!)

A perhaps interesting question here is: what do servers do? I didn't check but I assume Apollo will validate for this, because it's very good about that. But gqlgen, contra the spec, does not; it treats null as equivalent to [] in this case. That suggests two things: first, like with int32, it's not necessarily the case that we as a client should be trying to force everyone to follow the spec strictly, and second, making this an error could actually be a breaking change if anyone is relying on the fact that genqlient allows passing a null here.

The last thing is that I worry a little that this will be annoying to implement; I think it would require we write an unmarshaler method for all non-nullable types which are represented by nullable types in Go (which includes all non-nullable lists and objects, at least). That's not a dealbreaker but I don't feel great about it. We could revisit if/when we do #47, which would make it much easier.

So on the whole I think I'm inclined to say this is working as intended: you've passed an invalid value which is (unfortunately but necessarily) a member of the Go type we must use, and we leave it to the server to explicitly reject that (or not!). I'm open to further discussion though, or comments from anyone else who has been hit by this! So I'll leave it open for a bit for that.

[peterbraden]

Thanks for such a prompt and detailed response!

I'm inclined to agree with you, as long as this is documented, and now with your response in this ticket then it is, then I think the current behavior is reasonable.