Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.12.1 crashes inside gotpl where v0.11.3 gives error messages #1293

Closed
jwatte opened this issue Aug 17, 2020 · 15 comments
Closed

v0.12.1 crashes inside gotpl where v0.11.3 gives error messages #1293

jwatte opened this issue Aug 17, 2020 · 15 comments

Comments

@jwatte
Copy link

jwatte commented Aug 17, 2020

I'm running gqlgen on a schema that has some errors.
v0.12.1 gives a very unhelpful error message:

[21:20:52] dev@devenv-55d5ff879f-9blcj:~/observe/code/go/src/observe/meta/metagql$ go generate
generating core failed: type.gotpl: template: type.gotpl:49:28: executing "type.gotpl" at <$type.Elem.GO>: nil pointer evaluating *config.TypeReference.GOexit status 1
resolver.go:1: running "go": exit status 1

[gqlgen-crash.zip](https://github.com/99designs/gqlgen/files/5086900/gqlgen-crash.zip)

v0.11.3 is more helpful:

[21:26:17] dev@devenv-55d5ff879f-9blcj:~/observe/code/go/src/observe/meta/metagql$ go generate
validation failed: packages.Load: /home/dev/observe/code/go/src/observe/meta/metagql/gql_dataset.go:700:33: cannot use ret (variable of type []*metatypes.Dataset) as *metatypes.Dataset value in argument to updateDatasetNilsToEmptyArrays
/home/dev/observe/code/go/src/observe/meta/metagql/progressive.go:443:24: cannot use results (variable of type []*metaparser.TaskResult) as []metaparser.TaskResult value in argument to canSendResults
/home/dev/observe/code/go/src/observe/meta/metagql/progressive.go:493:23: cannot use results (variable of type []*metaparser.TaskResult) as []metaparser.TaskResult value in argument to canSendResults
/home/dev/observe/code/go/src/observe/meta/metagql/progressive.go:517:21: cannot use results (variable of type []*metaparser.TaskResult) as []metaparser.TaskResult value in argument to canSendResults
exit status 1
resolver.go:1: running "go": exit status 1

I expect gqlgen to not crash with an error message that tells me nothing about where the cause for the error is, instead I expect it to give me good error messages.

(I think 0.11.3 actually generated the code and the errors come from the go compile command, so maybe this reduces to "v0.12.1 shouldn't crash, period!")

Attaching schema.graphql and gqlgen.yml files

@tsaikd
Copy link

tsaikd commented Aug 27, 2020

I faced the issue when upgrading to v0.12.2 from v0.11.3. I guess it's a bug that assume Nilable Marshaler will have Elem at the same time. The following patch can work around, but I',m not sure any other edge cases will happen.

				{{- else if eq ($type.GO | ref) "map[string]interface{}" }}
					return v.(map[string]interface{}), nil
				{{- else if $type.IsMarshaler }}
-					{{- if $type.IsNilable }}
+					{{- if and $type.IsNilable $type.Elem }}
						var res = new({{ $type.Elem.GO | ref }})
					{{- else}}
						var res {{ $type.GO | ref }}
					{{- end }}
					err := res.UnmarshalGQL(v)
					return res, graphql.WrapErrorWithInputPath(ctx, err)

@tmc
Copy link
Contributor

tmc commented Sep 1, 2020

For what it's worth this was introduced in 997efd0

@jwatte
Copy link
Author

jwatte commented Sep 1, 2020

With 0.12.2, we now crash even on seemingly legitimate schemas (that work well in 11.3) so we have to pin version 11.3.
Unfortunately not easy to extract into a simple reproduction case, as we have a large gqlgen.yml that maps the API to our internal business entities in a bunch of different packages.

@fredbi
Copy link

fredbi commented Nov 12, 2020

I've had a similar issue that prevents me from upgrading from v0.11.3.
I described it in #1391, but I cannot tell if this is really a duplicate issue.
I've posted a PR to work around my problem (#1392), which is due to external scalar types of the map kind.

@jwatte
Copy link
Author

jwatte commented Dec 9, 2020

I think fredbi's problem is the same as mine -- I also have a scalar type that's map[string]interface{}

@oiime
Copy link

oiime commented Jan 15, 2021

Easily reproducible with a scalar like:

type ScalarType []string

func (s *ScalarType) UnmarshalGQL(v interface{}) error {
	s, ok := v.(*ScalarType)
	if !ok {
		return errors.New(fmt.Sprintf("Failed to unmarshal value: #%v", v))
	}
	return nil
}

func (s ScalarType) MarshalGQL(w io.Writer) {
	b, _ := json.Marshal(s)
	w.Write(b)
}

Are there any plans to fix this in the near future?

@minitauros
Copy link

Running into the same problem with a map[string]interface{} (which we use for JSON types).

v0.13.0

@oiime
Copy link

oiime commented Jan 20, 2021

For anyone looking for a temporary solution, doing something like this is enough to bypass it:

type ScalarType []string

func MarshalScalarType(t ScalarType) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		b, _ := json.Marshal(t)
		w.Write(b)
	})
}

