Merge branch 'master' into add-default-validate-timeout-value

vithushar · web-flow · commit 0a17a1e27483 · 2025-03-10T12:03:41.000-04:00
diff --git a/guides/authorization/visibility.md b/guides/authorization/visibility.md
@@ -151,8 +151,8 @@ For big schemas, this can be a worthwhile speed-up.
 - `Visibility` speeds up Rails app boot because it doesn't require all types to be loaded during boot and only loads types as they are used by queries.
 - `Visibility` supports predefined, reusable visibility profiles which speeds up queries using complicated `visible?` checks.
 - `Visibility` hides types differently in a few edge cases:
-  - Previously, `Warden` hide interface and union types which had no possible types. `Visibility` doesn't check possible types (in order to support performance improvements), so those types must return `false` for `visible?` in the same cases where all possible types were hidden. Otherwise, that interface or union type will be visible but have no possible types.
-  - Some other thing, see TODO
+  - Previously, `Warden` hid interface and union types which had no possible types. `Visibility` doesn't check possible types (in order to support performance improvements), so those types must return `false` for `visible?` in the same cases where all possible types were hidden. Otherwise, that interface or union type will be visible but have no possible types.
+  - When an object type is connected to the schema as a field return type or a union member, and also implements and interface, if the object type's _other_ connection(s) to the schema are hidden, then it won't appear as an implementer of that interface unless it's registered with `orphan_types` (either by the schema or interface). `Warden` used a "global" map of types so it could discover object types in this case, but `Visibility` doesn't have that global map. (Since time of writing, `Visibility` _does_ have some global type tracking, so maybe this could be fixed.)
 - When `Visibility` is used, several (Ruby-level) Schema introspection methods don't work because the caches they draw on haven't been calculated (`Schema.references_to`, `Schema.union_memberships`). If you're using these, please get in touch so that we can find a way forward.
 
 ### Migration Mode
diff --git a/lib/graphql/analysis.rb b/lib/graphql/analysis.rb
@@ -6,11 +6,16 @@
 require "graphql/analysis/max_query_complexity"
 require "graphql/analysis/query_depth"
 require "graphql/analysis/max_query_depth"
-require "timeout"
-
 module GraphQL
   module Analysis
     AST = self
+
+    class TimeoutError < AnalysisError
+      def initialize(...)
+        super("Timeout on validation of query")
+      end
+    end
+
     module_function
     # Analyze a multiplex, and all queries within.
     # Multiplex analyzers are ran for all queries, keeping state.
@@ -61,13 +66,11 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
           if !analyzers_to_run.empty?
             visitor = GraphQL::Analysis::Visitor.new(
               query: query,
-              analyzers: analyzers_to_run
+              analyzers: analyzers_to_run,
+              timeout: query.validate_timeout_remaining,
             )
 
-            # `nil` or `0` causes no timeout
-            Timeout::timeout(query.validate_timeout_remaining) do
-              visitor.visit
-            end
+            visitor.visit
 
             if !visitor.rescued_errors.empty?
               return visitor.rescued_errors
@@ -79,8 +82,8 @@ def analyze_query(query, analyzers, multiplex_analyzers: [])
           []
         end
       end
-    rescue Timeout::Error
-      [GraphQL::AnalysisError.new("Timeout on validation of query")]
+    rescue TimeoutError => err
+      [err]
     rescue GraphQL::UnauthorizedError, GraphQL::ExecutionError
       # This error was raised during analysis and will be returned the client before execution
       []
diff --git a/lib/graphql/analysis/visitor.rb b/lib/graphql/analysis/visitor.rb
@@ -10,7 +10,7 @@ module Analysis
     #
     # @see {GraphQL::Analysis::Analyzer} AST Analyzers for queries
     class Visitor < GraphQL::Language::StaticVisitor
-      def initialize(query:, analyzers:)
+      def initialize(query:, analyzers:, timeout:)
         @analyzers = analyzers
         @path = []
         @object_types = []
@@ -24,6 +24,11 @@ def initialize(query:, analyzers:)
         @types = query.types
         @response_path = []
         @skip_stack = [false]
+        @timeout_time = if timeout
+          Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) + timeout
+        else
+          Float::INFINITY
+        end
         super(query.selected_operation)
       end
 
@@ -72,28 +77,25 @@ def response_path
         module_eval <<-RUBY, __FILE__, __LINE__
         def call_on_enter_#{node_type}(node, parent)
           @analyzers.each do |a|
-            begin
-              a.on_enter_#{node_type}(node, parent, self)
-            rescue AnalysisError => err
-              @rescued_errors << err
-            end
+            a.on_enter_#{node_type}(node, parent, self)
+          rescue AnalysisError => err
+            @rescued_errors << err
           end
         end
 
         def call_on_leave_#{node_type}(node, parent)
           @analyzers.each do |a|
