GraphQL: Improve session token error messages #5753
Conversation
Fixes the session token related error messages during GraphQL operations. If any authentication error were thrown, it was not correctly handled by the GraphQL express middleware, and ended responding the request with a JSON parsing error.
src/GraphQL/ParseGraphQLServer.js
Outdated
| app.use(this.config.graphQLPath, (err, req, res, next) => { | ||
| if (err && err instanceof Parse.Error) { | ||
| res.json( | ||
| new ApolloError({ |
There was a problem hiding this comment.
Wouldn't it be better to use this.parseGraphQLSchema.handleError() function (https://github.com/parse-community/parse-server/blob/master/src/GraphQL/ParseGraphQLSchema.js#L102) or at least the same logic?
There was a problem hiding this comment.
I've refactored the conversion of the Parse.Error to ApolloError. Since the handleError method includes some logging and throws the error, I thought it was better to just refactor the middleware to use this conversion and set the error as response, because that's all we need for now.
Codecov Report
@@ Coverage Diff @@
## master #5753 +/- ##
==========================================
+ Coverage 93.62% 93.66% +0.04%
==========================================
Files 145 146 +1
Lines 10211 10236 +25
==========================================
+ Hits 9560 9588 +28
+ Misses 651 648 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I am wondering if wouldn't be better just add the middlewares.handleParseErrors like the REST api does here instead of using this whole thing.
Also, can you please add some test cases?
src/middlewares.js
Outdated
| @@ -319,6 +319,9 @@ export function handleParseErrors(err, req, res, next) { | |||
| case Parse.Error.OBJECT_NOT_FOUND: | |||
| httpStatus = 404; | |||
| break; | |||
| case Parse.Error.INVALID_SESSION_TOKEN: | |||
There was a problem hiding this comment.
humm.. I'm not sure... I think it can break some apps. Let's just leave 400 as it was before.
* GraphQL: Improve session token error message Fixes the session token related error messages during GraphQL operations. If any authentication error were thrown, it was not correctly handled by the GraphQL express middleware, and ended responding the request with a JSON parsing error. * Refactor handleError usage * Use handleParseErrors middleware to handle invalid session token error * fix: Status code 400 when session token is invalid * fix: Undo handleParseErrors middleware change
Fixes the session token related error messages during GraphQL operations. If any authentication error were thrown, it was not correctly handled by the GraphQL express middleware, and ended responding the request with a JSON parsing error.