diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-21 14:42:07 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-05 21:47:42 +0300 |
commit | c443133e779c4c508b9c6429dd4ba623d64f03f1 (patch) | |
tree | 390ec7649b0875f059c0962458319fe14ed8d34c | |
parent | aa4b1ae71260720b47695b8a256134f20280f61a (diff) |
Handle exceptions outside the GraphQL schema
This allows us to report JSON parse exceptions to clients and ignore
them in sentry.
-rw-r--r-- | app/controllers/graphql_controller.rb | 38 | ||||
-rw-r--r-- | lib/gitlab/graphql.rb | 5 | ||||
-rw-r--r-- | lib/gitlab/graphql/variables.rb | 37 | ||||
-rw-r--r-- | spec/controllers/graphql_controller_spec.rb | 15 |
4 files changed, 72 insertions, 23 deletions
diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index ef258bf07cb..0a1cf169aca 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -5,7 +5,7 @@ class GraphqlController < ApplicationController before_action :check_graphql_feature_flag! def execute - variables = ensure_hash(params[:variables]) + variables = Gitlab::Graphql::Variables.new(params[:variables]).to_h query = params[:query] operation_name = params[:operationName] context = { @@ -15,35 +15,31 @@ class GraphqlController < ApplicationController render json: result end + rescue_from StandardError do |exception| + log_exception(exception) + + render_error("Internal server error") + end + + rescue_from Gitlab::Graphql::Variables::Invalid do |exception| + render_error(exception.message, status: :unprocessable_entity) + end + private # Overridden from the ApplicationController to make the response look like # a GraphQL response. That is nicely picked up in Graphiql. def render_404 - error = { errors: [ message: "Not found" ] } + render_error("Not found!", status: :not_found) + end + + def render_error(message, status: 500) + error = { errors: [message: message] } - render json: error, status: :not_found + render json: error, status: status end def check_graphql_feature_flag! render_404 unless Feature.enabled?(:graphql) end - - # Handle form data, JSON body, or a blank value - def ensure_hash(ambiguous_param) - case ambiguous_param - when String - if ambiguous_param.present? - ensure_hash(JSON.parse(ambiguous_param)) - else - {} - end - when Hash, ActionController::Parameters - ambiguous_param - when nil - {} - else - raise ArgumentError, "Unexpected parameter: #{ambiguous_param}" - end - end end diff --git a/lib/gitlab/graphql.rb b/lib/gitlab/graphql.rb new file mode 100644 index 00000000000..04a89432230 --- /dev/null +++ b/lib/gitlab/graphql.rb @@ -0,0 +1,5 @@ +module Gitlab + module Graphql + StandardGraphqlError = Class.new(StandardError) + end +end diff --git a/lib/gitlab/graphql/variables.rb b/lib/gitlab/graphql/variables.rb new file mode 100644 index 00000000000..ffbaf65b512 --- /dev/null +++ b/lib/gitlab/graphql/variables.rb @@ -0,0 +1,37 @@ +module Gitlab + module Graphql + class Variables + Invalid = Class.new(Gitlab::Graphql::StandardGraphqlError) + + def initialize(param) + @param = param + end + + def to_h + ensure_hash(@param) + end + + private + + # Handle form data, JSON body, or a blank value + def ensure_hash(ambiguous_param) + case ambiguous_param + when String + if ambiguous_param.present? + ensure_hash(JSON.parse(ambiguous_param)) + else + {} + end + when Hash, ActionController::Parameters + ambiguous_param + when nil + {} + else + raise Invalid, "Unexpected parameter: #{ambiguous_param}" + end + rescue JSON::ParserError => e + raise Invalid.new(e) + end + end + end +end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index d6689dbc3c6..1449036e148 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' describe GraphqlController do describe 'execute' do + let(:user) { nil } + before do sign_in(user) if user @@ -39,17 +41,26 @@ describe GraphqlController do is_expected.to eq('echo' => '"Simon" says: test success') end end + + context 'invalid variables' do + it 'returns an error' do + run_test_query!(variables: "This is not JSON") + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors'].first['message']).not_to be_nil + end + end end # Chosen to exercise all the moving parts in GraphqlController#execute - def run_test_query! + def run_test_query!(variables: { 'text' => 'test success' }) query = <<~QUERY query Echo($text: String) { echo(text: $text) } QUERY - post :execute, query: query, operationName: 'Echo', variables: { 'text' => 'test success' } + post :execute, query: query, operationName: 'Echo', variables: variables end def query_response |