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

[Proposal] Panic message can be improve when schema type name is not compatible #629

Closed
louis993546 opened this issue Mar 17, 2019 · 3 comments · Fixed by #635
Closed

[Proposal] Panic message can be improve when schema type name is not compatible #629

louis993546 opened this issue Mar 17, 2019 · 3 comments · Fixed by #635
Labels
bug Something isn't working

Comments

@louis993546
Copy link

Expected Behaviour

  1. Enter the 1st schema below
  2. go run github.com/99designs/gqlgen init
  3. Panic on why having type Time is not a good idea

Actual Behavior

  1. Enter the 1st schema below
  2. go run github.com/99designs/gqlgen init
  3. Panic with this:
panic: interface conversion: types.Type is *types.Signature, not *types.Named

goroutine 1 [running]:
github.com/99designs/gqlgen/codegen.(*builder).bindField(0xc000225668, 0xc000314a00, 0xc000144990, 0x0, 0x0)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/codegen/field.go:110 +0x1011
github.com/99designs/gqlgen/codegen.(*builder).buildField(0xc000225668, 0xc000314a00, 0xc0000d3180, 0x0, 0xe91a00, 0x0)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/codegen/field.go:64 +0x31d
github.com/99designs/gqlgen/codegen.(*builder).buildObject(0xc000225668, 0xc00045ce40, 0x9d3c36, 0x6, 0x0)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/codegen/object.go:72 +0x4d4
github.com/99designs/gqlgen/codegen.BuildData(0xc0001c40c0, 0xaad6e0, 0xc000119120, 0x0)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/codegen/data.go:83 +0x47f
github.com/99designs/gqlgen/api.Generate(0xc0001c40c0, 0xc0000d9938, 0x1, 0x1, 0xc000136ec8, 0x6)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/api/generate.go:37 +0x250
github.com/99designs/gqlgen/cmd.GenerateGraphServer(0xc0001c40c0, 0x9ecc1f, 0x10)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/cmd/init.go:75 +0xad
github.com/99designs/gqlgen/cmd.glob..func2(0xc0001f06e0)
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/cmd/init.go:70 +0xc9
github.com/urfave/cli.HandleAction(0x9181c0, 0xa13a50, 0xc0001f06e0, 0xc000114a00, 0x0)
        /home/louis/go/pkg/mod/github.com/urfave/cli@v1.20.0/app.go:492 +0x7c
github.com/urfave/cli.Command.Run(0x9d1f86, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x9fad8f, 0x1b, 0x0, ...)
        /home/louis/go/pkg/mod/github.com/urfave/cli@v1.20.0/command.go:210 +0x92f
github.com/urfave/cli.(*App).Run(0xc0001b89c0, 0xc00000e080, 0x2, 0x2, 0x0, 0x0)
        /home/louis/go/pkg/mod/github.com/urfave/cli@v1.20.0/app.go:255 +0x69b
github.com/99designs/gqlgen/cmd.Execute()
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/cmd/root.go:40 +0x224
main.main()
        /home/louis/go/pkg/mod/github.com/99designs/gqlgen@v0.8.1/main.go:8 +0x20
exit status 2

Minimal graphql.schema and models to reproduce

This one cases the panic:

type Query {
    updatedProfiles: [User!]!
}

type User {
    id: ID!
    about: String!
    karma: Int!
    delay: Int!
}

type Time {    # this is no good
    epoch: Int!
    iso8601: String!
}

And this will work just fine

type Query {
    updatedProfiles: [User!]!
}

type User {
    id: ID!
    about: String!
    karma: Int!
    delay: Int!
}

type Timestamp {    # renamed and everything works fine
    epoch: Int!
    iso8601: String!
}
@louis993546 louis993546 changed the title Panic message can be improve when schema type name is not compatible [Proposal] Panic message can be improve when schema type name is not compatible Mar 17, 2019
@mathewbyrne
Copy link
Contributor

I think a panic is probably not the right place to improve the error messaging. Hitting a panic during codegen is usually a case where our validation needs to be improved so that the panic is never hit.

@louis993546
Copy link
Author

Wow that's quick! Thanks!

@jufemaiz
Copy link

jufemaiz commented Oct 8, 2020

FYI I am currently seeing this on v0.13.

Panics on:

type TimeOfDay {
  hours: Int
  minutes: Int
  seconds: Int
  nanos: Int
}

does not panic on:

type TimeOfDay2 {
  hours: Int
  minutes: Int
  seconds: Int
  nanos: Int
}

or

type TimeOfDayType {
  hours: Int
  minutes: Int
  seconds: Int
  nanos: Int
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants