n1ru4l / envelop

Envelop is a lightweight library allowing developers to easily develop, share, collaborate and extend their GraphQL execution layer. Envelop is the missing GraphQL plugin system.
https://envelop.dev
MIT License
788 stars 128 forks source link

sentry plugin does not report ScalarType errors for resolved values #1808

Open n1ru4l opened 1 year ago

n1ru4l commented 1 year ago

Breakdown

These errors thrown within graphql scalar types serialize function are unreported but should IMHO as this indicates something wrong within the GraphQL resolvers.

image

However, it is currently hard to catch these due to how we detect "unexpected" errors. Any GraphQLError that has an originalError property that is not a GraphQLError is considered an unexpected error. Any GraphQLError that has an originalError property which is a GraphQLError is considered an expected error.

This feels a bit inconsistent with how errors are asserted in other places. E.g. a resolver field non-null check will raise an Error instead of a GraphQLError.

image

Possible solutions for the problem

  1. Talk with graphql-js maintainers and convince them to throw Error instead of GraphQLError within the scalar functions.
  2. Add a way of wrapping/manipulating the schema for adding additional information to the error objects
n1ru4l commented 1 year ago

I created the following PR for resolving this within GraphQL Hive: https://github.com/kamilkisiela/graphql-hive/pull/2299

This logic could potentially life in the useMaskedErrors plugin. 🤔

enisdenjo commented 1 year ago

But how are serialisation errors unexpected errors? I mean, maybe the variables are wrongly provided by the user and he should be warned (using GQLError) instead of everything blowing up?

Compared to null for non-nullable, this is definitely an issue and should blow.

I misinterpreted, this is about serialisation - yeah, I agree then, serialisation issues should be errors.