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

GraphQL login mutation doesn't throw detailed error message #5558

Closed
roelbeerens opened this issue Mar 20, 2020 · 4 comments · Fixed by #5625
Closed

GraphQL login mutation doesn't throw detailed error message #5558

roelbeerens opened this issue Mar 20, 2020 · 4 comments · Fixed by #5625
Labels
issue: enhancement Issue suggesting an enhancement to an existing feature severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve source: plugin:graphql Source is plugin/graphql package

Comments

@roelbeerens
Copy link
Contributor

Describe the bug
I noticed that the schema.graphql.js in the latest version of the User Permissions plugin is somewhat changed. The checkBadRequest function doesn't return an Apollo error anymore which results in a JWT error when the password and/or username is incorrect.

Steps to reproduce the behavior

  1. Go to config/schema.graphql.js
  2. Go to line 8-14
  3. Check the throw new Error
  4. This used to be a throw new ApolloError

Expected behavior
A clear error message when the username and/or password is incorrect when trying to log in.

System

  • Node.js version: 13.10.1
  • NPM version: 6.14.3
  • Strapi version: 3.0.0-beta.19.3
  • Database: MySQL / MariaDB
  • Operating system: MacOS
@alexandrebodin
Copy link
Member

alexandrebodin commented Mar 20, 2020

Hi, Yes we had to remove the ApolloError to avoid making the ApolloServer a dependency of the users-permissions plugin.

We won't be changing this right away as we need to handle error handling differently in our case (certainly passing and errors object to the graphql context with some errors like ApolloError and such + some Strapi specific errors).

If you are interested in contributing let us know ;)

@alexandrebodin alexandrebodin added severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve issue: enhancement Issue suggesting an enhancement to an existing feature labels Mar 20, 2020
@roelbeerens
Copy link
Contributor Author

Hi, thanks for the clear message. I do share your idea that your code should be as independent as possible. I'd love to help out. Is there already like something a error handler Strapi uses right now?

@lauriejim lauriejim added the source: plugin:graphql Source is plugin/graphql package label Mar 23, 2020
@roelbeerens
Copy link
Contributor Author

@alexandrebodin would you guys prefer a error handler within the plug-in, or using a global one somewhere in the core?

@alexandrebodin
Copy link
Member

Hi @roelbeerens we found out where the issue is coming from and it isn't only because of the ApolloError change. We are going to fix this on our side as it requires more work than expected.

Thank you for reporting this !

alexandrebodin added a commit that referenced this issue Mar 27, 2020
…estLogic

Fix checkBadrequest useless logic in user-permissions' graphql config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: enhancement Issue suggesting an enhancement to an existing feature severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve source: plugin:graphql Source is plugin/graphql package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants