Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

revert revert "Allow custom handling of runtime query errors" #808

Open
joshribakoff-sm opened this issue Jul 22, 2022 · 1 comment · May be fixed by #780
Open

revert revert "Allow custom handling of runtime query errors" #808

joshribakoff-sm opened this issue Jul 22, 2022 · 1 comment · May be fixed by #780

Comments

@joshribakoff-sm
Copy link

#736 reverted something the community aligned behind changing, without addressing the concerns of the community. The author of said revert PR seems to be misreading their own link, since the spec seems pretty clear that responses that contain a null data property are recommended to contain status code 200, and nowhere does it say these responses must contain 500 which is @acao's claim.

Perhaps as a compromise, there can be a way to opt into (seemingly non spec compliant) behavior of forcing all errors to be 500 errors, however for all intents and purposes, this behavior completely breaks clients error handling behavior as described in detail in #427

diff --git a/node_modules/express-graphql/index.js b/node_modules/express-graphql/index.js
index 3bc78dc..d72a19d 100644
--- a/node_modules/express-graphql/index.js
+++ b/node_modules/express-graphql/index.js
@@ -174,14 +174,6 @@ function graphqlHTTP(options) {
                 result = { data: undefined, errors: error.graphqlErrors };
             }
         }
-        // If no data was included in the result, that indicates a runtime query
-        // error, indicate as such with a generic status code.
-        // Note: Information about the error itself will still be contained in
-        // the resulting JSON payload.
-        // https://graphql.github.io/graphql-spec/#sec-Data
-        if (response.statusCode === 200 && result.data == null) {
-            response.statusCode = 500;
-        }
         // Format any encountered errors.
         const formattedResult = {
             ...result,

This issue body was partially generated by patch-package.

@MatthiasKunnen MatthiasKunnen linked a pull request Aug 4, 2022 that will close this issue
@mihairadulescu
Copy link

This is destryoing Apollo Client integration !!!!!!!!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants