graphql / graphql-js

A reference implementation of GraphQL for JavaScript
http://graphql.org/graphql-js/
MIT License
20.08k stars 2.03k forks source link

object validation creates misleading non null error #1989

Closed mschipperheyn closed 1 month ago

mschipperheyn commented 5 years ago

What is happening

A non null error is reported for a field that is not null, e.g. Cannot return null for non-nullable field Account.id.

What is really happening

When you dig into graphql code and start console.logging intermediate results, you start seeing very different things:

Scenario 1 Expected a value of type "PostRegistration" but received: "TERMS"

So, in this case, I had added a value to an ENUM field in the database but forgot to add it to the graphql enum, hence the error. Impossible to determine from the provided error message (cannot return null ...).

Scenario 2 non nullable field Account.foo was null

In this case, another required key on the same object was provided with a null value. It seems obvious, but the error message is so specific, that you can easily miss this.

In short, a whole class of field validation errors, is being aggregated to a single highly specific message that is plain wrong.

tedb19 commented 5 years ago

any progress on this?

djay05554 commented 4 years ago

Hello, guys

I can't find the solution to the bug. I correctly confirm my schema but there is no wrong.

timbrownsf commented 4 years ago

Any recommendations on how to debug this if it is happening and figure out the actual error?

mschipperheyn commented 4 years ago

@timbrownsf Yeah, Easy: add console.log of the error at the error the erronous error occurs so you can see what the actual error is Harder but better. Use server side inspect and add a breakpoint there.

emma255 commented 4 years ago

this one works well return Customer::findOrFail($args['id']); and this one not brings that error return Customer::where('email', $args['email'])->get();

So for myself i think there is an issue on handling exceptions in case the second query above fails, as compared to the first query above that whether find or fail. so am continuing digging up whether i will get the solution of it..

vanthome commented 4 years ago

That is really annoying, very hard to trace errors. I see something like this a good remedyhttps://github.com/graphql/graphql-js/pull/402 With that we could add proper logging for field level resolver throws.

nidshar commented 4 years ago

Any updates on this one?

allaniftrue commented 4 years ago

https://github.com/apollographql/apollo-client/issues/4180#issuecomment-493795550, this reply helped solve the issue

evifere commented 4 years ago

I bypass the problem using JSON.parse and JSON.stringify like this.

  @Query(returns => Film, { nullable: false })
  async film(
    @Arg("episode_id")
    episode_id: number
  ) {
    let film = await FilmsModel.findOne({episode_id:episode_id});
    return JSON.parse(JSON.stringify(film)) ;
  }
vipiny35 commented 4 years ago

I bypass the problem using JSON.parse and JSON.stringify like this.

  @Query(returns => Film, { nullable: false })
  async film(
    @Arg("episode_id")
    episode_id: number
  ) {
    let film = await FilmsModel.findOne({episode_id:episode_id});
    return JSON.parse(JSON.stringify(film)) ;
  }

Didn't work. Any other work-around? I tried using lean() and toObject() but with no luck!

fromi commented 4 years ago

@vipiny35 Not sure if your problem is the same, but if you want to access sub-objects functions you have to do something like this:

  @Query(() => Game)
  async game(@Arg('id') id: string) {
    return GameModel.findById(id)
  }

  @FieldResolver(() => GraphQLJSON, {nullable: true})
  setup(@Root() game: Game, @Ctx('userId') userId: string) {
    const player = game.players.find(player => toObjectType(Player, player).isControlledBy(userId))
    ...
  }

export default function toObjectType<T>(Type: new (...args: any[]) => T, object: T) {
  return Object.assign(new Type(), object);
}

You would expect "game" to be the Document from mongoose here, however in the FieldResolver it is some flat data objet, not a class anymore.

yaacovCR commented 2 years ago

@mschipperheyn Can you provide a reproducible example? (3 years later!!!)

For posterity: when we bubble up nulls, the original error is still supposed to be included in the errors array for this exact purpose. So this problem at first glance seems surprising. I may have misunderstood, of course!

yaacovCR commented 1 month ago

Closing this old issue. Feel free to reopen as necessary.