search

webonyx / graphql-php

PHP implementation of the GraphQL specification based on the reference implementation in JavaScript
https://webonyx.github.io/graphql-php
MIT License
4.64k stars 566 forks source link

Distinguish errors of validators, within the executor and within business logic #1373

Open wutsch0 opened 1 year ago

wutsch0 commented 1 year ago

Hi, accoring to your error-handling documentation there are 3 kinds of errors: Syntax, Validation and Execution.

Syntax errors seem to be well covered by GraphQL\Error\SyntaxError, but for all other error cases the base GraphQL\Error\Error is used, which makes it impossible to distinguish errors qualitatively. This would e.g. be needed to filter out errors in bug trackers. E.g. if triggered by clients which send invalid queries, such as leaving out required parameters or sending incorrect parameter types. Theoretically it would also be possible to distinguish those kinds of cases by properties provided on GraphQL\Error\Error. So far there is only a category property which could be used, but it changes based on if a previous exceptions is provided during construction time. As both cases appear within the codebase, this property can not be used to distinguish such use cases at the moment.

Introducing more error types via exceptions inheriting from GraphQL\Error\Error seems to be the cleaner approach, but would have the disadventage of breaking backwards compatibility, since e.g. the visitors of validation rules should return more specific exceptions as of now. A backwards compatible way could be to introduce additional properties in GraphQL\Error\Error as described above, but would have the disadventage of not enabling a hierachy within exceptions.

What is your opinion on that topic?

spawnia commented 1 year ago

So far there is only a category property

No more, we got rid of that in v15.

I am not convinced I agree with the categorization the docs put forth. In my opinion, syntax errors and validation errors fall in the same category - they are both caused by clients sending invalid requests.

wutsch0 commented 1 year ago

I am not convinced I agree with the categorization the docs put forth. In my opinion, syntax errors and validation errors fall in the same category - they are both caused by clients sending invalid requests.

Yes, in some way they do. To me it would be fine to have them in one category, as long as they would be distinguishable from other errors which occure within the business logic implementation

spawnia commented 1 year ago

Perhaps you can take a look at the exception classes defined in https://github.com/webonyx/graphql-php/tree/master/src/Error and their usage. I am open for pull requests that introduce an improved hierarchy.