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

Return GraphQL JSON response in case of JSON Input error instead of Internal Server Error #208

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cvergne
Copy link
Contributor

@cvergne cvergne commented Apr 17, 2024

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.php not compatible with implemented interface, so fixed it but tell me if I should remove it and having it fixed in another PR.

@homersimpsons homersimpsons requested a review from mistraloz May 5, 2024 00:09
@homersimpsons
Copy link
Collaborator

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 return new JsonResponse(..., 422) would have been sufficient.

Thank you for the tests too!

@cvergne
Copy link
Contributor Author

cvergne commented Nov 7, 2024

May we reopen this pull request as it just needs review ? :)

@homersimpsons homersimpsons reopened this Nov 9, 2024
@github-actions github-actions bot removed the stale label Nov 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON Error : return readable error instead of RuntimeException
2 participants