diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..1a8663e2e7 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,18 @@ +--- +release type: patch +social_messages: + x: >- + {project_name} {version} is out! This release ensures `MaskErrors` also + masks parsing and validation errors during synchronous execution. 🍓 + https://strawberry.rocks/release/{version} + linkedin: >- + {project_name} {version} is out. This release fixes `MaskErrors` so + synchronous parsing and validation failures no longer expose their original + error details. +--- + +This release fixes an issue where `MaskErrors` leaked parsing and validation +error details during synchronous execution. + +Synchronous execution now masks pre-execution errors consistently with +asynchronous execution, including when `ValidationCache` is enabled. diff --git a/strawberry/extensions/mask_errors.py b/strawberry/extensions/mask_errors.py index cf556168b4..7f56812be1 100644 --- a/strawberry/extensions/mask_errors.py +++ b/strawberry/extensions/mask_errors.py @@ -1,5 +1,5 @@ from collections.abc import Callable, Iterator -from typing import Any +from typing import Protocol, runtime_checkable from graphql import ExecutionResult as GraphQLExecutionResult from graphql.error import GraphQLError @@ -11,6 +11,11 @@ from strawberry.types.execution import StreamExecutionResult +@runtime_checkable +class _ResultWithErrors(Protocol): + errors: list[GraphQLError] | None + + def default_should_mask_error(_: GraphQLError) -> bool: # Mask all errors return True @@ -39,12 +44,7 @@ def anonymise_error(self, error: GraphQLError) -> GraphQLError: original_error=None, ) - # TODO: proper typing - def _process_result(self, result: Any) -> None: - errors = getattr(result, "errors", None) - if not errors: - return - + def _process_errors(self, errors: list[GraphQLError]) -> list[GraphQLError]: processed_errors: list[GraphQLError] = [] for error in errors: @@ -53,7 +53,11 @@ def _process_result(self, result: Any) -> None: else: processed_errors.append(error) - result.errors = processed_errors + return processed_errors + + def _process_result(self, result: object) -> None: + if isinstance(result, _ResultWithErrors) and result.errors: + result.errors = self._process_errors(result.errors) def _process_stream_result(self, result: StreamExecutionResult) -> None: self._process_result(result) diff --git a/strawberry/schema/schema.py b/strawberry/schema/schema.py index 0b13d4b9d9..22141ded4a 100644 --- a/strawberry/schema/schema.py +++ b/strawberry/schema/schema.py @@ -840,6 +840,9 @@ def execute_sync( execution_context, extensions_runner ) ) is not None: + # Match the async path by exposing pre-execution results to + # operation extensions before their hooks unwind. + execution_context.result = pre_execution_result return pre_execution_result assert execution_context.graphql_document is not None diff --git a/tests/schema/extensions/test_mask_errors.py b/tests/schema/extensions/test_mask_errors.py index 01e8ccdcbb..c0cc417a7a 100644 --- a/tests/schema/extensions/test_mask_errors.py +++ b/tests/schema/extensions/test_mask_errors.py @@ -4,7 +4,81 @@ from graphql.error import GraphQLError import strawberry -from strawberry.extensions import MaskErrors +from strawberry.extensions import MaskErrors, ValidationCache + + +@pytest.mark.parametrize( + "query", + [ + "query { testField( }", + "query { missingField }", + ], +) +def test_mask_pre_execution_errors_sync(query: str): + @strawberry.type + class Query: + @strawberry.field + def test_field(self) -> str: + return "TestField" + + schema = strawberry.Schema( + query=Query, + extensions=[ValidationCache, MaskErrors], + ) + + result = schema.execute_sync(query) + + assert result.data is None + assert result.errors is not None + assert [error.message for error in result.errors] == ["Unexpected error."] + + +def test_mask_cached_validation_errors_sync(): + @strawberry.type + class Query: + @strawberry.field + def test_field(self) -> str: + return "TestField" + + schema = strawberry.Schema( + query=Query, + extensions=[ValidationCache, MaskErrors], + ) + + query = "query { missingField }" + results = (schema.execute_sync(query), schema.execute_sync(query)) + + for result in results: + assert result.data is None + assert result.errors is not None + assert [error.message for error in result.errors] == ["Unexpected error."] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "query", + [ + "query { testField( }", + "query { missingField }", + ], +) +async def test_mask_pre_execution_errors_async(query: str): + @strawberry.type + class Query: + @strawberry.field + def test_field(self) -> str: + return "TestField" + + schema = strawberry.Schema( + query=Query, + extensions=[ValidationCache, MaskErrors], + ) + + result = await schema.execute(query) + + assert result.data is None + assert result.errors is not None + assert [error.message for error in result.errors] == ["Unexpected error."] def test_mask_all_errors():