func UnmarshaScalarType(v interface{}) (ScalarType, error) {
	value, ok := v.(ScalarType)
	if !ok {
		return nil, errors.New(fmt.Sprintf("Failed to unmarshal ScalarType: #%v", v))
	}
	return value, nil
}

@minitauros
Copy link

minitauros commented Jan 20, 2021

How does this bypass the problem? It's a generation problem, right?

@duckbrain
Copy link
Contributor

@minitauros It's working around the problem by replacing the MarshalGQL method with a MarshalX function. This is templated differently, so it avoid the problem code-path.

It appears GQLGen gives preference to the function format, so you can simply call the original method as a work-around, and remove it when this bug is fixed.

func MarshalScalarType(t ScalarType) graphql.Marshaler {
	return graphql.WriterFunc(t.MarshalGQL)
}

func UnmarshalScalarType(v interface{}) (t ScalarType, err error) {
	err = t.UnmarshalGQL(v)
	return
}

@jwatte
Copy link
Author

jwatte commented Mar 5, 2021

I tried @duckbrain workaround, but it doesn't work on v0.13.0. I have a type:

scalar JsonObject

models:
  JsonObject:
     model: observe/meta/metatypes.ObjectScalar

type ObjectScalar map[string]interface{}

func MarshalObjectScalar(o ObjectScalar) graphql.Marshaler {
    return graphql.WriterFunc(o.MarshalGQL)
}

func UnmarshalObjectScalar(v interface{}) (o ObjectScalar, e error) {
    o = EmptyObject()
    e = o.UnmarshalGQL(v)
    return
}

Running gqlgen, I get this error:

jwatte:metagql$ go generate
validation failed: packages.Load: /home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:42632:9: cannot use res (variable of type metatypes.ObjectScalar) as *metatypes.ObjectScalar value in return statement
/home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:42639:39: cannot use v (variable of type *metatypes.ObjectScalar) as metatypes.ObjectScalar value in argument to metatypes.MarshalObjectScalar

The generated callers are:

func (ec *executionContext) unmarshalOJsonObject2ᚖobserveᚋmetaᚋmetatypesᚐObjectScalar(ctx context.Context, v interface{}) (*metatypes.ObjectScalar, error) {
    if v == nil {
        return nil, nil
    }
    res, err := metatypes.UnmarshalObjectScalar(v)
    return res, graphql.ErrorOnPath(ctx, err)  <-- error here
}

func (ec *executionContext) marshalOJsonObject2ᚖobserveᚋmetaᚋmetatypesᚐObjectScalar(ctx context.Context, sel ast.SelectionSet, v *metatypes.ObjectScalar) graphql.Marshaler {
    if v == nil {
        return graphql.Null
    }
    return metatypes.MarshalObjectScalar(v)  <-- error here
}

