Skip to content

Fix execution next invalid resolved types - #5727

Open
ydah wants to merge 2 commits into
rmosolgo:masterfrom
ydah:fix-execution-next-invalid-resolved-types
Open

ydah wants to merge 2 commits into
rmosolgo:masterfrom
ydah:fix-execution-next-invalid-resolved-types

Conversation

@ydah

@ydah ydah commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Execution::Next validates the result of Schema.resolve_type, but its callers currently assume that Schema.type_error always raises.

Schemas may override type_error to report an UnresolvedTypeError and return normally. In that case, execution continues using a type that isn't one of the abstract type's possible types. Nullable abstract fields aren't set to null, non-null list items don't perform normal null propagation, and deferred list resolution may leave placeholder results behind.

Abstract roots executed with Query#run_partials have another issue: type validation expects a field resolution step even when no field is available, causing a NoMethodError instead of the intended UnresolvedTypeError.

This PR makes resolved-type validation return whether the type is valid and stops the affected execution path when type_error handles the mismatch. It also:

  • Applies normal null propagation to invalid nullable and non-null abstract values.
  • Tracks result locations for eagerly resolved abstract lists.
  • Supports UnresolvedTypeError without a field for abstract partial roots.
  • Preserves field and parent-type details when the partial originated from a field.

Tests cover immediate and lazy type resolution, abstract partial roots, non-null list elements, and batched list execution after a sibling error.

Comment on lines +694 to +697
if @static_type.kind.abstract?
@invalid_type_locations ||= {}.compare_by_identity
@invalid_type_locations[next_result_h] = [graphql_result, key, is_nn, is_from_array]
end

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't quite grokked how this value is used... could you help me understand it? Under what conditions is it assigned and populated? Under what conditions are values read from it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used by the eager abstract-value path that bypasses PrepareObjectStep.

build_graphql_result recursively builds list results and flattens the object candidates into @all_next_objects / @all_next_results for batched type resolution. Once that happens, each placeholder result no longer tells us which parent Hash or Array contains it, its key/index, or whether that location is non-null.

For abstract values, @invalid_type_locations records that information while the placeholder is inserted:

  • the parent result container
  • its key or list index
  • whether the value is non-null
  • whether it came from a list

Later, enqueue_next_steps resolves and validates the runtime type. If type_error handles an invalid resolved type and returns normally, set_invalid_type_result uses the saved location to replace the correct value with nil or perform normal non-null propagation.

It isn't read for valid resolved types, and it is cleared after that batch has been processed. compare_by_identity is needed because these placeholders begin as empty Hashes, which would otherwise compare equal as Hash keys.

The PrepareObjectStep path doesn't need this mapping because each step already retains its parent result, key, nullability, and list-location state.

@ydah
ydah force-pushed the fix-execution-next-invalid-resolved-types branch from 1e3f0f2 to b152ec0 Compare September 16, 2026 06:31
@ydah
ydah requested a review from rmosolgo September 16, 2026 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants