diff --git a/CHANGELOG.md b/CHANGELOG.md index 8980c2e..9deb379 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] * Added/Changed/Deprecated/Removed/Fixed/Security: YOUR CHANGE HERE +* Changed: do not throw "Cannot return null for non-nullable" error when errors are manually rendered +* Added: allow to specify type error hooks in router ## [3.1.0](2025-05-26) diff --git a/docs/components/routes.md b/docs/components/routes.md index d11f55c..04840d3 100644 --- a/docs/components/routes.md +++ b/docs/components/routes.md @@ -174,3 +174,29 @@ end ``` If you want to access raw graphql schema, you can call `GraphqlRouter.graphql_schema(:mobile)` + +## _before_type_error_ + +Sometimes it's handy to log or perform other actions when a GraphQL type error occurs. To do so, specify the Router.before_type_error hook: + +```ruby +GraphqlRails::Router.draw do + before_type_error do |type_error, context| + MyCustomExceptionLogger.call(type_error) + end +end +``` + +Note: This might log `GraphQL::InvalidNullError` errors that are not visible to the end user. If you do not want that, use Router.after_type_error. + +## _after_type_error_ + +Sometimes it's handy to perform logging or other actions after a GraphQL type error occurs. To do so, specify the Router.after_type_error hook: + +```ruby +GraphqlRails::Router.draw do + after_type_error do |type_error, context| + MyCustomExceptionLogger.call(type_error) + end +end +``` diff --git a/lib/graphql_rails/controller.rb b/lib/graphql_rails/controller.rb index d528a4b..22e82a6 100644 --- a/lib/graphql_rails/controller.rb +++ b/lib/graphql_rails/controller.rb @@ -81,6 +81,7 @@ def render(object_or_errors) object = errors.empty? ? object_or_errors : nil graphql_request.errors = errors + graphql_request.context[:rendered_errors] = true unless errors.empty? graphql_request.object_to_return = object end diff --git a/lib/graphql_rails/router.rb b/lib/graphql_rails/router.rb index 7cd02db..72b6289 100644 --- a/lib/graphql_rails/router.rb +++ b/lib/graphql_rails/router.rb @@ -13,6 +13,7 @@ module GraphqlRails class Router RAW_ACTION_NAMES = %i[ use rescue_from query_analyzer instrument cursor_encoder default_max_page_size tracer trace_with + before_type_error after_type_error ].freeze def self.draw(&block) diff --git a/lib/graphql_rails/router/schema.rb b/lib/graphql_rails/router/schema.rb new file mode 100644 index 0000000..469d6bd --- /dev/null +++ b/lib/graphql_rails/router/schema.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'graphql/pagination/active_record_relation_connection' +require 'graphql_rails/decorator/relation_decorator' +require 'graphql_rails/router/plain_cursor_encoder' + +module GraphqlRails + class Router + # Builds GraphQL::Schema based on previously defined GraphQL data. + # Also handles type error hooks (before_type_error, after_type_error) + # and implements logic for skipping null errors. + class Schema < ::GraphQL::Schema + use GraphQL::Schema::Visibility + + connections.add( + GraphqlRails::Decorator::RelationDecorator, + GraphQL::Pagination::ActiveRecordRelationConnection + ) + + cursor_encoder(Router::PlainCursorEncoder) + + def self.before_type_error(&block) + @before_type_error = block if block_given? + @before_type_error + end + + def self.after_type_error(&block) + @after_type_error = block if block_given? + @after_type_error + end + + def self.type_error(type_error, context) + before_type_error&.call(type_error, context) + + return nil if type_error.is_a?(GraphQL::InvalidNullError) && context[:rendered_errors] + + super.tap do + after_type_error&.call(type_error, context) + end + end + end + end +end diff --git a/lib/graphql_rails/router/schema_builder.rb b/lib/graphql_rails/router/schema_builder.rb index cc65981..cc405f5 100644 --- a/lib/graphql_rails/router/schema_builder.rb +++ b/lib/graphql_rails/router/schema_builder.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'graphql_rails/router/schema' + module GraphqlRails class Router # builds GraphQL::Schema based on previously defined grahiti data @@ -31,14 +33,7 @@ def call # rubocop:disable Metrics/MethodLength def define_schema_class(query_type, mutation_type, subscription_type, raw) - Class.new(GraphQL::Schema) do - use GraphQL::Schema::Visibility - - connections.add( - GraphqlRails::Decorator::RelationDecorator, - GraphQL::Pagination::ActiveRecordRelationConnection - ) - cursor_encoder(Router::PlainCursorEncoder) + Class.new(GraphqlRails::Router::Schema) do raw.each { |action| send(action[:name], *action[:args], **action[:kwargs], &action[:block]) } query(query_type) if query_type diff --git a/spec/lib/graphql_rails/controller/handle_controller_error_spec.rb b/spec/lib/graphql_rails/controller/handle_controller_error_spec.rb index d72ec9b..14f21ec 100644 --- a/spec/lib/graphql_rails/controller/handle_controller_error_spec.rb +++ b/spec/lib/graphql_rails/controller/handle_controller_error_spec.rb @@ -16,23 +16,29 @@ let(:error) { StandardError.new('error') } before do + allow(context).to receive(:[]=).with(:rendered_errors, true) allow(controller).to receive(:render).and_call_original end context 'when error is a GraphQL::ExecutionError' do let(:error) { GraphQL::ExecutionError.new('error') } - it 'raises error' do + it 'raises error and does not set "rendered_errors" flag in context', :aggregate_failures do expect { call }.to raise_error(error) + expect(context).not_to have_received(:[]=).with(:rendered_errors, true) end end context 'when error is not a GraphQL::ExecutionError' do it 'renders SystemError' do call - expect(controller).to have_received(:render).with(error: GraphqlRails::SystemError.new(error)) end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when controller has custom error handler' do @@ -51,9 +57,13 @@ it 'renders error' do call - expect(controller).to have_received(:render).with(error: error.message) end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when custom handler is a method' do @@ -71,9 +81,13 @@ def custom_handler it 'renders error' do call - expect(controller).to have_received(:render).with(error: 'custom error') end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when custom handler raises error' do @@ -87,9 +101,13 @@ def custom_handler it 'renders SystemError' do call - expect(controller).to have_received(:render).with(error: GraphqlRails::SystemError) end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end end end diff --git a/spec/lib/graphql_rails/controller_spec.rb b/spec/lib/graphql_rails/controller_spec.rb index 8eac17c..55373ee 100644 --- a/spec/lib/graphql_rails/controller_spec.rb +++ b/spec/lib/graphql_rails/controller_spec.rb @@ -201,6 +201,10 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare let(:inputs) { {} } let(:context) { instance_double(GraphQL::Query::Context) } + before do + allow(context).to receive(:[]=).with(:rendered_errors, true) + end + describe '.action' do subject(:action) { DummyInputsController.controller_configuration.action(:create) } @@ -342,6 +346,11 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare expect { call }.not_to raise_error end + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end + it 'adds first error in to context' do call expect(context).to have_received(:add_error).once @@ -370,6 +379,11 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare ) end + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end + it 'returns error from hook' do call expect(context).to have_received(:add_error).with(ExecutionError.new('ups!')) @@ -414,12 +428,22 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare call expect(context).to have_received(:add_error).with(ExecutionError.new('bam!')) end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when result was rendered' do it 'returns rendered result' do expect(call).to eq 'Hello' end + + it 'does not set "rendered_errors" flag in context' do + call + expect(context).not_to have_received(:[]=).with(:rendered_errors, anything) + end end context 'when error was raised' do @@ -437,6 +461,11 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare it 'returns nil' do expect(call).to be_nil end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when handled error was raised' do @@ -454,6 +483,11 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare it 'returns nil' do expect(call).to be_nil end + + it 'sets "rendered_errors" flag in context' do + call + expect(context).to have_received(:[]=).with(:rendered_errors, true) + end end context 'when rendering was not triggered' do @@ -462,6 +496,11 @@ class DummyMultipleBeforeActionsChildController < DummyMultipleBeforeActionsPare it 'render last value' do expect(call).to eq 'Hello without render!' end + + it 'does not set "rendered_errors" flag in context' do + call + expect(context).not_to have_received(:[]=).with(:rendered_errors, anything) + end end end end diff --git a/spec/lib/graphql_rails/router_spec.rb b/spec/lib/graphql_rails/router_spec.rb index b8d2701..c457882 100644 --- a/spec/lib/graphql_rails/router_spec.rb +++ b/spec/lib/graphql_rails/router_spec.rb @@ -183,6 +183,20 @@ class RouterDummyUsersController < GraphqlRails::Controller end end + describe '#before_type_error' do + it 'allows to set before_type_error hook' do + router.before_type_error { 'my before hook' } + expect(router.before_type_error.first[:block]).to be_a(Proc) + end + end + + describe '#after_type_error' do + it 'allows to set after_type_error hook' do + router.after_type_error { 'my after hook' } + expect(router.after_type_error.first[:block]).to be_a(Proc) + end + end + describe '#scope' do context 'with named scopes without module' do before do