Confusing error message when passing non-interface{} slice type for List variable

[vergenzt]

Demo

See https://gist.github.com/vergenzt/d5685cee2c8f80e9d98bca7ef3845173 for example schema/inputs demonstrating this issue.

Issue

When I pass in a concrete slice value (for instance, a []map[string]interface{}) instead of an []interface{} as the value of a GraphQL variable of type [SomeInputType!]!, I get the following panic:

graphql: graphql: panic occurred: interface conversion: interface {} is []map[string]interface {}, not map[string]interface {}

occurring at: https://github.com/neelance/graphql-go/blob/b46637030579abd312c5eea21d36845b6e9e7ca4/internal/exec/packer/packer.go#L223

and called from: https://github.com/neelance/graphql-go/blob/b46637030579abd312c5eea21d36845b6e9e7ca4/internal/exec/packer/packer.go#L254

Cause

The issue is caused by: https://github.com/neelance/graphql-go/blob/b46637030579abd312c5eea21d36845b6e9e7ca4/internal/exec/packer/packer.go#L246-L250

If the value passed in for the variable is not exactly of type []interface{}--for instance if it's a slice of a more specific type--then the code above wraps it into []interface{}{ <original_slice_value> } in order to coerce single-value inputs into lists (#41).

However slice types in Go are not covariant: a []myStructType is not assignable to a []interface{}. So this check ends up wrapping the input slice type into another slice, and the attempt to convert the "first element" of the wrapped slice (i.e. the original full slice) into the corresponding inner GraphQL list element type fails.

Proposed fix

Can we just add a better error message if the type of the value passed is a non-interface{} slice type?

[thrasher-redhat]

I believe I've run into the same thing with a []string not satisfying the interface. It feels really ugly, but you can use a *[]string in the resolver args struct to get around this.

Is the correct approach to force everyone to use interface wrappers for any list (and update the error message to better reflect that)? Or should the packer properly handle lists that are not interfaces?

Changing https://github.com/graph-gophers/graphql-go/blob/b46637030579abd312c5eea21d36845b6e9e7ca4/internal/exec/packer/packer.go#L246-L250 to something like the following would work. It would also probably break backwards compatibility for those who work around it by using *[]type though.

func (e *listPacker) Pack(value interface{}) (reflect.Value, error) {
	list, ok := value.([]interface{})
	if !ok {
		list = make([]interface{}, len(value))
		for i := range value {
			list[i] = value[i]
		}
	}

	v := reflect.MakeSlice(e.sliceType, len(list), len(list))
	for i := range list {
		packed, err := e.elem.Pack(list[i])
		if err != nil {
			return reflect.Value{}, err
		}
		v.Index(i).Set(packed)
	}
	return v, nil
}

[thrasher-redhat]

^Thoughts @neelance ?

[neelance]

@thrasher-redhat Sorry, I'm quite busy with bringing WebAssembly to the Go compiler. @tonyghita is the primary maintainer of this project now.

[thrasher-redhat]

Thanks for the redirect! I just pinged the name on the code =P