If I update the implementation of marshal/unmarshal, I instead get these errors:

func MarshalObjectScalar(o *ObjectScalar) graphql.Marshaler {
    return graphql.WriterFunc(o.MarshalGQL)
}

func UnmarshalObjectScalar(v interface{}) (o *ObjectScalar, e error) {
    ro := EmptyObject()
    o = &ro
    e = o.UnmarshalGQL(v)
    return
}

validation failed: packages.Load: /home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:39920:9: cannot use res (variable of type *metatypes.ObjectScalar) as metatypes.ObjectScalar value in return statement
/home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:39930:39: cannot use v (variable of type metatypes.ObjectScalar) as *metatypes.ObjectScalar value in argument to metatypes.MarshalObjectScalar
/home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:42617:9: cannot use res (variable of type *metatypes.ObjectScalar) as metatypes.ObjectScalar value in return statement
/home/jwatte/observe/code/go/src/observe/meta/metagql/generated.go:42624:39: cannot use v (variable of type metatypes.ObjectScalar) as *metatypes.ObjectScalar value in argument to metatypes.MarshalObjectScalar

The erroring lines are:

func (ec *executionContext) marshalNJsonObject2observeᚋmetaᚋmetatypesᚐObjectScalar(ctx context.Context, sel ast.SelectionSet, v metatypes.ObjectScalar) graphql.Marshaler {
    if v == nil {
        if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) {
            ec.Errorf(ctx, "must not be null")
        }
        return graphql.Null
    }
    res := metatypes.MarshalObjectScalar(v)  <-- error here
    if res == graphql.Null {
        if !graphql.HasFieldError(ctx, graphql.GetFieldContext(ctx)) {
            ec.Errorf(ctx, "must not be null")
        }
    }
    return res
}

func (ec *executionContext) unmarshalOJsonObject2observeᚋmetaᚋmetatypesᚐObjectScalar(ctx context.Context, v interface{}) (metatypes.ObjectScalar, error) {
    if v == nil {
        return nil, nil
    }
    res, err := metatypes.UnmarshalObjectScalar(v)
    return res, graphql.ErrorOnPath(ctx, err)  <-- error here
}

func (ec *executionContext) marshalOJsonObject2observeᚋmetaᚋmetatypesᚐObjectScalar(ctx context.Context, sel ast.SelectionSet, v metatypes.ObjectScalar) graphql.Marshaler {
    if v == nil {
        return graphql.Null
    }
    return metatypes.MarshalObjectScalar(v)  <-- error here
}                                                                                                                                       

So, I'm now in a catch-22 -- with a pointer implementation, some functions fail; with a value implementation, some other functions fail.

And removing the function implementation, falling back to the interface, still provokes the bad code path, which causes gqlgen to error out with a bad template:

jwatte:metagql$ go generate
generating core failed: type.gotpl: template: type.gotpl:49:28: executing "type.gotpl" at <$type.Elem.GO>: nil pointer evaluating *config.TypeReference.GOexit status 1
resolver.go:1: running "go": exit status 1

@oiime
Copy link

oiime commented Mar 10, 2021

@jwatte You can change it to MarshalObjectScalar(v interface{}) graphql.Marshal and switch by the type you receive

@hardlyknowem
Copy link

We tried the fix proposed by @oiime and @duckbrain above and it didn't work. There has been a PR up for this for quite a while (see #1317); I have poked one of the maintainers to see if it can be merged sooner rather than later.

@ThanhPhanTiki
Copy link

i also tries byt above hack didn't work

@tankbusta
Copy link

I just noticed #1317 was merged in 3 days ago so I updated to gqlgen@master and this is resolved (for me at least). I was having the same issue as @jwatte but now everything works with custom MarshalGQL/UnmarshalGQL methods on the ScalarType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests