From b735fdffd5a008fad4404e86d3da7b4304c12324 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 28 Jan 2025 14:50:47 -0500 Subject: [PATCH 1/5] Add Backtrace implementation which uses existing runtime structures --- lib/graphql/backtrace/table.rb | 70 ++++++++++++++++++- lib/graphql/execution/interpreter/runtime.rb | 10 +-- .../interpreter/runtime/graphql_result.rb | 15 ++-- 3 files changed, 85 insertions(+), 10 deletions(-) diff --git a/lib/graphql/backtrace/table.rb b/lib/graphql/backtrace/table.rb index c94cd0de9a..8c83148c50 100644 --- a/lib/graphql/backtrace/table.rb +++ b/lib/graphql/backtrace/table.rb @@ -36,7 +36,75 @@ def to_backtrace private def rows - @rows ||= build_rows(@context, rows: [HEADERS], top: true) + @rows ||= begin + query = @context.query + query_ctx = query.context + runtime_inst = query_ctx.namespace(:interpreter_runtime)[:runtime] + result = runtime_inst.instance_variable_get(:@response) + rows = [] + result_path = [] + last_part = nil + path = @context.path + path.each do |path_part| + value = value_at(runtime_inst, result_path) + + if result_path.empty? + name = query.selected_operation.operation_type || "query" + if (n = query.selected_operation_name) + name += " #{n}" + end + args = query.variables + else + name = result.graphql_field.path + args = result.graphql_arguments + end + + object = result.graphql_parent ? result.graphql_parent.graphql_application_value : result.graphql_application_value + object = object.object.inspect + + rows << [ + result.ast_node.position.join(":"), + name, + "#{object}", + args.to_h.inspect, + Backtrace::InspectResult.inspect_result(value), + ] + + result_path << path_part + if path_part == path.last + last_part = path_part + else + result = result[path_part] + end + end + + if last_part + object = result.graphql_application_value.object.inspect + ast_node = result.graphql_selections.find { |s| s.alias == last_part || s.name == last_part } + field_defn = query.get_field(result.graphql_result_type, ast_node.name) + if field_defn + args = query.arguments_for(ast_node, field_defn).to_h + field_path = field_defn.path + if ast_node.alias + field_path += " as #{ast_node.alias}" + end + else + args = {} + field_path = "#{result.graphql_result_type.graphql_name}.#{last_part}" + end + + rows << [ + ast_node.position.join(":"), + field_path, + "#{object}", + args.inspect, + Backtrace::InspectResult.inspect_result(@override_value) + ] + end + rows << HEADERS + rows.reverse! + rows + end end # @return [String] diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 5a3c0ce76c..5adc8988df 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -74,7 +74,7 @@ def run_eager runtime_object = root_type.wrap(query.root_value, context) runtime_object = schema.sync_lazy(runtime_object) is_eager = root_op_type == "mutation" - @response = GraphQLResultHash.new(nil, root_type, runtime_object, nil, false, root_operation.selections, is_eager) + @response = GraphQLResultHash.new(nil, root_type, runtime_object, nil, false, root_operation.selections, is_eager, root_operation, nil, nil) st = get_current_runtime_state st.current_result = @response @@ -85,7 +85,7 @@ def run_eager call_method_on_directives(:resolve, runtime_object, root_operation.directives) do # execute query level directives each_gathered_selections(@response) do |selections, is_selection_array| if is_selection_array - selection_response = GraphQLResultHash.new(nil, root_type, runtime_object, nil, false, selections, is_eager) + selection_response = GraphQLResultHash.new(nil, root_type, runtime_object, nil, false, selections, is_eager, root_operation, nil, nil) final_response = @response else selection_response = @response @@ -609,11 +609,11 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select after_lazy(object_proxy, ast_node: ast_node, field: field, owner_object: owner_object, arguments: arguments, trace: false, result_name: result_name, result: selection_result, runtime_state: runtime_state) do |inner_object, runtime_state| continue_value = continue_value(inner_object, field, is_non_null, ast_node, result_name, selection_result) if HALT != continue_value - response_hash = GraphQLResultHash.new(result_name, current_type, continue_value, selection_result, is_non_null, next_selections, false) + response_hash = GraphQLResultHash.new(result_name, current_type, continue_value, selection_result, is_non_null, next_selections, false, ast_node, arguments, field) set_result(selection_result, result_name, response_hash, true, is_non_null) each_gathered_selections(response_hash) do |selections, is_selection_array| if is_selection_array - this_result = GraphQLResultHash.new(result_name, current_type, continue_value, selection_result, is_non_null, selections, false) + this_result = GraphQLResultHash.new(result_name, current_type, continue_value, selection_result, is_non_null, selections, false, ast_node, arguments, field) final_result = response_hash else this_result = response_hash @@ -634,7 +634,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select # This is true for objects, unions, and interfaces use_dataloader_job = !inner_type.unwrap.kind.input? inner_type_non_null = inner_type.non_null? - response_list = GraphQLResultArray.new(result_name, current_type, owner_object, selection_result, is_non_null, next_selections, false) + response_list = GraphQLResultArray.new(result_name, current_type, owner_object, selection_result, is_non_null, next_selections, false, ast_node, arguments, field) set_result(selection_result, result_name, response_list, true, is_non_null) idx = nil list_value = begin diff --git a/lib/graphql/execution/interpreter/runtime/graphql_result.rb b/lib/graphql/execution/interpreter/runtime/graphql_result.rb index 3792860380..d6fa4d8f17 100644 --- a/lib/graphql/execution/interpreter/runtime/graphql_result.rb +++ b/lib/graphql/execution/interpreter/runtime/graphql_result.rb @@ -5,7 +5,10 @@ module Execution class Interpreter class Runtime module GraphQLResult - def initialize(result_name, result_type, application_value, parent_result, is_non_null_in_parent, selections, is_eager) + def initialize(result_name, result_type, application_value, parent_result, is_non_null_in_parent, selections, is_eager, ast_node, graphql_arguments, graphql_field) # rubocop:disable Metrics/ParameterLists + @ast_node = ast_node + @graphql_arguments = graphql_arguments + @graphql_field = graphql_field @graphql_parent = parent_result @graphql_application_value = application_value @graphql_result_type = result_type @@ -31,14 +34,14 @@ def build_path(path_array) attr_accessor :graphql_dead attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent, - :graphql_application_value, :graphql_result_type, :graphql_selections, :graphql_is_eager + :graphql_application_value, :graphql_result_type, :graphql_selections, :graphql_is_eager, :ast_node, :graphql_arguments, :graphql_field # @return [Hash] Plain-Ruby result data (`@graphql_metadata` contains Result wrapper objects) attr_accessor :graphql_result_data end class GraphQLResultHash - def initialize(_result_name, _result_type, _application_value, _parent_result, _is_non_null_in_parent, _selections, _is_eager) + def initialize(_result_name, _result_type, _application_value, _parent_result, _is_non_null_in_parent, _selections, _is_eager, _ast_node, _graphql_arguments, graphql_field) # rubocop:disable Metrics/ParameterLists super @graphql_result_data = {} end @@ -126,7 +129,7 @@ def merge_into(into_result) class GraphQLResultArray include GraphQLResult - def initialize(_result_name, _result_type, _application_value, _parent_result, _is_non_null_in_parent, _selections, _is_eager) + def initialize(_result_name, _result_type, _application_value, _parent_result, _is_non_null_in_parent, _selections, _is_eager, _ast_node, _graphql_arguments, graphql_field) # rubocop:disable Metrics/ParameterLists super @graphql_result_data = [] end @@ -168,6 +171,10 @@ def set_child_result(idx, value) def values (@graphql_metadata || @graphql_result_data) end + + def [](idx) + (@graphql_metadata || @graphql_result_data)[idx] + end end end end From b001c112f8390750eeb24f61d4feaa949af12b1c Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 28 Jan 2025 15:04:26 -0500 Subject: [PATCH 2/5] Stop using trace error handler --- lib/graphql/backtrace.rb | 3 +- lib/graphql/backtrace/table.rb | 4 +- lib/graphql/backtrace/trace.rb | 20 ++++---- lib/graphql/execution/interpreter/runtime.rb | 50 ++++++++++++-------- lib/graphql/schema.rb | 6 +++ 5 files changed, 49 insertions(+), 34 deletions(-) diff --git a/lib/graphql/backtrace.rb b/lib/graphql/backtrace.rb index cbd1c5f478..a927e171d2 100644 --- a/lib/graphql/backtrace.rb +++ b/lib/graphql/backtrace.rb @@ -24,7 +24,8 @@ class Backtrace def_delegators :to_a, :each, :[] def self.use(schema_defn) - schema_defn.trace_with(self::Trace) + schema_defn.using_backtrace = true + # schema_defn.trace_with(self::Trace) end def initialize(context, value: nil) diff --git a/lib/graphql/backtrace/table.rb b/lib/graphql/backtrace/table.rb index 8c83148c50..ab611a3a6d 100644 --- a/lib/graphql/backtrace/table.rb +++ b/lib/graphql/backtrace/table.rb @@ -38,13 +38,13 @@ def to_backtrace def rows @rows ||= begin query = @context.query - query_ctx = query.context + query_ctx = @context runtime_inst = query_ctx.namespace(:interpreter_runtime)[:runtime] result = runtime_inst.instance_variable_get(:@response) rows = [] result_path = [] last_part = nil - path = @context.path + path = @context.current_path path.each do |path_part| value = value_at(runtime_inst, result_path) diff --git a/lib/graphql/backtrace/trace.rb b/lib/graphql/backtrace/trace.rb index ca99022ea9..9571c5d1e3 100644 --- a/lib/graphql/backtrace/trace.rb +++ b/lib/graphql/backtrace/trace.rb @@ -45,16 +45,16 @@ def execute_field_lazy(field:, query:, ast_node:, arguments:, object:) def execute_multiplex(multiplex:) super - rescue StandardError => err - # This is an unhandled error from execution, - # Re-raise it with a GraphQL trace. - potential_context = @__backtrace_last_context - if potential_context.is_a?(GraphQL::Query::Context) || - potential_context.is_a?(Backtrace::Frame) - raise TracedError.new(err, potential_context) - else - raise - end + # rescue StandardError => err + # # This is an unhandled error from execution, + # # Re-raise it with a GraphQL trace. + # potential_context = @__backtrace_last_context + # if potential_context.is_a?(GraphQL::Query::Context) || + # potential_context.is_a?(Backtrace::Frame) + # raise TracedError.new(err, potential_context) + # else + # raise + # end end private diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 5adc8988df..05ec8ce1e4 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -574,7 +574,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select r = begin current_type.coerce_result(value, context) rescue StandardError => err - schema.handle_or_reraise(context, err) + query.handle_or_reraise(err) end set_result(selection_result, result_name, r, false, is_non_null) r @@ -638,31 +638,39 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select set_result(selection_result, result_name, response_list, true, is_non_null) idx = nil list_value = begin - value.each do |inner_value| - idx ||= 0 - this_idx = idx - idx += 1 - if use_dataloader_job - @dataloader.append_job do + begin + value.each do |inner_value| + idx ||= 0 + this_idx = idx + idx += 1 + if use_dataloader_job + @dataloader.append_job do + resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, owner_type, was_scoped, runtime_state) + end + else resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, owner_type, was_scoped, runtime_state) end - else - resolve_list_item(inner_value, inner_type, inner_type_non_null, ast_node, field, owner_object, arguments, this_idx, response_list, owner_type, was_scoped, runtime_state) end - end - response_list - rescue NoMethodError => err - # Ruby 2.2 doesn't have NoMethodError#receiver, can't check that one in this case. (It's been EOL since 2017.) - if err.name == :each && (err.respond_to?(:receiver) ? err.receiver == value : true) - # This happens when the GraphQL schema doesn't match the implementation. Help the dev debug. - raise ListResultFailedError.new(value: value, field: field, path: current_path) - else - # This was some other NoMethodError -- let it bubble to reveal the real error. - raise + response_list + rescue NoMethodError => err + # Ruby 2.2 doesn't have NoMethodError#receiver, can't check that one in this case. (It's been EOL since 2017.) + if err.name == :each && (err.respond_to?(:receiver) ? err.receiver == value : true) + # This happens when the GraphQL schema doesn't match the implementation. Help the dev debug. + raise ListResultFailedError.new(value: value, field: field, path: current_path) + else + # This was some other NoMethodError -- let it bubble to reveal the real error. + raise + end + rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => ex_err + ex_err + rescue StandardError => err + begin + query.handle_or_reraise(err) + rescue GraphQL::ExecutionError => ex_err + ex_err + end end - rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => ex_err - ex_err rescue StandardError => err begin query.handle_or_reraise(err) diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index a59d6f8e0f..69977db7d7 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -1118,8 +1118,14 @@ def error_handlers } end + # @api private + attr_accessor :using_backtrace + # @api private def handle_or_reraise(context, err) + if context[:backtrace] || using_backtrace + err = GraphQL::Backtrace::TracedError.new(err, context) + end handler = Execution::Errors.find_handler_for(self, err.class) if handler obj = context[:current_object] From f7e86066dd70369aa56a253936f06e5747453a1e Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 28 Jan 2025 15:10:20 -0500 Subject: [PATCH 3/5] Remove trace-related Backtrace code --- lib/graphql/backtrace.rb | 2 - lib/graphql/backtrace/trace.rb | 93 ------------------------------ lib/graphql/backtrace/tracer.rb | 80 ------------------------- lib/graphql/execution/multiplex.rb | 4 -- lib/graphql/query.rb | 8 --- lib/graphql/schema.rb | 26 +-------- lib/graphql/tracing/trace.rb | 2 +- 7 files changed, 3 insertions(+), 212 deletions(-) delete mode 100644 lib/graphql/backtrace/trace.rb delete mode 100644 lib/graphql/backtrace/tracer.rb diff --git a/lib/graphql/backtrace.rb b/lib/graphql/backtrace.rb index a927e171d2..501941b6de 100644 --- a/lib/graphql/backtrace.rb +++ b/lib/graphql/backtrace.rb @@ -2,8 +2,6 @@ require "graphql/backtrace/inspect_result" require "graphql/backtrace/table" require "graphql/backtrace/traced_error" -require "graphql/backtrace/tracer" -require "graphql/backtrace/trace" module GraphQL # Wrap unhandled errors with {TracedError}. # diff --git a/lib/graphql/backtrace/trace.rb b/lib/graphql/backtrace/trace.rb deleted file mode 100644 index 9571c5d1e3..0000000000 --- a/lib/graphql/backtrace/trace.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true -module GraphQL - class Backtrace - module Trace - def initialize(*args, **kwargs, &block) - @__backtrace_contexts = {} - @__backtrace_last_context = nil - super - end - - def validate(query:, validate:) - if query.multiplex - push_query_backtrace_context(query) - end - super - end - - def analyze_query(query:) - if query.multiplex # missing for stand-alone static validation - push_query_backtrace_context(query) - end - super - end - - def execute_query(query:) - push_query_backtrace_context(query) - super - end - - def execute_query_lazy(query:, multiplex:) - query ||= multiplex.queries.first - push_query_backtrace_context(query) - super - end - - def execute_field(field:, query:, ast_node:, arguments:, object:) - push_field_backtrace_context(field, query, ast_node, arguments, object) - super - end - - def execute_field_lazy(field:, query:, ast_node:, arguments:, object:) - push_field_backtrace_context(field, query, ast_node, arguments, object) - super - end - - def execute_multiplex(multiplex:) - super - # rescue StandardError => err - # # This is an unhandled error from execution, - # # Re-raise it with a GraphQL trace. - # potential_context = @__backtrace_last_context - # if potential_context.is_a?(GraphQL::Query::Context) || - # potential_context.is_a?(Backtrace::Frame) - # raise TracedError.new(err, potential_context) - # else - # raise - # end - end - - private - - def push_query_backtrace_context(query) - push_data = query - push_key = [] - @__backtrace_contexts[push_key] = push_data - @__backtrace_last_context = push_data - end - - def push_field_backtrace_context(field, query, ast_node, arguments, object) - push_key = query.context[:current_path] - push_storage = @__backtrace_contexts - parent_frame = push_storage[push_key[0..-2]] - - if parent_frame.is_a?(GraphQL::Query) - parent_frame = parent_frame.context - end - - push_data = Frame.new( - query: query, - path: push_key, - ast_node: ast_node, - field: field, - object: object, - arguments: arguments, - parent_frame: parent_frame, - ) - push_storage[push_key] = push_data - @__backtrace_last_context = push_data - end - - end - end -end diff --git a/lib/graphql/backtrace/tracer.rb b/lib/graphql/backtrace/tracer.rb deleted file mode 100644 index 45e0b1e85a..0000000000 --- a/lib/graphql/backtrace/tracer.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true -module GraphQL - class Backtrace - # TODO this is not fiber-friendly - module Tracer - module_function - - # Implement the {GraphQL::Tracing} API. - def trace(key, metadata) - case key - when "lex", "parse" - # No context here, don't have a query yet - nil - when "execute_multiplex", "analyze_multiplex" - # No query context yet - nil - when "validate", "analyze_query", "execute_query", "execute_query_lazy" - push_key = [] - if (query = metadata[:query]) || ((queries = metadata[:queries]) && (query = queries.first)) - push_data = query - multiplex = query.multiplex - elsif (multiplex = metadata[:multiplex]) - push_data = multiplex.queries.first - end - when "execute_field", "execute_field_lazy" - query = metadata[:query] - multiplex = query.multiplex - push_key = query.context[:current_path] - parent_frame = multiplex.context[:graphql_backtrace_contexts][push_key[0..-2]] - - if parent_frame.is_a?(GraphQL::Query) - parent_frame = parent_frame.context - end - - push_data = Frame.new( - query: query, - path: push_key, - ast_node: metadata[:ast_node], - field: metadata[:field], - object: metadata[:object], - arguments: metadata[:arguments], - parent_frame: parent_frame, - ) - else - # Custom key, no backtrace data for this - nil - end - - if push_data && multiplex - push_storage = multiplex.context[:graphql_backtrace_contexts] ||= {} - push_storage[push_key] = push_data - multiplex.context[:last_graphql_backtrace_context] = push_data - end - - if key == "execute_multiplex" - multiplex_context = metadata[:multiplex].context - begin - yield - rescue StandardError => err - # This is an unhandled error from execution, - # Re-raise it with a GraphQL trace. - potential_context = multiplex_context[:last_graphql_backtrace_context] - - if potential_context.is_a?(GraphQL::Query::Context) || - potential_context.is_a?(Backtrace::Frame) - raise TracedError.new(err, potential_context) - else - raise - end - ensure - multiplex_context.delete(:graphql_backtrace_contexts) - multiplex_context.delete(:last_graphql_backtrace_context) - end - else - yield - end - end - end - end -end diff --git a/lib/graphql/execution/multiplex.rb b/lib/graphql/execution/multiplex.rb index 1e7df471f2..582d379f1f 100644 --- a/lib/graphql/execution/multiplex.rb +++ b/lib/graphql/execution/multiplex.rb @@ -35,10 +35,6 @@ def initialize(schema:, queries:, context:, max_complexity:) @current_trace = @context[:trace] || schema.new_trace(multiplex: self) @dataloader = @context[:dataloader] ||= @schema.dataloader_class.new @tracers = schema.tracers + (context[:tracers] || []) - # Support `context: {backtrace: true}` - if context[:backtrace] && !@tracers.include?(GraphQL::Backtrace::Tracer) - @tracers << GraphQL::Backtrace::Tracer - end @max_complexity = max_complexity end end diff --git a/lib/graphql/query.rb b/lib/graphql/query.rb index eb253ec242..752d9219f4 100644 --- a/lib/graphql/query.rb +++ b/lib/graphql/query.rb @@ -127,14 +127,6 @@ def initialize(schema, query_string = nil, query: nil, document: nil, context: n context_tracers = (context ? context.fetch(:tracers, []) : []) @tracers = schema.tracers + context_tracers - # Support `ctx[:backtrace] = true` for wrapping backtraces - if context && context[:backtrace] && !@tracers.include?(GraphQL::Backtrace::Tracer) - if schema.trace_class <= GraphQL::Tracing::CallLegacyTracers - context_tracers += [GraphQL::Backtrace::Tracer] - @tracers << GraphQL::Backtrace::Tracer - end - end - if !context_tracers.empty? && !(schema.trace_class <= GraphQL::Tracing::CallLegacyTracers) raise ArgumentError, "context[:tracers] are not supported without `trace_with(GraphQL::Tracing::CallLegacyTracers)` in the schema configuration, please add it." end diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 69977db7d7..b4c01dda89 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -167,9 +167,6 @@ def trace_class(new_class = nil) mods.each { |mod| new_class.include(mod) } new_class.include(DefaultTraceClass) trace_mode(:default, new_class) - backtrace_class = Class.new(new_class) - backtrace_class.include(GraphQL::Backtrace::Trace) - trace_mode(:default_backtrace, backtrace_class) end trace_class_for(:default, build: true) end @@ -216,11 +213,6 @@ def build_trace_mode(mode) const_set(:DefaultTrace, Class.new(base_class) do include DefaultTraceClass end) - when :default_backtrace - schema_base_class = trace_class_for(:default, build: true) - const_set(:DefaultTraceBacktrace, Class.new(schema_base_class) do - include(GraphQL::Backtrace::Trace) - end) else # First, see if the superclass has a custom-defined class for this. # Then, if it doesn't, use this class's default trace @@ -1455,22 +1447,8 @@ def new_trace(mode: nil, **options) target = options[:query] || options[:multiplex] mode ||= target && target.context[:trace_mode] - trace_mode = if mode - mode - elsif target && target.context[:backtrace] - if default_trace_mode != :default - raise ArgumentError, "Can't use `context[:backtrace]` with a custom default trace mode (`#{dm.inspect}`)" - else - own_trace_modes[:default_backtrace] ||= build_trace_mode(:default_backtrace) - options_trace_mode = :default - :default_backtrace - end - else - default_trace_mode - end - - options_trace_mode ||= trace_mode - base_trace_options = trace_options_for(options_trace_mode) + trace_mode = mode || default_trace_mode + base_trace_options = trace_options_for(trace_mode) trace_options = base_trace_options.merge(options) trace_class_for_mode = trace_class_for(trace_mode, build: true) trace_class_for_mode.new(**trace_options) diff --git a/lib/graphql/tracing/trace.rb b/lib/graphql/tracing/trace.rb index ef9a3722a8..7d3b86e683 100644 --- a/lib/graphql/tracing/trace.rb +++ b/lib/graphql/tracing/trace.rb @@ -8,7 +8,7 @@ module Tracing # "Trace modes" are subclasses of this with custom tracing modules mixed in. # # A trace module may implement any of the methods on `Trace`, being sure to call `super` - # to continue any tracing hooks and call the actual runtime behavior. See {GraphQL::Backtrace::Trace} for example. + # to continue any tracing hooks and call the actual runtime behavior. # class Trace # @param multiplex [GraphQL::Execution::Multiplex, nil] From d9bf3a2279e883205c2fce38db639c233698814d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 28 Jan 2025 15:15:57 -0500 Subject: [PATCH 4/5] remove dead Backtrace code --- lib/graphql/backtrace.rb | 17 ----- lib/graphql/backtrace/inspect_result.rb | 38 ----------- lib/graphql/backtrace/table.rb | 83 ++++++++++--------------- 3 files changed, 32 insertions(+), 106 deletions(-) delete mode 100644 lib/graphql/backtrace/inspect_result.rb diff --git a/lib/graphql/backtrace.rb b/lib/graphql/backtrace.rb index 501941b6de..c97543d1fe 100644 --- a/lib/graphql/backtrace.rb +++ b/lib/graphql/backtrace.rb @@ -1,5 +1,4 @@ # frozen_string_literal: true -require "graphql/backtrace/inspect_result" require "graphql/backtrace/table" require "graphql/backtrace/traced_error" module GraphQL @@ -23,7 +22,6 @@ class Backtrace def self.use(schema_defn) schema_defn.using_backtrace = true - # schema_defn.trace_with(self::Trace) end def initialize(context, value: nil) @@ -39,20 +37,5 @@ def inspect def to_a @table.to_backtrace end - - # Used for internal bookkeeping - # @api private - class Frame - attr_reader :path, :query, :ast_node, :object, :field, :arguments, :parent_frame - def initialize(path:, query:, ast_node:, object:, field:, arguments:, parent_frame:) - @path = path - @query = query - @ast_node = ast_node - @field = field - @object = object - @arguments = arguments - @parent_frame = parent_frame - end - end end end diff --git a/lib/graphql/backtrace/inspect_result.rb b/lib/graphql/backtrace/inspect_result.rb deleted file mode 100644 index ed1575de61..0000000000 --- a/lib/graphql/backtrace/inspect_result.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true -module GraphQL - class Backtrace - module InspectResult - module_function - - def inspect_result(obj) - case obj - when Hash - "{" + - obj.map do |key, val| - "#{key}: #{inspect_truncated(val)}" - end.join(", ") + - "}" - when Array - "[" + - obj.map { |v| inspect_truncated(v) }.join(", ") + - "]" - else - inspect_truncated(obj) - end - end - - def inspect_truncated(obj) - case obj - when Hash - "{...}" - when Array - "[...]" - when GraphQL::Execution::Lazy - "(unresolved)" - else - "#{obj.inspect}" - end - end - end - end -end diff --git a/lib/graphql/backtrace/table.rb b/lib/graphql/backtrace/table.rb index ab611a3a6d..911ae7353f 100644 --- a/lib/graphql/backtrace/table.rb +++ b/lib/graphql/backtrace/table.rb @@ -67,7 +67,7 @@ def rows name, "#{object}", args.to_h.inspect, - Backtrace::InspectResult.inspect_result(value), + inspect_result(value), ] result_path << path_part @@ -98,7 +98,7 @@ def rows field_path, "#{object}", args.inspect, - Backtrace::InspectResult.inspect_result(@override_value) + inspect_result(@override_value) ] end rows << HEADERS @@ -143,55 +143,6 @@ def render_table(rows) table end - # @return [Array] 5 items for a backtrace table (not `key`) - def build_rows(context_entry, rows:, top: false) - case context_entry - when Backtrace::Frame - field_alias = context_entry.ast_node.respond_to?(:alias) && context_entry.ast_node.alias - value = if top && @override_value - @override_value - else - value_at(@context.query.context.namespace(:interpreter_runtime)[:runtime], context_entry.path) - end - rows << [ - "#{context_entry.ast_node ? context_entry.ast_node.position.join(":") : ""}", - "#{context_entry.field.path}#{field_alias ? " as #{field_alias}" : ""}", - "#{context_entry.object.object.inspect}", - context_entry.arguments.to_h.inspect, # rubocop:disable Development/ContextIsPassedCop -- unrelated method - Backtrace::InspectResult.inspect_result(value), - ] - if (parent = context_entry.parent_frame) - build_rows(parent, rows: rows) - else - rows - end - when GraphQL::Query::Context - query = context_entry.query - op = query.selected_operation - if op - op_type = op.operation_type - position = "#{op.line}:#{op.col}" - else - op_type = "query" - position = "?:?" - end - op_name = query.selected_operation_name - object = query.root_value - if object.is_a?(GraphQL::Schema::Object) - object = object.object - end - value = value_at(context_entry.namespace(:interpreter_runtime)[:runtime], []) - rows << [ - "#{position}", - "#{op_type}#{op_name ? " #{op_name}" : ""}", - "#{object.inspect}", - query.variables.to_h.inspect, - Backtrace::InspectResult.inspect_result(value), - ] - else - raise "Unexpected get_rows subject #{context_entry.class} (#{context_entry.inspect})" - end - end def value_at(runtime, path) response = runtime.final_result @@ -204,6 +155,36 @@ def value_at(runtime, path) end response end + + def inspect_result(obj) + case obj + when Hash + "{" + + obj.map do |key, val| + "#{key}: #{inspect_truncated(val)}" + end.join(", ") + + "}" + when Array + "[" + + obj.map { |v| inspect_truncated(v) }.join(", ") + + "]" + else + inspect_truncated(obj) + end + end + + def inspect_truncated(obj) + case obj + when Hash + "{...}" + when Array + "[...]" + when GraphQL::Execution::Lazy + "(unresolved)" + else + "#{obj.inspect}" + end + end end end end From 8308eb20b09ec38b6b3bc6e53ce98ee842ef88dd Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 28 Jan 2025 15:34:52 -0500 Subject: [PATCH 5/5] remove untested code --- .github/workflows/ci.yaml | 2 +- benchmark/run.rb | 1 + lib/graphql/backtrace/table.rb | 43 ++++++++++++------------------ spec/graphql/authorization_spec.rb | 2 -- 4 files changed, 19 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 405c2e5504..70aac7e6e6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -78,7 +78,7 @@ jobs: - run: bundle exec rake compile - run: bundle exec rake test - run: git fetch --no-tags --prune --depth=10 origin +refs/heads/*:refs/remotes/origin/* - - run: bundle exec pronto run -f github_status github_pr -c origin/${{ github.base_ref }} + - run: bundle exec pronto run -f github_pr -c origin/${{ github.base_ref }} if: ${{ !!matrix.coverage }} env: PRONTO_PULL_REQUEST_ID: ${{ github.event.pull_request.number }} diff --git a/benchmark/run.rb b/benchmark/run.rb index 8052e0486f..32c09db6c6 100644 --- a/benchmark/run.rb +++ b/benchmark/run.rb @@ -6,6 +6,7 @@ require "stackprof" require "memory_profiler" require "graphql/batch" +require "securerandom" module GraphQLBenchmark QUERY_STRING = GraphQL::Introspection::INTROSPECTION_QUERY diff --git a/lib/graphql/backtrace/table.rb b/lib/graphql/backtrace/table.rb index 911ae7353f..dc36728cec 100644 --- a/lib/graphql/backtrace/table.rb +++ b/lib/graphql/backtrace/table.rb @@ -78,29 +78,24 @@ def rows end end - if last_part - object = result.graphql_application_value.object.inspect - ast_node = result.graphql_selections.find { |s| s.alias == last_part || s.name == last_part } - field_defn = query.get_field(result.graphql_result_type, ast_node.name) - if field_defn - args = query.arguments_for(ast_node, field_defn).to_h - field_path = field_defn.path - if ast_node.alias - field_path += " as #{ast_node.alias}" - end - else - args = {} - field_path = "#{result.graphql_result_type.graphql_name}.#{last_part}" - end - rows << [ - ast_node.position.join(":"), - field_path, - "#{object}", - args.inspect, - inspect_result(@override_value) - ] + object = result.graphql_application_value.object.inspect + ast_node = result.graphql_selections.find { |s| s.alias == last_part || s.name == last_part } + field_defn = query.get_field(result.graphql_result_type, ast_node.name) + args = query.arguments_for(ast_node, field_defn).to_h + field_path = field_defn.path + if ast_node.alias + field_path += " as #{ast_node.alias}" end + + rows << [ + ast_node.position.join(":"), + field_path, + "#{object}", + args.inspect, + inspect_result(@override_value) + ] + rows << HEADERS rows.reverse! rows @@ -147,11 +142,7 @@ def render_table(rows) def value_at(runtime, path) response = runtime.final_result path.each do |key| - if response && (response = response[key]) - next - else - break - end + response && (response = response[key]) end response end diff --git a/spec/graphql/authorization_spec.rb b/spec/graphql/authorization_spec.rb index 9241064b1f..b49af2aa44 100644 --- a/spec/graphql/authorization_spec.rb +++ b/spec/graphql/authorization_spec.rb @@ -364,8 +364,6 @@ def self.unauthorized_object(err) raise GraphQL::ExecutionError, "Unauthorized #{err.type.graphql_name}: #{err.object.inspect}" end end - - # use GraphQL::Backtrace end class SchemaWithFieldHook < GraphQL::Schema