Return GraphQL JSON response in case of JSON Input error instead of Internal Server Error #208
Conversation
|
I will let @mistraloz review this as you discussed it. About the implementation I'm okay with the current status. But I'm feeling that it may be too complex, this adds a lew listener, a new exception (which names conflict with a std one). I think this is the right way to approach this as the end user will have enough control on it. But maybe a simple Thank you for the tests too! |
|
May we reopen this pull request as it just needs review ? :) |
|
@cvergne what do you think about such refactoring - to avoid introducing an exception listener for now (as suggested in #208 (comment))? |
|
@andrew-demb looks good, we ended up adding a listener in our app to make sure we always return a valid json in |
|
@cvergne please rebase this PR over the current master with the applied suggestion from #208 (comment) |
|
@andrew-demb I will do it by tuesday at the latest 😉 |
cvergne
force-pushed
the
feat/json-error-to-graphql-response
branch
2 times, most recently
from
57abcaa to
fba23aa
Compare
cvergne
force-pushed
the
feat/json-error-to-graphql-response
branch
from
fba23aa to
80cdfbb
Compare
|
@andrew-demb Please do not merge it yet, I have to check why the tests are failing. I'll do it this monday 😉 |
There was a problem hiding this comment.
should be moved to src/Exceptions
Fixes #207
The purpose of the PR is to return a standard GraphQL JSON error response in case of JSON error in given input (request body).
{ "errors": [ { "message": "Invalid JSON.", "extensions": { "reason": "Syntax error" } } ] }It adds an Exception Listener which set a JsonResponse in case of thrown GraphQLExceptionInterface (to catch any GraphQL Exception outside of the GraphQL Server context).
Also added the two exception cases of the controller into errors functional tests.
Note
Got an error when running test about
DependencyInjection/Configuration.phpnot compatible with implemented interface, so fixed it but tell me if I should remove it and having it fixed in another PR.