more stuff.

Author: gmac

docs/error_handling.md

@@ -4,7 +4,7 @@ Failed stitching requests can be tricky to debug because it's not always obvious
 
 ### Supergraph errors
 
-When exceptions happen while executing requests within the stitching layer, they will be rescued by the stitching client and trigger an `on_error` hook. You should add your stack's error reporting here:
+When exceptions happen while executing requests within the stitching layer, they will be rescued by the stitching client and trigger an `on_error` hook. You can add your stack's error reporting here:
 
 ```ruby
 client = GraphQL::Stitching::Client.new(locations: { ... })
@@ -20,7 +20,7 @@ To modify the format of returned error messages that appear in [GraphQL errors](
 class MyClient < GraphQL::Stitching::Client
   def build_graphql_error(request, err)
     graphql_error = super(request, err)
-    graphql_error["message"] << ". Contact support about Request ID #{request.context[:request_id]}"
+    graphql_error["message"] << " Contact support about Request ID #{request.context[:request_id]}"
     graphql_error
   end
 end

lib/graphql/stitching/client.rb

@@ -58,6 +58,8 @@ def execute(raw_query = nil, query: nil, variables: nil, operation_name: nil, co
 
         load_plan(request)
         request.execute
+      rescue GraphQL::ParseError, GraphQL::ExecutionError => e
+        error_result(request, [e.to_h])
       rescue StandardError => e
         @on_error.call(request, e) if @on_error
         error_result(request, [build_graphql_error(request, e)])

lib/graphql/stitching/composer.rb

@@ -11,11 +11,6 @@ module Stitching
     # representing various graph locations and merges them into one 
     # combined Supergraph that is validated for integrity.
     class Composer
-      # @api private
-      class DefaultFormatter
-        extend Formatter
-      end
-
       # @api private
       NO_DEFAULT_VALUE = begin
         t = Class.new(GraphQL::Schema::Object) do
@@ -58,7 +53,7 @@ def initialize(
         @mutation_name = mutation_name
         @subscription_name = subscription_name
         @root_entrypoints = root_entrypoints || {}
-        @formatter = formatter || DefaultFormatter
+        @formatter = formatter || Formatter::Default
 
         @field_map = {}
         @resolver_map = {}
@@ -472,7 +467,7 @@ def build_merged_directives(type_name, members_by_location, owner, field_name: n
 
             if (profiles = kwarg_values_by_name_location["profiles"])
               @visibility_profiles.merge(profiles.each_value.reduce(&:|))
-              kwarg_formatter = DefaultFormatter
+              kwarg_formatter = Formatter::Default
             end
           end
 

lib/graphql/stitching/formatter.rb

@@ -6,6 +6,10 @@
 module GraphQL
   module Stitching
     module Formatter
+      class Default
+        extend Formatter
+      end
+
       class Info
         attr_reader :type_name, :field_name, :argument_name, :enum_value, :directive_name, :kwarg_name
 
@@ -50,13 +54,8 @@ def merge_kwargs(values_by_location, info)
         end
       end
 
-      def build_graphql_error(request, err)
-        case err
-        when GraphQL::ParseError, GraphQL::ExecutionError
-          err.to_h
-        else
-          { "message" => "An unexpected error occured." }
-        end
+      def build_graphql_error(_request, _err)
+        { "message" => "An unexpected error occured." }
       end
     end
   end

test/graphql/stitching/client/class_extension_test.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require_relative "../../../schemas/example"
+
+describe "GraphQL::Stitching::Client" do
+  class MyClient < GraphQL::Stitching::Client
+    def merge_descriptions(values_by_location, _info)
+      values_by_location.values.join("/")
+    end
+
+    def build_graphql_error(request, err)
+      { "message" => "Contact support about request #{request.context[:request_id]}." }
+    end
+  end
+
+  def test_client_acts_as_composition_formatter
+    alpha = %|
+      """
+      a
+      """
+      type Query { a:Boolean }
+    |
+    bravo = %|
+      """
+      b
+      """
+      type Query { b:Boolean }
+    |
+
+    client = MyClient.new(locations: {
+      "alpha" => { schema: GraphQL::Schema.from_definition(alpha) },
+      "bravo" => { schema: GraphQL::Schema.from_definition(bravo) },
+    })
+    
+    assert_equal "a/b", client.supergraph.schema.query.description
+  end
+
+  def test_client_builds_graphql_errors
+    client = MyClient.new(locations: {
+      products: { schema: Schemas::Example::Products },
+    })
+
+    result = client.execute(
+      query: "query { invalidSelection }",
+      context: { request_id: "R2d2c3P0" },
+      validate: false
+    )
+
+    expected_errors = [{
+      "message" => "Contact support about request R2d2c3P0.",
+    }]
+
+    assert_nil result["data"]
+    assert_equal expected_errors, result["errors"]
+  end
+
+  def test_client_from_definition_builds_specific_class
+    alpha = %|
+      type T { id:ID! a:String }
+      type Query { a(id:ID!):T @stitch(key: "id") }
+    |
+    bravo = %|
+      type T { id:ID! b:String }
+      type Query { b(id:ID!):T @stitch(key: "id") }
+    |
+
+    sdl = compose_definitions({ "alpha" => alpha, "bravo" => bravo }).to_definition
+    client = MyClient.from_definition(sdl, executables: {
+      "alpha" => Proc.new {},
+      "bravo" => Proc.new {},
+    })
+    assert client.is_a?(MyClient)
+  end
+end

test/graphql/stitching/client_test.rb renamed to test/graphql/stitching/client/client_test.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "test_helper"
-require_relative "../../schemas/example"
+require_relative "../../../schemas/example"
 
 describe "GraphQL::Stitching::Client" do
   def setup_client
@@ -297,7 +297,7 @@ def test_caching_hooks_receive_request_context
     assert_equal context[:key], write_context
   end
 
-  def test_invalid_query
+  def test_query_with_static_validation_errors
     client = GraphQL::Stitching::Client.new(locations: {
       products: {
         schema: Schemas::Example::Products,
@@ -314,48 +314,24 @@ def test_invalid_query
     assert_equal expected_errors, result["errors"].map { _1.slice("message", "path") }
   end
 
-  def test_errors_are_handled_by_default
-    client = GraphQL::Stitching::Client.new(locations: {
-      products: {
-        schema: Schemas::Example::Products,
-      }
-    })
-
-    result = client.execute('query { invalidSelection }', validate: false)
-
-    expected_errors = [{
-      "message" => "An unexpected error occured.",
-    }]
-
-    assert_nil result["data"]
-    assert_equal expected_errors, result["errors"]
-  end
-
-  def test_errors_trigger_hooks_that_may_return_a_custom_message
+  def test_stitching_errors_are_handled
     called = false
     client = GraphQL::Stitching::Client.new(locations: {
       products: {
         schema: Schemas::Example::Products,
       }
     })
+    client.on_error { called = true }
 
-    client.on_error do |_req, _err|
-      called = true
-    end
-
-    result = client.execute(
-      query: "query { invalidSelection }",
-      context: { request_id: "R2d2c3P0" },
-      validate: false
-    )
+    result = client.execute('query { invalidSelection }', validate: false)
 
     expected_errors = [{
       "message" => "An unexpected error occured.",
     }]
 
+    assert called
     assert_nil result["data"]
     assert_equal expected_errors, result["errors"]
-    assert called
   end
 
   def test_passes_composer_options_through_to_composition
@@ -377,50 +353,4 @@ def test_errors_for_composer_options_given_with_precomposed_supergraph
       )
     end
   end
-
-  class MyClient < GraphQL::Stitching::Client
-    def merge_descriptions(values_by_location, _info)
-      values_by_location.values.join("/")
-    end
-  end
-
-  def test_from_definition_builds_with_invocation_class
-    alpha = %|
-      type T { id:ID! a:String }
-      type Query { a(id:ID!):T @stitch(key: "id") }
-    |
-    bravo = %|
-      type T { id:ID! b:String }
-      type Query { b(id:ID!):T @stitch(key: "id") }
-    |
-
-    sdl = compose_definitions({ "alpha" => alpha, "bravo" => bravo }).to_definition
-    client = MyClient.from_definition(sdl, executables: {
-      "alpha" => Proc.new {},
-      "bravo" => Proc.new {},
-    })
-    assert client.is_a?(MyClient)
-  end
-
-  def test_custom_client_acts_as_composition_formatter
-    alpha = %|
-      """
-      a
-      """
-      type Query { a:Boolean }
-    |
-    bravo = %|
-      """
-      b
-      """
-      type Query { b:Boolean }
-    |
-
-    client = MyClient.new(locations: {
-      "alpha" => { schema: GraphQL::Schema.from_definition(alpha) },
-      "bravo" => { schema: GraphQL::Schema.from_definition(bravo) },
-    })
-    
-    assert_equal "a/b", client.supergraph.schema.query.description
-  end
 end