Client formatting hooks

docs/composing_a_supergraph.md

@@ -42,11 +42,7 @@ client = GraphQL::Stitching::Client.new(
     mutation_name: "Mutation",
     subscription_name: "Subscription",
     visibility_profiles: nil, # ["public", "private", ...]
-    description_merger: ->(values_by_location, info) { values_by_location.values.join("\n") },
-    deprecation_merger: ->(values_by_location, info) { values_by_location.values.first },
-    default_value_merger: ->(values_by_location, info) { values_by_location.values.first },
-    directive_kwarg_merger: ->(values_by_location, info) { values_by_location.values.last },
-    root_entrypoints: {},
+    root_entrypoints: { "Query.ping" => "pings" },
   },
   locations: {
     # ...
@@ -62,47 +58,49 @@ client = GraphQL::Stitching::Client.new(
 
 - **`visibility_profiles:`**, an array of [visibility profiles](./visibility.md) that the supergraph responds to.
 
-- **`description_merger:`**, a [value merger function](#value-merger-functions) for merging element description strings from across locations.
-
-- **`deprecation_merger:`**, a [value merger function](#value-merger-functions) for merging element deprecation strings from across locations.
-
-- **`default_value_merger:`**, a [value merger function](#value-merger-functions) for merging argument default values from across locations.
-
-- **`directive_kwarg_merger:`**, a [value merger function](#value-merger-functions) for merging directive keyword arguments from across locations.
-
 - **`root_entrypoints:`**, a hash of root field names mapped to their entrypoint locations, see [overlapping root fields](#overlapping-root-fields) below.
 
-#### Value merger functions
+### Value mergers
 
-Static data values such as element descriptions and directive arguments must also merge across locations. By default, the first non-null value encountered for a given element attribute is used. A value merger function may customize this process by selecting a different value or computing a new one:
+Static data values such as element descriptions and directive arguments must also merge across locations. By default, the first non-null value encountered for a given element property is used. Value merger methods can be customized by defining them on your own `Client` class:
 
 ```ruby
-join_values_merger = ->(values_by_location, info) { values_by_location.values.compact.join("\n") }
-
-client = GraphQL::Stitching::Client.new(
-  composer_options: {
-    description_merger: join_values_merger,
-    deprecation_merger: join_values_merger,
-    default_value_merger: join_values_merger,
-    directive_kwarg_merger: join_values_merger,
-  },
-)
+class MyClient < GraphQL::Stitching::Client
+  def merge_descriptions(values_by_location, info)
+    # return a merged element description string from across locations...
+    values_by_location.each_value.join("\n")
+  end
+
+  def merge_deprecations(values_by_location, info)
+    # return a merged element deprecation string from across locations...
+  end
+
+  def merge_default_values(values_by_location, info)
+    # return a merged argument default value from across locations...
+  end
+
+  def merge_kwargs(values_by_location, info)
+    # return a merged directive keyword argument from across locations...
+  end
+end
+
+client = MyClient.new(locations: ...)
 ```
 
-A merger function receives `values_by_location` and `info` arguments; these provide possible values keyed by location and info about where in the schema these values were encountered:
+All merge functions receive `values_by_location` and `info` arguments; these provide possible values keyed by location and info about where in the schema these values were encountered. For example:
 
 ```ruby
 values_by_location = {
-  "users" => "A fabulous data type.",
-  "products" => "An excellent data type.",
+  "users" => "A fabulous data type description.",
+  "products" => "An excellent data type description.",
 }
 
-info = {
+info = GraphQL::Stitching::Formatter::Info.new(
   type_name: "Product",
   # field_name: ...,
   # argument_name: ...,
   # directive_name: ...,
-}
+)
 ```
 
 ### Cached supergraphs

docs/error_handling.md

@@ -4,20 +4,28 @@ 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 and return a formatted error message to appear in [GraphQL errors](https://spec.graphql.org/June2018/#sec-Errors) for the request.
+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: { ... })
 client.on_error do |request, err|
   # log the error
   Bugsnag.notify(err)
+end
+```
 
-  # return a formatted message for the public response
-  "Whoops, please contact support abount request '#{request.context[:request_id]}'"
+To modify the format of returned error messages that appear in [GraphQL errors](https://spec.graphql.org/June2018/#sec-Errors), extend `Client` and define your own `build_graphql_error` method:
+
+```ruby
+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
+  end
 end
 
-# Result:
-# { "errors" => [{ "message" => "Whoops, please contact support abount request '12345'" }] }
+client = MyClient.new(locations: { ... })
 ```
 
 ### Subgraph errors

lib/graphql/stitching.rb

@@ -71,6 +71,7 @@ def supports_visibility?
   end
 end
 
+require_relative "stitching/formatter"
 require_relative "stitching/directives"
 require_relative "stitching/supergraph"
 require_relative "stitching/client"

lib/graphql/stitching/client.rb

@@ -7,6 +7,8 @@ module Stitching
     # Client is an out-of-the-box helper that assembles all 
     # stitching components into a workflow that executes requests.
     class Client
+      include Formatter
+
       class << self
         def from_definition(schema, executables:)
           new(supergraph: Supergraph.from_definition(schema, executables: executables))
@@ -30,6 +32,7 @@ def initialize(locations: nil, supergraph: nil, composer_options: {})
         elsif supergraph
           supergraph
         else
+          composer_options = { formatter: self }.merge!(composer_options)
           composer = Composer.new(**composer_options)
           composer.perform(locations)
         end
@@ -50,16 +53,16 @@ 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])
+        error_result(request, [e.to_h])
       rescue StandardError => e
-        custom_message = @on_error.call(request, e) if @on_error
-        error_result(request, [{ "message" => custom_message || "An unexpected error occured." }])
+        @on_error.call(request, e) if @on_error
+        error_result(request, [build_graphql_error(request, e)])
       end
 
       def on_cache_read(&block)
@@ -77,33 +80,33 @@ def on_error(&block)
         @on_error = block
       end
 
+      def read_cached_plan(request)
+        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
+
       private
 
       def load_plan(request)
-        if @on_cache_read && plan_json = @on_cache_read.call(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
 
-        plan = request.plan
-
-        if @on_cache_write
-          @on_cache_write.call(request, JSON.generate(plan.as_json))
+        request.plan.tap do |plan|
+          write_cached_plan(request, plan)
         end
-
-        plan
       end
 
-      def error_result(request, errors)
-        public_errors = errors.map! do |e|
-          e.is_a?(Hash) ? e : e.to_h
-        end
-
-        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

lib/graphql/stitching/composer.rb

@@ -20,12 +20,6 @@ class Composer
         t.get_field("f").get_argument("a").default_value
       end
 
-      # @api private
-      BASIC_VALUE_MERGER = ->(values_by_location, _info) { values_by_location.values.find { !_1.nil? } }
-
-      # @api private
-      VISIBILITY_PROFILES_MERGER = ->(values_by_location, _info) { values_by_location.values.reduce(:&) }
-
       # @api private
       COMPOSITION_VALIDATORS = [
         ValidateInterfaces,
@@ -52,22 +46,14 @@ def initialize(
         mutation_name: "Mutation",
         subscription_name: "Subscription",
         visibility_profiles: [],
-        description_merger: nil,
-        deprecation_merger: nil,
-        default_value_merger: nil,
-        directive_kwarg_merger: nil,
-        root_field_location_selector: nil,
-        root_entrypoints: nil
+        root_entrypoints: nil,
+        formatter: nil
       )
         @query_name = query_name
         @mutation_name = mutation_name
         @subscription_name = subscription_name
-        @description_merger = description_merger || BASIC_VALUE_MERGER
-        @deprecation_merger = deprecation_merger || BASIC_VALUE_MERGER
-        @default_value_merger = default_value_merger || BASIC_VALUE_MERGER
-        @directive_kwarg_merger = directive_kwarg_merger || BASIC_VALUE_MERGER
-        @root_field_location_selector = root_field_location_selector
         @root_entrypoints = root_entrypoints || {}
+        @formatter = formatter || Formatter::Default
 
         @field_map = {}
         @resolver_map = {}
@@ -424,12 +410,12 @@ def build_merged_arguments(type_name, members_by_location, owner, field_name: ni
           end
 
           unless default_values_by_location.empty?
-            kwargs[:default_value] = @default_value_merger.call(default_values_by_location, {
+            kwargs[:default_value] = @formatter.merge_default_values(default_values_by_location, Formatter::Info.new(
               type_name: type_name,
               field_name: field_name,
               argument_name: argument_name,
               directive_name: directive_name,
-            }.tap(&:compact!))
+            ))
           end
 
           type = merge_value_types(type_name, value_types, argument_name: argument_name, field_name: field_name)
@@ -458,7 +444,7 @@ def build_merged_directives(type_name, members_by_location, owner, field_name: n
         end
 
         directives_by_name_location.each do |directive_name, directives_by_location|
-          kwarg_merger = @directive_kwarg_merger
+          kwarg_formatter = @formatter
           directive_class = @schema_directives[directive_name]
           next unless directive_class
 
@@ -481,19 +467,19 @@ 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_merger = VISIBILITY_PROFILES_MERGER
+              kwarg_formatter = Formatter::Default
             end
           end
 
           kwargs = kwarg_values_by_name_location.each_with_object({}) do |(kwarg_name, kwarg_values_by_location), memo|
-            memo[kwarg_name.to_sym] = kwarg_merger.call(kwarg_values_by_location, {
+            memo[kwarg_name.to_sym] = kwarg_formatter.merge_kwargs(kwarg_values_by_location, Formatter::Info.new(
               type_name: type_name,
               field_name: field_name,
               argument_name: argument_name,
               enum_value: enum_value,
               directive_name: directive_name,
               kwarg_name: kwarg_name,
-            }.tap(&:compact!))
+            ))
           end
 
           owner.directive(directive_class, **kwargs)
@@ -537,24 +523,24 @@ def merge_value_types(type_name, subgraph_types, field_name: nil, argument_name:
       # @!visibility private
       def merge_descriptions(type_name, members_by_location, field_name: nil, argument_name: nil, enum_value: nil)
         strings_by_location = members_by_location.each_with_object({}) { |(l, m), memo| memo[l] = m.description }
-        @description_merger.call(strings_by_location, {
+        @formatter.merge_descriptions(strings_by_location, Formatter::Info.new(
           type_name: type_name,
           field_name: field_name,
           argument_name: argument_name,
           enum_value: enum_value,
-        }.tap(&:compact!))
+        ))
       end
 
       # @!scope class
       # @!visibility private
       def merge_deprecations(type_name, members_by_location, field_name: nil, argument_name: nil, enum_value: nil)
         strings_by_location = members_by_location.each_with_object({}) { |(l, m), memo| memo[l] = m.deprecation_reason }
-        @deprecation_merger.call(strings_by_location, {
+        @formatter.merge_deprecations(strings_by_location, Formatter::Info.new(
           type_name: type_name,
           field_name: field_name,
           argument_name: argument_name,
           enum_value: enum_value,
-        }.tap(&:compact!))
+        ))
       end
 
       # @!scope class
@@ -629,15 +615,7 @@ def select_root_field_locations(schema)
             next unless root_field_locations.length > 1
 
             root_field_path = "#{root_type.graphql_name}.#{root_field_name}"
-            target_location = if @root_field_location_selector && @root_entrypoints.empty?
-              Warning.warn("Composer option `root_field_location_selector` is deprecated and will be removed.")
-              @root_field_location_selector.call(root_field_locations, {
-                type_name: root_type.graphql_name,
-                field_name: root_field_name,
-              })
-            else
-              @root_entrypoints[root_field_path] || root_field_locations.last
-            end
+            target_location = @root_entrypoints[root_field_path] || root_field_locations.last
 
             unless root_field_locations.include?(target_location)
               raise CompositionError, "Invalid `root_entrypoints` configuration: `#{root_field_path}` has no `#{target_location}` location."