refine it.

Author: gmac

lib/graphql/stitching/client.rb

@@ -53,69 +53,67 @@ def execute(raw_query = nil, query: nil, variables: nil, operation_name: nil, co
 
         if validate
           validation_errors = request.validate
-          return error_result(request, validation_errors) unless validation_errors.empty?
+          return error_result(request, validation_errors.map(&:to_h)) unless validation_errors.empty?
         end
 
         load_plan(request)
         request.execute
-      rescue GraphQL::ParseError, GraphQL::ExecutionError => e
-        error_result(request, [e])
       rescue StandardError => e
-        error_result(request, [handle_error(request, e)])
+        @on_error.call(request, e) if @on_error
+        error_result(request, [build_graphql_error(request, e)])
       end
 
       def on_cache_read(&block)
-        Warning.warn("Using `on_cache_read` is deprecated and will be removed. Use `read_cached_plan` instead.")
         raise ArgumentError, "A cache read block is required." unless block_given?
         @on_cache_read = block
       end
 
       def on_cache_write(&block)
-        Warning.warn("Using `on_cache_write` is deprecated and will be removed. Use `write_cached_plan` instead.")
         raise ArgumentError, "A cache write block is required." unless block_given?
         @on_cache_write = block
       end
 
       def on_error(&block)
-        Warning.warn("Using `on_error` is deprecated and will be removed. Use `handle_error` instead.")
         raise ArgumentError, "An error handler block is required." unless block_given?
         @on_error = block
       end
 
       def read_cached_plan(request)
-        @on_cache_read.call(request) if @on_cache_read
+        if @on_cache_read && (plan_json = @on_cache_read.call(request))
+          Plan.from_json(JSON.parse(plan_json))
+        end
       end
 
       def write_cached_plan(request, plan)
         @on_cache_write.call(request, JSON.generate(plan.as_json)) if @on_cache_write
       end
 
-      def handle_error(request, error)
-        message = @on_error.call(request, error) if @on_error
-        message ||= "An unexpected error occured."
-        { "message" => message }
+      def build_graphql_error(request, err)
+        case err
+        when GraphQL::ParseError, GraphQL::ExecutionError
+          err.to_h
+        else
+          { "message" => "An unexpected error occured." }
+        end
       end
 
       private
 
       def load_plan(request)
-        if (plan_json = read_cached_plan(request))
-          plan = Plan.from_json(JSON.parse(plan_json))
+        plan = read_cached_plan(request)
 
-          # only use plans referencing current resolver versions
-          if plan.ops.all? { |op| !op.resolver || @supergraph.resolvers_by_version[op.resolver] }
-            return request.plan(plan)
-          end
+        # only use plans referencing current resolver versions
+        if plan && plan.ops.all? { |op| !op.resolver || @supergraph.resolvers_by_version[op.resolver] }
+          return request.plan(plan)
         end
 
         request.plan.tap do |plan|
           write_cached_plan(request, plan)
         end
       end
 
-      def error_result(request, errors)
-        public_errors = errors.map! { |e| e.is_a?(Hash) ? e : e.to_h }
-        GraphQL::Query::Result.new(query: request, values: { "errors" => public_errors })
+      def error_result(request, graphql_errors)
+        GraphQL::Query::Result.new(query: request, values: { "errors" => graphql_errors })
       end
     end
   end

test/graphql/stitching/client_test.rb

@@ -332,14 +332,15 @@ def test_errors_are_handled_by_default
   end
 
   def test_errors_trigger_hooks_that_may_return_a_custom_message
+    called = false
     client = GraphQL::Stitching::Client.new(locations: {
       products: {
         schema: Schemas::Example::Products,
       }
     })
 
-    client.on_error do |req, _err|
-      "An error occured. Request id: #{req.context[:request_id]}"
+    client.on_error do |_req, _err|
+      called = true
     end
 
     result = client.execute(
@@ -349,11 +350,12 @@ def test_errors_trigger_hooks_that_may_return_a_custom_message
     )
 
     expected_errors = [{
-      "message" => "An error occured. Request id: R2d2c3P0",
+      "message" => "An unexpected error occured.",
     }]
 
     assert_nil result["data"]
     assert_equal expected_errors, result["errors"]
+    assert called
   end
 
   def test_passes_composer_options_through_to_composition