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

Add GraphQL::Execution::Errors module #142

Merged
merged 3 commits into from
Apr 15, 2021

Conversation

AleksandrSeleznev
Copy link
Contributor

Summary

@ArthurZaharov suggested to implement these GraphQL::Execution::Errors module in order to be able to handle errors.

How it works

We can handle errors with rescue_from method. For example, for ActiveRecord::RecordNotFound:

rescue_from(ActiveRecord::RecordNotFound) do |_err, _obj, _args, _ctx, field|
    raise GraphQL::ExecutionError.new("#{field.type.unwrap.graphql_name} not found",
                                      extensions: { message: "Not Found", status: 404, code: :not_found })
  end

Test plan

N/A

Review notes

While reviewing pull-request (especially when it's your pull-request),
please make sure that:

  • you understand what problem is solved by PR and how is it solved
  • new tests are in place, no redundant tests
  • DB schema changes reflect new migrations
  • newly introduced DB fields have indexes and constraints
  • there are no missed files (migrations, view templates)
  • required ENV variables added and described in .env.example and added to Heroku
  • associated Heroku review app works correctly with introduced changes

Deploy notes

N/A

References

N/A

@timurvafin timurvafin temporarily deployed to rails-base-graphql-pr-142 April 2, 2021 19:24 Inactive
@EvgeniyEsaulkov
Copy link
Contributor

Can you please add a test for some graphql query or mutation that illustrates how it work?

@AleksandrSeleznev
Copy link
Contributor Author

Can you please add a test for some graphql query or mutation that illustrates how it work?

@EvgeniyEsaulkov is it necessary to implement separated spec for this?
I haven't found any query or mutation with find method for some Model, only find_by.

@EvgeniyEsaulkov
Copy link
Contributor

I haven't found any query or mutation with find method for some Model, only find_by

That's why I asked :-)
What are the benefits to add code that probably will used never? And in case we will use .find in some query or mutation in the future, would it better to specify error explicitely in application schema?

@EvgeniyEsaulkov
Copy link
Contributor

@AleksandrSeleznev I discussed addding of errors to Application Schema with FE developers - they are not happy with that. So I think we can use your solution for now - every project we started on the base of rails-base-graphql-api uses .find method.

@EvgeniyEsaulkov EvgeniyEsaulkov temporarily deployed to rails-base-graphql-pr-142 April 7, 2021 10:19 Inactive
@EvgeniyEsaulkov EvgeniyEsaulkov temporarily deployed to rails-base-graphql-pr-142 April 15, 2021 04:56 Inactive
@EvgeniyEsaulkov EvgeniyEsaulkov merged commit 8d7ad87 into master Apr 15, 2021
@EvgeniyEsaulkov EvgeniyEsaulkov deleted the ImplementExecutionErrorsModule branch April 15, 2021 05:04
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.

None yet

4 participants