-            begin
-              a.on_leave_#{node_type}(node, parent, self)
-            rescue AnalysisError => err
-              @rescued_errors << err
-            end
+            a.on_leave_#{node_type}(node, parent, self)
+          rescue AnalysisError => err
+            @rescued_errors << err
           end
         end
 
         RUBY
       end
 
       def on_operation_definition(node, parent)
+        check_timeout
         object_type = @schema.root_type_for_operation(node.operation_type)
         @object_types.push(object_type)
         @path.push("#{node.operation_type}#{node.name ? " #{node.name}" : ""}")
@@ -104,31 +106,27 @@ def on_operation_definition(node, parent)
         @path.pop
       end
 
-      def on_fragment_definition(node, parent)
-        on_fragment_with_type(node) do
-          @path.push("fragment #{node.name}")
-          @in_fragment_def = false
-          call_on_enter_fragment_definition(node, parent)
-          super
-          @in_fragment_def = false
-          call_on_leave_fragment_definition(node, parent)
-        end
-      end
-
       def on_inline_fragment(node, parent)
-        on_fragment_with_type(node) do
-          @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
-          @skipping = @skip_stack.last || skip?(node)
-          @skip_stack << @skipping
-
-          call_on_enter_inline_fragment(node, parent)
-          super
-          @skipping = @skip_stack.pop
-          call_on_leave_inline_fragment(node, parent)
+        check_timeout
+        object_type = if node.type
+          @types.type(node.type.name)
+        else
+          @object_types.last
         end
+        @object_types.push(object_type)
+        @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
+        @skipping = @skip_stack.last || skip?(node)
+        @skip_stack << @skipping
+        call_on_enter_inline_fragment(node, parent)
+        super
+        @skipping = @skip_stack.pop
+        call_on_leave_inline_fragment(node, parent)
+        @object_types.pop
+        @path.pop
       end
 
       def on_field(node, parent)
+        check_timeout
         @response_path.push(node.alias || node.name)
         parent_type = @object_types.last
         # This could be nil if the previous field wasn't found:
@@ -156,6 +154,7 @@ def on_field(node, parent)
       end
 
       def on_directive(node, parent)
+        check_timeout
         directive_defn = @schema.directives[node.name]
         @directive_definitions.push(directive_defn)
         call_on_enter_directive(node, parent)
@@ -165,6 +164,7 @@ def on_directive(node, parent)
       end
 
       def on_argument(node, parent)
+        check_timeout
         argument_defn = if (arg = @argument_definitions.last)
           arg_type = arg.type.unwrap
           if arg_type.kind.input_object?
@@ -190,6 +190,7 @@ def on_argument(node, parent)
       end
 
       def on_fragment_spread(node, parent)
+        check_timeout
         @path.push("... #{node.name}")
         @skipping = @skip_stack.last || skip?(node)
         @skip_stack << @skipping
@@ -267,16 +268,10 @@ def skip?(ast_node)
         !dir.empty? && !GraphQL::Execution::DirectiveChecks.include?(dir, query)
       end
 
-      def on_fragment_with_type(node)
-        object_type = if node.type
-          @types.type(node.type.name)
-        else
-          @object_types.last
+      def check_timeout
+        if Process.clock_gettime(Process::CLOCK_MONOTONIC, :float_second) > @timeout_time
+          raise GraphQL::Analysis::TimeoutError
         end
-        @object_types.push(object_type)
-        yield(node)
-        @object_types.pop
-        @path.pop
       end
     end
   end
diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb
@@ -473,7 +473,7 @@ def continue_value(value, field, is_non_null, ast_node, result_name, selection_r
                 # When this comes from a list item, use the parent object:
                 parent_type = selection_result.is_a?(GraphQLResultArray) ? selection_result.graphql_parent.graphql_result_type : selection_result.graphql_result_type
                 # This block is called if `result_name` is not dead. (Maybe a previous invalid nil caused it be marked dead.)
-                err = parent_type::InvalidNullError.new(parent_type, field, value, ast_node)
+                err = parent_type::InvalidNullError.new(parent_type, field, ast_node)
                 schema.type_error(err, context)
               end
             else
diff --git a/lib/graphql/invalid_null_error.rb b/lib/graphql/invalid_null_error.rb
@@ -9,16 +9,12 @@ class InvalidNullError < GraphQL::Error
     # @return [GraphQL::Field] The field which failed to return a value
     attr_reader :field
 
-    # @return [nil, GraphQL::ExecutionError] The invalid value for this field
-    attr_reader :value
-
     # @return [GraphQL::Language::Nodes::Field] the field where the error occurred
     attr_reader :ast_node
 
-    def initialize(parent_type, field, value, ast_node)
+    def initialize(parent_type, field, ast_node)
       @parent_type = parent_type
       @field = field
-      @value = value
       @ast_node = ast_node
       super("Cannot return null for non-nullable field #{@parent_type.graphql_name}.#{@field.graphql_name}")
     end
diff --git a/lib/graphql/language/lexer.rb b/lib/graphql/language/lexer.rb
@@ -13,17 +13,21 @@ def initialize(graphql_str, filename: nil, max_tokens: nil)
         @pos = nil
         @max_tokens = max_tokens || Float::INFINITY
         @tokens_count = 0
+        @finished = false
       end
 
-      def eos?
-        @scanner.eos?
+      def finished?
+        @finished
       end
 
       attr_reader :pos, :tokens_count
 
       def advance
         @scanner.skip(IGNORE_REGEXP)
-        return false if @scanner.eos?
+        if @scanner.eos?
+          @finished = true
+          return false
+        end
         @tokens_count += 1
         if @tokens_count > @max_tokens
           raise_parse_error("This query is too large to execute.")
diff --git a/lib/graphql/language/parser.rb b/lib/graphql/language/parser.rb
@@ -110,7 +110,7 @@ def document
           # Only ignored characters is not a valid document
           raise GraphQL::ParseError.new("Unexpected end of document", nil, nil, @graphql_str)
         end
-        while !@lexer.eos?
+        while !@lexer.finished?
           defns << definition
         end
         Document.new(pos: 0, definitions: defns, filename: @filename, source: self)
diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb
@@ -821,8 +821,8 @@ def subscription_execution_strategy(new_subscription_execution_strategy = nil, d
 
       attr_writer :validate_timeout
 
-      def validate_timeout(new_validate_timeout = :not_provided)
-        if new_validate_timeout != :not_provided
+      def validate_timeout(new_validate_timeout = NOT_CONFIGURED)
+        if !NOT_CONFIGURED.equal?(new_validate_timeout)
           @validate_timeout = new_validate_timeout
         elsif defined?(@validate_timeout)
           @validate_timeout
diff --git a/lib/graphql/schema/resolver.rb b/lib/graphql/schema/resolver.rb
@@ -22,7 +22,6 @@ class Resolver
       include Schema::Member::GraphQLTypeNames
       # Really we only need description & comment from here, but:
       extend Schema::Member::BaseDSLMethods
-      extend Member::BaseDSLMethods::ConfigurationExtension
       extend GraphQL::Schema::Member::HasArguments
       extend GraphQL::Schema::Member::HasValidators
       include Schema::Member::HasPath
@@ -404,6 +403,11 @@ def extensions
           end
         end
 
+        def inherited(child_class)
+          child_class.description(description)
+          super
+        end
+
         private
 
         attr_reader :own_extensions
diff --git a/lib/graphql/static_validation/rules/fields_will_merge.rb b/lib/graphql/static_validation/rules/fields_will_merge.rb
@@ -345,7 +345,7 @@ def find_fields_and_fragments(selections, owner_type:, parents:, fields:, fragme
             fields << Field.new(node, definition, owner_type, parents)
           when GraphQL::Language::Nodes::InlineFragment
             fragment_type = node.type ? @types.type(node.type.name) : owner_type
-            find_fields_and_fragments(node.selections, parents: [*parents, fragment_type], owner_type: owner_type, fields: fields, fragment_spreads: fragment_spreads) if fragment_type
+            find_fields_and_fragments(node.selections, parents: [*parents, fragment_type], owner_type: fragment_type, fields: fields, fragment_spreads: fragment_spreads) if fragment_type
           when GraphQL::Language::Nodes::FragmentSpread
             fragment_spreads << FragmentSpread.new(node.name, parents)
           end
diff --git a/lib/graphql/tracing/new_relic_trace.rb b/lib/graphql/tracing/new_relic_trace.rb
@@ -13,6 +13,9 @@ module Tracing
     #     # Optional, use the operation name to set the new relic transaction name:
     #     # trace_with GraphQL::Tracing::NewRelicTrace, set_transaction_name: true
     #   end
+    #
+    # @example Installing without trace events for `authorized?` or `resolve_type` calls
+    #   trace_with GraphQL::Tracing::NewRelicTrace, trace_authorized: false, trace_resolve_type: false
     module NewRelicTrace
       # @param set_transaction_name [Boolean] If true, the GraphQL operation name will be used as the transaction name.
       #   This is not advised if you run more than one query per HTTP request, for example, with `graphql-client` or multiplexing.
diff --git a/spec/graphql/analysis_spec.rb b/spec/graphql/analysis_spec.rb
diff --git a/spec/graphql/language/parser_spec.rb b/spec/graphql/language/parser_spec.rb
diff --git a/spec/graphql/non_null_type_spec.rb b/spec/graphql/non_null_type_spec.rb
diff --git a/spec/graphql/schema/resolver_spec.rb b/spec/graphql/schema/resolver_spec.rb
diff --git a/spec/graphql/static_validation/rules/fields_will_merge_spec.rb b/spec/graphql/static_validation/rules/fields_will_merge_spec.rb
