MarshalGQL implementations that write invalid JSON cause panic in graphqlHandler.ServeHTTP

[tv42]

What happened?

A custom scalar MarshqlGQL implementation triggered the following, by outputting a string instead of a JSON-quoted string. None of my relevant code was in the stack trace; after the copy-paste bit is just me starting the http server.

json: error calling MarshalJSON for type json.RawMessage: invalid character 'a' looking for beginning of value

goroutine 24 [running]:
runtime/debug.Stack(0x1, 0x0, 0x0)
	runtime/debug/stack.go:24 +0x9d
runtime/debug.PrintStack()
	runtime/debug/stack.go:16 +0x22
github.com/99designs/gqlgen/graphql.DefaultRecover(0xed8a00, 0xc0003386c0, 0xaab940, 0xc000538c80, 0x100, 0xc000014b08)
	github.com/99designs/[email protected]/graphql/recovery.go:16 +0xa7
github.com/99designs/gqlgen/handler.(*graphqlHandler).ServeHTTP.func2(0xc0001e42c0, 0xed8a00, 0xc0003386c0, 0xed6d00, 0xc000538340)
	github.com/99designs/[email protected]/handler/graphql.go:536 +0x73
panic(0xaab940, 0xc000538c80)
	runtime/panic.go:679 +0x1b2
github.com/99designs/gqlgen/handler.(*graphqlHandler).ServeHTTP(0xc000262920, 0xed6d00, 0xc000538340, 0xc0000eaa00)
	github.com/99designs/[email protected]/handler/graphql.go:559 +0x19e0
net/http.HandlerFunc.ServeHTTP(0xc000274670, 0xed6d00, 0xc000538340, 0xc0000eaa00)
	net/http/server.go:2007 +0x44
...

What did you expect?

1. Error handling to be in place, no panic
2. An easier to debug error message (had to hack a temporary copy of gqlgen outside of module cache to add debug prints to see what the RawMessage contents were).
3. An easier to use, less error-prone, marshaling API (is there a reason MarshalGQL is separate from just JSON marshaling?)

versions

• gqlgen version: v0.10.1
• go version? go version go1.13.3 linux/amd64
• dep or go modules? modules

[visteras]

You need add " as first and last symbol
P.S. I apologize. I did not immediately read correctly what was meant.

[tv42]

I am well aware of what I need to do -- but I think the API as it is today is not great, and makes debugging a simple mistake way too hard. I'd love to know more about the intent behind that MarshalGQL API; I feel like I personally would have just passed things to json.Marshal, and let them have MarshalJSON methods if they want.

I understand GraphQL might need something special on the unmarshal side (inputs can come from either inside the query or JSON, but that sounds like you can pretend they're JSON in either case because that compatibility has to exist anyway). But for the marshaling -- even if you can make the argument that GraphQL pretends to be independent of JSON, the current io.Writer-based API is tying things down to a specific wire format anyway.

[vektah]

Aliases are the main thing that prevent using json.Marshal, the same field can be present multiple times with different names and different args. Can't use maps either as the request determined order.

The outer json marshal will be removed soon, which is what is generating this error. Once that's gone you will get the same behaviour you would from a broken custom marshal func with json.marshal - bad json

[tv42]

That's not what encoding/json does, though: https://play.golang.org/p/Sz_7cJx4pBl

[tv42]

I wish I understood what "same field being present multiple times" means.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[tv42]

Hey stale bot, just because bugs are not fixed, or they are flagged as "question" for some reason, doesn't mean they're fixed.

[adrianduke]

I suspect you can replicate this issue with: #1195. which would imply you can send a valid go float value into your GQL and cause a panic with invalid JSON.

[utx0]

I am having what looks like the same issue:

json: error calling MarshalJSON for type json.RawMessage: invalid character 'T' looking for beginning of value

From a simple bit of testing code that I put together after read the same page:

func MarshalMatrix(m model.Matrix) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		_, err := io.WriteString(w, fmt.Sprintf("Testing"))
		if err != nil {
			panic(err)
		}
	})
}

I however have no idea how to resolve this issues?

[enjoylife]

At the minimum you could check for errors, and log them or defer and recover from any panics. Falling back to some default, such as null, if something goes wrong.

func MarshalMatrix(s CustomStr) graphql.Marshaler {
	return graphql.WriterFunc(func(w io.Writer) {
		// to recover from panics..
		defer func() {
			if err := recover(); err != nil {
				// I would log a panic occurred,
				// ...

				// but still return null..
				w.Write([]byte(`null`))
			}
		}()

		// Do stuff which could error...
		_, err := io.WriteString(w, strconv.Quote(s))
		if err != nil {
			// log errror occured
                         // ..
                         // fallback
			w.Write([]byte(`null`))
		}
	})
}

For your specific issue @utx0 you need to provide a valid json string using strconv.Quote(fmt.Sprintf("Testing"))