From 23ab6be1ebb06054d8c235137893835630b1d039 Mon Sep 17 00:00:00 2001 From: Christoph Zwerschke Date: Sun, 25 Oct 2026 09:28:45 +0100 Subject: [PATCH 1/2] Create a new error for every aborted validation Raising the shared ValidationAbortedError added the frames of every aborted validation to its traceback, which kept them alive for the life of the process. --- src/graphql/validation/validate.py | 17 ++-- .../test_overlapping_fields_can_be_merged.py | 32 ++++++- tests/validation/test_validation.py | 84 ++++++++++++++++++- 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/graphql/validation/validate.py b/src/graphql/validation/validate.py index 4d183510..d452d5f8 100644 --- a/src/graphql/validation/validate.py +++ b/src/graphql/validation/validate.py @@ -30,11 +30,6 @@ class ValidationAbortedError(GraphQLError): """Error when a validation has been aborted (error limit reached).""" -validation_aborted_error = ValidationAbortedError( - "Too many validation errors, error limit reached. Validation aborted." -) - - # Per the specification, descriptions must not affect validation. # See https://spec.graphql.org/draft/#sec-Descriptions query_document_keys_to_validate: dict[str, tuple[str, ...]] = { @@ -113,7 +108,10 @@ def validate( def on_error(error: GraphQLError) -> None: if len(errors) >= max_errors: - raise validation_aborted_error + # Unlike GraphQL.js, create a new error each time, because raising a + # shared error would add the frames of every validation to its traceback. + msg = "Too many validation errors, error limit reached. Validation aborted." + raise ValidationAbortedError(msg) errors.append(error) context = ValidationContext( @@ -131,8 +129,11 @@ def on_error(error: GraphQLError) -> None: TypeInfoVisitor(type_info, ParallelVisitor(visitors)), query_document_keys_to_validate, ) - except ValidationAbortedError: - errors.append(validation_aborted_error) + except ValidationAbortedError as error: + # Do not keep the frames of the validation alive with the returned error, + # also not via the context of an error that was handled while aborting. + error.__context__ = None + errors.append(error.with_traceback(None)) return errors diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index c492da06..ec044734 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1,11 +1,17 @@ from contextlib import contextmanager from functools import partial +from graphql.language import parse from graphql.utilities import build_schema -from graphql.validation import OverlappingFieldsCanBeMergedRule +from graphql.validation import ( + FieldsOnCorrectTypeRule, + OverlappingFieldsCanBeMergedRule, + validate, +) from graphql.validation.rules import ( overlapping_fields_can_be_merged as overlapping_module, ) +from graphql.validation.validate import ValidationAbortedError from .harness import assert_validation_errors @@ -1666,6 +1672,30 @@ def aborts_validation_when_the_comparison_budget_is_exceeded(): schema=budget_schema, ) + def aborted_error_does_not_keep_the_exceeded_budget_alive(): + # The unknown field reaches the error limit, so reporting the exceeded + # budget aborts the validation. This happens while the internal budget + # signal is handled, which must not stay attached to the aborted error. + query = "{ unknown field { " + " ".join(["... on Node { x }"] * 10) + " } }" + with budget_of(5): + errors = validate( + budget_schema, + parse(query, no_location=True), + [FieldsOnCorrectTypeRule, OverlappingFieldsCanBeMergedRule], + max_errors=1, + ) + assert errors == [ + {"message": "Cannot query field 'unknown' on type 'Query'."}, + { + "message": "Too many validation errors, error limit reached." + " Validation aborted." + }, + ] + error = errors[-1] + assert isinstance(error, ValidationAbortedError) + assert error.__traceback__ is None + assert error.__context__ is None + def deduplication_alone_would_not_bound_this_query(): # Each `f` selection shares the response name `f` but wraps a distinct # sub-selection (`a0: x`, `a1: x`, ...), so every field has a distinct diff --git a/tests/validation/test_validation.py b/tests/validation/test_validation.py index bd01a7b3..bfd97652 100644 --- a/tests/validation/test_validation.py +++ b/tests/validation/test_validation.py @@ -1,9 +1,13 @@ +import gc +import weakref + import pytest from graphql.error import GraphQLError from graphql.language import parse from graphql.utilities import build_schema -from graphql.validation import ValidationRule, validate +from graphql.validation import FieldsOnCorrectTypeRule, ValidationRule, validate +from graphql.validation.validate import ValidationAbortedError from .harness import test_schema @@ -128,6 +132,84 @@ def limits_to_100_when_max_errors_is_not_passed(): " Validation aborted." } + class ReportWhileHandlingErrorRule(ValidationRule): + def enter_field(self, node, *_args): + try: + raise RuntimeError("Handled error from custom rule!") + except RuntimeError: + self.report_error(GraphQLError("Error from custom rule!", node)) + + def _validate_and_track(rules, count=3): + # Run aborted validations with fresh documents, keep the aborted errors, + # and track the documents and contexts with weak references. The other + # errors are dropped, since errors created while handling an exception + # keep its traceback on purpose. + aborted_errors, refs = [], [] + + class TrackContextRule(ValidationRule): + def __init__(self, context): + super().__init__(context) + refs.append(weakref.ref(context)) + + for _ in range(count): + document = parse(query, no_location=True) + refs.append(weakref.ref(document)) + errors = validate( + test_schema, document, [TrackContextRule, *rules], max_errors=1 + ) + assert len(errors) == 2 + aborted_errors.append(errors[-1]) + del document, errors + gc.collect() + return aborted_errors, refs + + def creates_a_new_error_for_every_aborted_validation(): + first_error = _validate_document(max_errors=1)[-1] + second_error = _validate_document(max_errors=1)[-1] + assert type(first_error) is ValidationAbortedError + assert type(second_error) is ValidationAbortedError + assert first_error == second_error + assert first_error is not second_error + + def returns_the_aborted_error_without_traceback_and_context(): + error = _validate_document(max_errors=1)[-1] + assert isinstance(error, ValidationAbortedError) + assert error.__traceback__ is None + assert error.__context__ is None + assert error.__cause__ is None + + def returns_the_aborted_error_without_context_of_a_handled_error(): + errors = validate( + test_schema, doc, [ReportWhileHandlingErrorRule], max_errors=1 + ) + assert len(errors) == 2 + error = errors[-1] + assert isinstance(error, ValidationAbortedError) + assert error.__traceback__ is None + assert error.__context__ is None + assert error.__cause__ is None + + def does_not_keep_aborted_validations_alive(): + aborted_errors, refs = _validate_and_track([FieldsOnCorrectTypeRule]) + assert all(isinstance(e, ValidationAbortedError) for e in aborted_errors) + assert len(refs) == 6 + assert all(ref() is None for ref in refs) + + def does_not_keep_validations_alive_that_were_aborted_while_handling_errors(): + aborted_errors, refs = _validate_and_track([ReportWhileHandlingErrorRule]) + assert all(isinstance(e, ValidationAbortedError) for e in aborted_errors) + assert len(refs) == 6 + assert all(ref() is None for ref in refs) + + def aborted_error_accepts_the_arguments_of_graphql_error(): + node = doc.definitions[0] + error = ValidationAbortedError( + "Validation aborted.", node, extensions={"code": "ABORTED"} + ) + assert error.message == "Validation aborted." + assert error.nodes == [node] + assert error.extensions == {"code": "ABORTED"} + def pass_through_exceptions_from_rules(): class CustomRule(ValidationRule): def enter_field(self, *_args): From 39f79f1e76d90e3b82c2ea64af9eb59c1d36e0fb Mon Sep 17 00:00:00 2001 From: Christoph Zwerschke Date: Sun, 25 Oct 2026 09:40:18 +0100 Subject: [PATCH 2/2] Do not let errors adopt or accumulate tracebacks A GraphQLError no longer adopts the traceback of the exception that is being handled when it is created, and the memoized errors of computations and the reasons of abort signals are raised again with their original tracebacks. --- src/graphql/error/graphql_error.py | 3 - src/graphql/execution/executor.py | 9 ++- .../execution/incremental/computation.py | 30 ++++++---- src/graphql/pyutils/abort_signal.py | 8 ++- tests/error/test_graphql_error.py | 30 +++++++--- .../execution/incremental/test_computation.py | 60 +++++++++++++++++++ tests/execution/test_cancellation.py | 50 ++++++++++++++++ .../test_overlapping_fields_can_be_merged.py | 16 +++++ 8 files changed, 180 insertions(+), 26 deletions(-) diff --git a/src/graphql/error/graphql_error.py b/src/graphql/error/graphql_error.py index 2161c5e4..be88acc4 100644 --- a/src/graphql/error/graphql_error.py +++ b/src/graphql/error/graphql_error.py @@ -2,7 +2,6 @@ from __future__ import annotations -from sys import exc_info from typing import TYPE_CHECKING, Any, TypeAlias, TypedDict from ..pyutils import Undefined @@ -316,8 +315,6 @@ def __init__( if isinstance(original_extensions, dict): extensions = original_extensions self.extensions = extensions or {} - if not self.__traceback__: - self.__traceback__ = exc_info()[2] def __str__(self) -> str: """Get this error as a human-readable message with source locations. diff --git a/src/graphql/execution/executor.py b/src/graphql/execution/executor.py index 1cb594b8..ed887831 100644 --- a/src/graphql/execution/executor.py +++ b/src/graphql/execution/executor.py @@ -1211,9 +1211,14 @@ def abort_error(self) -> Exception: :meta private: """ - reason = self.abort_signal.reason # type: ignore[union-attr] + abort_signal = self.abort_signal + reason = abort_signal.reason # type: ignore[union-attr] if isinstance(reason, Exception): - return reason + # Restore the traceback the reason had when the signal was aborted, + # since raising the same reason again and again would extend it. + return reason.with_traceback( + getattr(abort_signal, "_reason_traceback", reason.__traceback__) + ) msg = f"Unexpected error value: {inspect(reason)}" return TypeError(msg) diff --git a/src/graphql/execution/incremental/computation.py b/src/graphql/execution/incremental/computation.py index 74a51d1d..0b8fd844 100644 --- a/src/graphql/execution/incremental/computation.py +++ b/src/graphql/execution/incremental/computation.py @@ -10,6 +10,7 @@ if TYPE_CHECKING: from asyncio import Future from collections.abc import Callable + from types import TracebackType from ...pyutils import AwaitableOrValue @@ -40,9 +41,10 @@ class Computation(Generic[T]): For internal use only. """ - __slots__ = "_fn", "_on_abort", "_status", "_value" + __slots__ = "_fn", "_on_abort", "_status", "_traceback", "_value" _status: str | None + _traceback: TracebackType | None _value: Any def __init__( @@ -55,6 +57,7 @@ def __init__( self._fn = fn self._on_abort = on_abort self._status = None + self._traceback = None self._value = None def prime(self) -> None: @@ -64,8 +67,7 @@ def prime(self) -> None: try: result = self._fn() except Exception as reason: - self._status = _REJECTED - self._value = reason + self._reject(reason) return if is_awaitable(result): future = ensure_future(result) @@ -95,7 +97,9 @@ def result(self) -> AwaitableOrValue[T]: reason = self._value if reason is None: raise CancelledError - raise reason + # Restore the original traceback, which would grow otherwise, + # since the same reason is raised every time the result is requested. + raise reason.with_traceback(self._traceback) return self._value # the pending future def abort(self, reason: BaseException | None = None) -> AwaitableOrValue[None]: @@ -106,12 +110,10 @@ def abort(self, reason: BaseException | None = None) -> AwaitableOrValue[None]: """ status = self._status if status is None: - self._status = _REJECTED - self._value = reason + self._reject(reason) elif status is _PENDING: future = self._value - self._status = _REJECTED - self._value = reason + self._reject(reason) future.cancel() on_abort = self._on_abort if on_abort is not None: @@ -123,15 +125,19 @@ def _settle(self, future: Future[T]) -> None: if future.cancelled(): if self._status is _PENDING: # cancelled externally, not via abort - self._status = _REJECTED - self._value = CancelledError() + self._reject(CancelledError()) return reason = future.exception() # always retrieve, avoiding warnings if self._status is not _PENDING: return # aborted after the future had already completed if reason is not None: - self._status = _REJECTED - self._value = reason + self._reject(reason) else: self._status = _FULFILLED self._value = future.result() + + def _reject(self, reason: BaseException | None) -> None: + """Memoize the reason why the computation failed or was aborted.""" + self._status = _REJECTED + self._value = reason + self._traceback = None if reason is None else reason.__traceback__ diff --git a/src/graphql/pyutils/abort_signal.py b/src/graphql/pyutils/abort_signal.py index b2a541ef..91a51f2a 100644 --- a/src/graphql/pyutils/abort_signal.py +++ b/src/graphql/pyutils/abort_signal.py @@ -3,7 +3,10 @@ from __future__ import annotations from asyncio import Event -from typing import Any +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from types import TracebackType __all__ = ["AbortController", "AbortError", "AbortSignal"] @@ -42,6 +45,7 @@ class AbortSignal: def __init__(self) -> None: self.aborted = False self.reason = None + self._reason_traceback: TracebackType | None = None self._event = Event() async def wait(self) -> Any: @@ -110,4 +114,6 @@ def abort(self, reason: Any = None) -> None: signal.reason = ( AbortError("This operation was aborted") if reason is None else reason ) + if isinstance(reason, BaseException): + signal._reason_traceback = reason.__traceback__ # noqa: SLF001 signal._event.set() # noqa: SLF001 diff --git a/tests/error/test_graphql_error.py b/tests/error/test_graphql_error.py index 191b604f..7ff14ff6 100644 --- a/tests/error/test_graphql_error.py +++ b/tests/error/test_graphql_error.py @@ -2,6 +2,8 @@ from typing import cast +import pytest + from graphql.error import GraphQLError from graphql.language import ( NameNode, @@ -32,6 +34,18 @@ assert field_node +def assert_raised_with_new_traceback(error: GraphQLError, handled_error: Exception): + # In Python, an error gets its traceback when it is raised, and an error that + # is being handled at this time becomes its context, not part of its traceback. + with pytest.raises(GraphQLError) as exc_info: + raise error + assert exc_info.value is error + assert error.__traceback__ is not None + assert error.__traceback__ is not handled_error.__traceback__ + assert error.__traceback__.tb_next is None + assert error.__context__ is handled_error + + def describe_graphql_error(): def is_a_class_and_is_a_subclass_of_exception(): assert type(GraphQLError) is type @@ -117,14 +131,14 @@ def passes_the_cause_of_an_original_error(): def creates_new_stack_if_original_error_has_no_stack(): try: raise RuntimeError - except RuntimeError as original_with_traceback: - original_traceback = original_with_traceback.__traceback__ + except RuntimeError as handled_error: original = RuntimeError("original") e = GraphQLError("msg", original_error=original) + # the traceback of the handled error is not adopted + assert e.__traceback__ is None + assert_raised_with_new_traceback(e, handled_error) assert e.__class__.__name__ == "GraphQLError" assert original.__traceback__ is None - assert original_traceback is not None - assert e.__traceback__ is original_traceback assert e.message == "msg" assert e.original_error is original assert str(e.original_error) == "original" @@ -179,13 +193,13 @@ def prefers_cause_and_original_error_separately(): def creates_new_stack_if_cause_has_no_stack(): try: raise RuntimeError - except RuntimeError as runtime_error: - current_traceback = runtime_error.__traceback__ + except RuntimeError as handled_error: cause = RuntimeError("cause") e = GraphQLError("msg", cause=cause) + # the traceback of the handled error is not adopted + assert e.__traceback__ is None + assert_raised_with_new_traceback(e, handled_error) assert cause.__traceback__ is None - assert current_traceback is not None - assert e.__traceback__ is current_traceback assert e.message == "msg" assert e.cause is cause assert e.original_error is cause diff --git a/tests/execution/incremental/test_computation.py b/tests/execution/incremental/test_computation.py index c344f27f..b49823fa 100644 --- a/tests/execution/incremental/test_computation.py +++ b/tests/execution/incremental/test_computation.py @@ -9,6 +9,7 @@ if TYPE_CHECKING: from collections.abc import Callable + from types import TracebackType pytestmark = pytest.mark.anyio @@ -25,6 +26,15 @@ def __call__(self, *args: Any, **kwargs: Any) -> Any: return self.fn(*args, **kwargs) +def traceback_entries(error: BaseException) -> list[TracebackType]: + entries = [] + traceback = error.__traceback__ + while traceback: + entries.append(traceback) + traceback = traceback.tb_next + return entries + + def describe_computation(): def can_return_a_result(): computation = Computation(lambda: {"value": 123}) @@ -95,6 +105,56 @@ async def run() -> None: computation.result() assert run_spy.call_count == 1 + def raises_a_sync_error_again_with_its_original_traceback(): + def run() -> None: + raise RuntimeError("failure") + + computation = Computation(run) + + lengths = [] + for _ in range(3): + with pytest.raises(RuntimeError, match="failure") as exc_info: + computation.result() + lengths.append(len(traceback_entries(exc_info.value))) + assert lengths[0] > 1 + assert lengths[1] == lengths[2] == lengths[0] + + async def raises_an_async_error_again_with_its_original_traceback(): + async def run() -> None: + await sleep(0) + raise RuntimeError("failure") + + computation = Computation(Spy(run)) + + computation.prime() + with pytest.raises(RuntimeError, match="failure"): + await computation.result() + lengths = [] + for _ in range(3): + with pytest.raises(RuntimeError, match="failure") as exc_info: + computation.result() + lengths.append(len(traceback_entries(exc_info.value))) + assert lengths[1] == lengths[2] == lengths[0] + + def raises_an_abort_reason_again_with_its_original_traceback(): + try: + raise RuntimeError("aborted") + except RuntimeError as error: + abort_reason = error + original_traceback = abort_reason.__traceback__ + computation = Computation(lambda: {"value": 123}) + + computation.abort(abort_reason) + lengths = [] + for _ in range(3): + with pytest.raises(RuntimeError, match="aborted") as exc_info: + computation.result() + assert exc_info.value is abort_reason + entries = traceback_entries(abort_reason) + assert entries[-1] is original_traceback + lengths.append(len(entries)) + assert lengths[1] == lengths[2] == lengths[0] + def can_be_aborted_before_running(): on_abort_spy = Spy(lambda _reason: None) run_spy = Spy(lambda: {"value": 123}) diff --git a/tests/execution/test_cancellation.py b/tests/execution/test_cancellation.py index af625aca..bccb488d 100644 --- a/tests/execution/test_cancellation.py +++ b/tests/execution/test_cancellation.py @@ -4,6 +4,7 @@ from asyncio import Event, Future, ensure_future, sleep from collections.abc import AsyncIterator, Awaitable +from typing import TYPE_CHECKING import pytest @@ -26,6 +27,9 @@ from graphql.language import parse from graphql.pyutils import AbortController, AbortError, is_awaitable +if TYPE_CHECKING: + from types import TracebackType + pytestmark = [ pytest.mark.anyio, pytest.mark.filterwarnings("ignore:coroutine .* was never awaited:RuntimeWarning"), @@ -65,6 +69,15 @@ ) +def traceback_entries(error: BaseException) -> list[TracebackType]: + entries = [] + traceback = error.__traceback__ + while traceback: + entries.append(traceback) + traceback = traceback.tb_next + return entries + + def must_not_be_called(*_args, **_kwargs): pytest.fail("Should not be called") # pragma: no cover @@ -1236,3 +1249,40 @@ async def aclose(self): # wait for the partial result so that the cleanup has settled await exc_info.value.aborted_result assert return_called is True + + +def describe_abort_reason_traceback(): + def does_not_extend_the_traceback_of_the_default_reason(): + abort_controller = AbortController() + abort_controller.abort() + reason = abort_controller.signal.reason + document = parse("{ todo { id } }") + + lengths = [] + for _ in range(3): + with pytest.raises(AbortError) as exc_info: + execute(schema, document, abort_signal=abort_controller.signal) + assert exc_info.value is reason + lengths.append(len(traceback_entries(reason))) + assert lengths[0] > 0 + assert lengths[1] == lengths[2] == lengths[0] + + def does_not_extend_the_traceback_of_a_provided_reason(): + try: + raise RuntimeError("Custom abort error") + except RuntimeError as error: + reason = error + original_traceback = reason.__traceback__ + abort_controller = AbortController() + abort_controller.abort(reason) + document = parse("{ todo { id } }") + + lengths = [] + for _ in range(3): + with pytest.raises(RuntimeError, match=r"^Custom abort error$") as exc_info: + execute(schema, document, abort_signal=abort_controller.signal) + assert exc_info.value is reason + entries = traceback_entries(reason) + assert entries[-1] is original_traceback + lengths.append(len(entries)) + assert lengths[1] == lengths[2] == lengths[0] diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index ec044734..dcd90215 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1672,6 +1672,22 @@ def aborts_validation_when_the_comparison_budget_is_exceeded(): schema=budget_schema, ) + def error_does_not_keep_the_exceeded_budget_alive(): + # The error is created while the internal budget signal is handled, + # but must not adopt its traceback, which keeps the comparison alive. + query = "{ field { " + " ".join(["... on Node { x }"] * 10) + " } }" + with budget_of(5): + errors = validate( + budget_schema, + parse(query, no_location=True), + [OverlappingFieldsCanBeMergedRule], + ) + assert len(errors) == 1 + error = errors[0] + assert error.message.startswith("Query is too complex to validate") + assert error.__traceback__ is None + assert error.__context__ is None + def aborted_error_does_not_keep_the_exceeded_budget_alive(): # The unknown field reaches the error limit, so reporting the exceeded # budget aborts the validation. This happens while the internal budget