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

Improve error messages #12

Open
vwkd opened this issue Jun 21, 2023 · 2 comments
Open

Improve error messages #12

vwkd opened this issue Jun 21, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@vwkd
Copy link
Owner

vwkd commented Jun 21, 2023

  • explain why restrictions are necessary: non-null query return type is error because database might return null for non-existent id or if throws error, e.g. bookById: Book!
  • maybe validate returned data ourselves instead of relying on GraphQL? Could provide more specific database corruption error, e.g. if row doesn't conform to schema. But adding lots of checks would increase complexity, maintenance burden to update with every change creates friction, etc.
  • throw errors uniformly if concurrent change, e.g. currently returns null if concurrent change in mutation, but throws if concurrent change in query
@vwkd
Copy link
Owner Author

vwkd commented Jul 3, 2023

The reasons for nullability are now mentioned in the README with e34f56f

We continue returning null in a delete mutation instead of throwing a concurrent change error, because the versionstamps are given by user input and we don't want to throw for outdated user input, just like we don't throw for a non-existent id in a query.

We can think about nicer error messages by moving the return data validation in-house instead of leaving it to GraphQL later when the library is in a stable shape.

@vwkd vwkd closed this as completed Jul 3, 2023
@vwkd
Copy link
Owner Author

vwkd commented Jul 8, 2023

Currently, a many query returns a pageInfo even if an error is thrown in the leaf resolver and the edges are empty. This is due to the root resolvers reading the id's only, and then the leaf resolver reading any selected fields. Not clear how to solve this.

@vwkd vwkd reopened this Jul 8, 2023
@vwkd vwkd added the bug Something isn't working label Jul 15, 2023
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

No branches or pull requests

1 participant