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

JSON Error : return readable error instead of RuntimeException #207

Open
cvergne opened this issue Apr 17, 2024 · 3 comments · May be fixed by #208
Open

JSON Error : return readable error instead of RuntimeException #207

cvergne opened this issue Apr 17, 2024 · 3 comments · May be fixed by #208

Comments

@cvergne
Copy link
Contributor

cvergne commented Apr 17, 2024

Hi,

I've noticed in the main Controller of the bundle, in case of JSON syntax error, a RuntimeException in thrown, which result to a 500 HTTP return code by Symfony with an "Internal Server Error" HTML response.

if (json_last_error() !== JSON_ERROR_NONE) {
throw new \RuntimeException('Invalid JSON received in POST body: '.json_last_error_msg());
}
if (!is_array($parsedBody)){
throw new \RuntimeException('Expecting associative array from request, got ' . gettype($parsedBody));
}

In my point of view, both cases should return a JSON response formatted as GraphQL response with errors, with a 400 HTTP code like :

{
    "errors": [
        {
            "message": "Invalid JSON received in POST body",
            "extensions": {
                "reason": "Syntax error"
            }
        }
    ]
}

or

{
    "errors": [
        {
            "message": "Invalid JSON received in POST body",
            "extensions": {
                "reason": "Expecting associative array from request, got string"
            }
        }
    ]
}

The trick is that we are outside of the GraphQL Server context, so throwing a GraphQLException here doesn't have the desired effect. But the solution could be to create an GraphQLException Listener then create a JsonResponse from an ExecutionResult+Error from the GraphQLException.

What do you think ?
Already tried it so I can work on it and submit a PR.

@mistraloz
Copy link
Collaborator

I totaly agree with you, it's would be very more clear (and usefull to monitor the error log).
But for error code I suggest to use 415 (Unsupported Media Type) if it's not parsable json, or 422 (Unprocessable Entity) if the json is not associative.

Copy link

This issue is stale because it has been open 6 months with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Oct 15, 2024
@cvergne
Copy link
Contributor Author

cvergne commented Oct 15, 2024

A comment to prevent closing as it just wait for review :)

@github-actions github-actions bot removed the stale label Oct 16, 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
2 participants