diff --git a/packages/smithy-core/.changes/next-release/smithy-core-bugfix-068484dcc3df4c57abfa1cace59988d1.json b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-068484dcc3df4c57abfa1cace59988d1.json new file mode 100644 index 000000000..07a364844 --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-068484dcc3df4c57abfa1cace59988d1.json @@ -0,0 +1,4 @@ +{ + "type": "bugfix", + "description": "Fixed `InterceptorChain` discarding changes made by `modify_*` hooks. Each interceptor now receives the value returned by the interceptor before it, so an interceptor that returns its input unchanged no longer undoes earlier modifications. `modify_before_completion` also now calls each interceptor's `modify_before_completion` hook instead of `modify_before_attempt_completion`." +} \ No newline at end of file diff --git a/packages/smithy-core/.changes/next-release/smithy-core-bugfix-21165dd714af453ea800bc3004248979.json b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-21165dd714af453ea800bc3004248979.json new file mode 100644 index 000000000..a5d7f3867 --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-21165dd714af453ea800bc3004248979.json @@ -0,0 +1,4 @@ +{ + "type": "bugfix", + "description": "Fixed the request pipeline ignoring the request returned by `modify_before_serialization`. The modified request is now serialized, used for endpoint resolution, and passed to later hooks if serialization fails. `InterceptorChain` now also raises a `TypeError` when a `modify_*` hook returns `None`, instead of passing `None` to the next interceptor." +} \ No newline at end of file diff --git a/packages/smithy-core/.changes/next-release/smithy-core-bugfix-59eb8351a1d040ee910f2c25c758ca12.json b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-59eb8351a1d040ee910f2c25c758ca12.json new file mode 100644 index 000000000..4b79caa1d --- /dev/null +++ b/packages/smithy-core/.changes/next-release/smithy-core-bugfix-59eb8351a1d040ee910f2c25c758ca12.json @@ -0,0 +1,4 @@ +{ + "type": "bugfix", + "description": "Fixed `InterceptorChain` skipping later interceptors when one raised in a `read_*` hook. Every interceptor's read hook now runs, then the last exception is raised and earlier ones are logged. `read_after_attempt` also now passes each exception to later interceptors as the `response`, as `read_after_execution` already did." +} \ No newline at end of file diff --git a/packages/smithy-core/src/smithy_core/aio/client.py b/packages/smithy-core/src/smithy_core/aio/client.py index c358a9c1b..c26435a4c 100644 --- a/packages/smithy-core/src/smithy_core/aio/client.py +++ b/packages/smithy-core/src/smithy_core/aio/client.py @@ -322,10 +322,9 @@ async def _handle_execution[I: SerializeableShape, O: DeserializeableShape]( call: ClientCall[I, O], request_future: Future[RequestContext[I, TRequest]] | None, ) -> OutputContext[I, O, TRequest | None, TResponse | None]: + interceptor = call.interceptor + input_context = InputContext(request=call.input, properties=call.context) try: - interceptor = call.interceptor - - input_context = InputContext(request=call.input, properties=call.context) interceptor.read_before_execution(input_context) input_context = replace( @@ -338,7 +337,7 @@ async def _handle_execution[I: SerializeableShape, O: DeserializeableShape]( transport_request = self.protocol.serialize_request( operation=call.operation, - input=call.input, + input=input_context.request, endpoint=_UNRESOLVED, context=input_context.properties, ) @@ -353,11 +352,11 @@ async def _handle_execution[I: SerializeableShape, O: DeserializeableShape]( ) except Exception as e: return OutputContext( - request=call.input, + request=input_context.request, response=e, transport_request=None, transport_response=None, - properties=call.context, + properties=input_context.properties, ) try: @@ -449,7 +448,7 @@ async def _handle_attempt[I: SerializeableShape, O: DeserializeableShape]( endpoint_params = EndpointResolverParams( operation=call.operation, - input=call.input, + input=request_context.request, context=request_context.properties, ) _LOGGER.debug("Calling endpoint resolver with params: %s", endpoint_params) diff --git a/packages/smithy-core/src/smithy_core/interceptors.py b/packages/smithy-core/src/smithy_core/interceptors.py index aca9a4cfa..381892f31 100644 --- a/packages/smithy-core/src/smithy_core/interceptors.py +++ b/packages/smithy-core/src/smithy_core/interceptors.py @@ -1,6 +1,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -from collections.abc import Sequence +import logging +from collections.abc import Callable, Sequence from dataclasses import dataclass, replace from typing import Any @@ -8,6 +9,8 @@ from .interfaces import TypedProperties from .serializers import SerializeableShape +_LOGGER = logging.getLogger(__name__) + @dataclass(kw_only=True, frozen=True, slots=True) class InputContext[Request: SerializeableShape]: @@ -122,8 +125,11 @@ def read_before_serialization(self, context: InputContext[Request]) -> None: The `request` of the context will always be available. Other static properties will be None. - If exceptions are thrown by this hook, execution will jump to - `modify_before_completion` with the thrown exception as the `response`. + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_before_serialization` invoked. Execution will then jump to + `modify_before_completion` with the thrown exception as the `response`. If + multiple `read_before_serialization` methods throw exceptions, the latest will + be used and earlier ones will be logged and dropped. """ def read_after_serialization( @@ -142,8 +148,11 @@ def read_after_serialization( The `request` and `transport_request` of the context will always be available. Other static properties will be None. - If exceptions are thrown by this hook, execution will jump to - `modify_before_completion` with the thrown exception as the `response`. + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_after_serialization` invoked. Execution will then jump to + `modify_before_completion` with the thrown exception as the `response`. If + multiple `read_after_serialization` methods throw exceptions, the latest will be + used and earlier ones will be logged and dropped. """ def modify_before_retry_loop( @@ -235,8 +244,11 @@ def read_before_signing( not include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_before_signing` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_before_signing` methods throw exceptions, the latest will be + used and earlier ones will be logged and dropped. """ def read_after_signing( @@ -258,8 +270,11 @@ def read_after_signing( not include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_after_signing` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_after_signing` methods throw exceptions, the latest will be + used and earlier ones will be logged and dropped. """ def modify_before_transmit( @@ -308,8 +323,11 @@ def read_before_transmit( not include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_before_transmit` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_before_transmit` methods throw exceptions, the latest will be + used and earlier ones will be logged and dropped. """ def read_after_transmit( @@ -334,8 +352,11 @@ def read_after_transmit( retries, the context will not include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_after_transmit` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_after_transmit` methods throw exceptions, the latest will be + used and earlier ones will be logged and dropped. """ def modify_before_deserialization( @@ -389,8 +410,11 @@ def read_before_deserialization( retries, the context will not include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_before_deserialization` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_before_deserialization` methods throw exceptions, the latest + will be used and earlier ones will be logged and dropped. """ def read_after_deserialization( @@ -414,8 +438,11 @@ def read_after_deserialization( include changes made in previous attempts (e.g. by request signers or other interceptors). - If exceptions are thrown by this hook, execution will jump to + Exceptions thrown by this hook will be stored until all interceptors have had + their `read_after_deserialization` invoked. Execution will then jump to `modify_before_attempt_completion` with the thrown exception as the `response`. + If multiple `read_after_deserialization` methods throw exceptions, the latest + will be used and earlier ones will be logged and dropped. """ def modify_before_attempt_completion( @@ -469,9 +496,10 @@ def read_after_attempt( other interceptors). Exceptions thrown by this hook will be stored until all interceptors have had - their `read_after_attempt` invoked. If multiple `read_after_attempt` methods - throw exceptions, the latest will be used and earlier ones will be logged and - dropped. If the retry strategy determines that the execution is retryable, + their `read_after_attempt` invoked, and later interceptors will receive the + most recent exception as the `response`. If multiple `read_after_attempt` + methods throw exceptions, the latest will be used and earlier ones will be + logged and dropped. If the retry strategy determines that the execution is retryable, execution will then jump to `read_before_attempt`. Otherwise, execution will jump to `modify_before_completion` with the thrown exception as the `response`. """ @@ -521,8 +549,9 @@ def read_after_execution( proceeded far enough for them to be generated. Exceptions thrown by this hook will be stored until all interceptors have had - their `read_after_execution` invoked. The exception will then be treated as the - final response. If multiple `read_after_execution` methods throw exceptions, + their `read_after_execution` invoked, and later interceptors will receive the + most recent exception as the `response`. The exception will then be treated as + the final response. If multiple `read_after_execution` methods throw exceptions, the latest will be used and earlier ones will be logged and dropped. """ @@ -547,113 +576,171 @@ def __repr__(self) -> str: return f"InterceptorChain(chain={self._chain!r})" def read_before_execution(self, context: InputContext[Any]) -> None: - for interceptor in self._chain: - interceptor.read_before_execution(context) + self._invoke_all(lambda interceptor: interceptor.read_before_execution(context)) def modify_before_serialization(self, context: InputContext[Any]) -> Any: - request = context.request for interceptor in self._chain: request = interceptor.modify_before_serialization(context) - return request + _check_returned(request, interceptor, "modify_before_serialization") + # Most hooks return what they were given, so only copy on a change. + if request is not context.request: + context = replace(context, request=request) + return context.request def read_before_serialization(self, context: InputContext[Any]) -> None: - for interceptor in self._chain: - interceptor.read_before_serialization(context) + self._invoke_all( + lambda interceptor: interceptor.read_before_serialization(context) + ) def read_after_serialization(self, context: RequestContext[Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_after_serialization(context) + self._invoke_all( + lambda interceptor: interceptor.read_after_serialization(context) + ) def modify_before_retry_loop(self, context: RequestContext[Any, Any]) -> Any: - transport_request = context.transport_request for interceptor in self._chain: transport_request = interceptor.modify_before_retry_loop(context) - return transport_request + _check_returned(transport_request, interceptor, "modify_before_retry_loop") + if transport_request is not context.transport_request: + context = replace(context, transport_request=transport_request) + return context.transport_request def read_before_attempt(self, context: RequestContext[Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_before_attempt(context) + self._invoke_all(lambda interceptor: interceptor.read_before_attempt(context)) def modify_before_signing(self, context: RequestContext[Any, Any]) -> Any: - transport_request = context.transport_request for interceptor in self._chain: transport_request = interceptor.modify_before_signing(context) - return transport_request + _check_returned(transport_request, interceptor, "modify_before_signing") + if transport_request is not context.transport_request: + context = replace(context, transport_request=transport_request) + return context.transport_request def read_before_signing(self, context: RequestContext[Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_before_signing(context) + self._invoke_all(lambda interceptor: interceptor.read_before_signing(context)) def read_after_signing(self, context: RequestContext[Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_after_signing(context) + self._invoke_all(lambda interceptor: interceptor.read_after_signing(context)) def modify_before_transmit(self, context: RequestContext[Any, Any]) -> Any: - transport_request = context.transport_request for interceptor in self._chain: transport_request = interceptor.modify_before_transmit(context) - return transport_request + _check_returned(transport_request, interceptor, "modify_before_transmit") + if transport_request is not context.transport_request: + context = replace(context, transport_request=transport_request) + return context.transport_request def read_before_transmit(self, context: RequestContext[Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_before_transmit(context) + self._invoke_all(lambda interceptor: interceptor.read_before_transmit(context)) def read_after_transmit(self, context: ResponseContext[Any, Any, Any]) -> None: - for interceptor in self._chain: - interceptor.read_after_transmit(context) + self._invoke_all(lambda interceptor: interceptor.read_after_transmit(context)) def modify_before_deserialization( self, context: ResponseContext[Any, Any, Any] ) -> Any: - transport_response = context.transport_response for interceptor in self._chain: transport_response = interceptor.modify_before_deserialization(context) - return transport_response + _check_returned( + transport_response, interceptor, "modify_before_deserialization" + ) + if transport_response is not context.transport_response: + context = replace(context, transport_response=transport_response) + return context.transport_response def read_before_deserialization( self, context: ResponseContext[Any, Any, Any] ) -> None: - for interceptor in self._chain: - interceptor.read_before_deserialization(context) + self._invoke_all( + lambda interceptor: interceptor.read_before_deserialization(context) + ) def read_after_deserialization( self, context: OutputContext[Any, Any, Any, Any] ) -> None: - for interceptor in self._chain: - interceptor.read_after_deserialization(context) + self._invoke_all( + lambda interceptor: interceptor.read_after_deserialization(context) + ) def modify_before_attempt_completion( self, context: OutputContext[Any, Any, Any, Any | None] ) -> Any | Exception: - response = context.response for interceptor in self._chain: response = interceptor.modify_before_attempt_completion(context) - return response + _check_returned(response, interceptor, "modify_before_attempt_completion") + if response is not context.response: + context = replace(context, response=response) + return context.response def read_after_attempt( self, context: OutputContext[Any, Any, Any, Any | None] ) -> None: - for interceptor in self._chain: - interceptor.read_after_attempt(context) + self._invoke_all_passing_errors( + context, lambda interceptor, ctx: interceptor.read_after_attempt(ctx) + ) def modify_before_completion( self, context: OutputContext[Any, Any, Any | None, Any | None] ) -> Any | Exception: - response = context.response for interceptor in self._chain: - response = interceptor.modify_before_attempt_completion(context) - return response + response = interceptor.modify_before_completion(context) + _check_returned(response, interceptor, "modify_before_completion") + if response is not context.response: + context = replace(context, response=response) + return context.response def read_after_execution( self, context: OutputContext[Any, Any, Any | None, Any | None] ) -> None: + self._invoke_all_passing_errors( + context, lambda interceptor, ctx: interceptor.read_after_execution(ctx) + ) + + def _invoke_all(self, hook: Callable[[AnyInterceptor], None]) -> None: + # A failing read hook mustn't keep later interceptors from running, so + # every interceptor is invoked before the last exception is raised. + exception: Exception | None = None + for interceptor in self._chain: + try: + hook(interceptor) + except Exception as e: + if exception is not None: + _log_dropped(exception) + exception = e + if exception is not None: + raise exception + + def _invoke_all_passing_errors[C: OutputContext[Any, Any, Any, Any]]( + self, context: C, hook: Callable[[AnyInterceptor, C], None] + ) -> None: + # Like _invoke_all, but each exception also becomes the response that later + # interceptors see, since it has replaced the attempt's or execution's result. exception: Exception | None = None for interceptor in self._chain: - # Every one of these is supposed to be guaranteed to be called. try: - interceptor.read_after_execution(context) + hook(interceptor, context) except Exception as e: + if exception is not None: + _log_dropped(exception) context = replace(context, response=e) exception = e if exception is not None: raise exception + + +def _check_returned(value: Any, interceptor: AnyInterceptor, hook: str) -> None: + # A modify hook that changes its value in place and forgets to return it would + # otherwise hand None to the next interceptor, which then fails confusingly. + if value is None: + raise TypeError( + f"{interceptor!r}.{hook} returned None. Modify hooks must return the " + "value to use, even if they changed it in place." + ) + + +def _log_dropped(exception: Exception) -> None: + _LOGGER.debug( + "An interceptor raised an exception that was dropped because a later " + "interceptor raised one too.", + exc_info=exception, + ) diff --git a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py index 52cb2712a..c4420181a 100644 --- a/packages/smithy-core/tests/unit/aio/_pipeline_harness.py +++ b/packages/smithy-core/tests/unit/aio/_pipeline_harness.py @@ -147,6 +147,7 @@ async def close(self) -> None: class StubProtocol: def __init__(self) -> None: self.serialize_request_calls = 0 + self.serialized_inputs: list[Any] = [] self.set_service_endpoint_calls = 0 self.deserialize_response_calls = 0 self.create_event_publisher_calls = 0 @@ -166,6 +167,7 @@ def id(self) -> ShapeID: def serialize_request(self, **kwargs: Any) -> StubRequest: self.serialize_request_calls += 1 + self.serialized_inputs.append(kwargs["input"]) return StubRequest() def set_service_endpoint(self, *, request: Any, endpoint: Any) -> Any: @@ -198,7 +200,11 @@ def __init__(self) -> None: class StubEndpointResolver: + def __init__(self) -> None: + self.resolved_inputs: list[Any] = [] + async def resolve_endpoint(self, params: EndpointResolverParams[Any]) -> Any: + self.resolved_inputs.append(params.input) return StubEndpoint() diff --git a/packages/smithy-core/tests/unit/aio/test_client.py b/packages/smithy-core/tests/unit/aio/test_client.py index f971d142b..1e3f3d37a 100644 --- a/packages/smithy-core/tests/unit/aio/test_client.py +++ b/packages/smithy-core/tests/unit/aio/test_client.py @@ -10,7 +10,13 @@ from smithy_core.aio.eventstream import DuplexEventStream, InputEventStream from smithy_core.aio.interfaces import ClientTransport from smithy_core.auth import AuthOption -from smithy_core.exceptions import CallError, UnsupportedTransportError +from smithy_core.exceptions import CallError, SmithyError, UnsupportedTransportError +from smithy_core.interceptors import ( + InputContext, + Interceptor, + InterceptorChain, + OutputContext, +) from smithy_core.response import EMPTY_RESPONSE_METADATA from smithy_core.shapes import ShapeID from smithy_core.types import TypedProperties @@ -18,8 +24,10 @@ from ._pipeline_harness import ( DuplexTransport, NonDuplexTransport, + StubEndpointResolver, StubEvent, StubEventReceiver, + StubInput, StubOutput, UndeclaredTransport, client_call, @@ -172,6 +180,55 @@ async def test_auth_option_signer_properties_override_scheme_defaults() -> None: } +class _ReplaceInput(Interceptor[Any, Any, Any, Any]): + """Replaces the input before serialization and records what later hooks see.""" + + def __init__(self, fail_after_modifying: bool = False) -> None: + self.replacement = StubInput() + self.fail_after_modifying = fail_after_modifying + self.completion_request: Any = None + + def modify_before_serialization(self, context: InputContext[Any]) -> Any: + return self.replacement + + def read_before_serialization(self, context: InputContext[Any]) -> None: + if self.fail_after_modifying: + raise RuntimeError("boom") + + def modify_before_completion( + self, context: OutputContext[Any, Any, Any, Any] + ) -> Any: + self.completion_request = context.request + return context.response + + +async def test_modified_input_is_serialized_and_used_to_resolve_endpoint() -> None: + harness = pipeline_harness(NonDuplexTransport()) + interceptor = _ReplaceInput() + resolver = StubEndpointResolver() + call = replace( + client_call(), + interceptor=InterceptorChain([interceptor]), + endpoint_resolver=resolver, + ) + + await harness.pipeline(call) + + assert harness.protocol.serialized_inputs == [interceptor.replacement] + assert resolver.resolved_inputs == [interceptor.replacement] + + +async def test_modified_input_reaches_completion_when_serialization_fails() -> None: + harness = pipeline_harness(NonDuplexTransport()) + interceptor = _ReplaceInput(fail_after_modifying=True) + call = replace(client_call(), interceptor=InterceptorChain([interceptor])) + + with pytest.raises(SmithyError): + await harness.pipeline(call) + + assert interceptor.completion_request is interceptor.replacement + + class _CountingTransport: def __init__(self) -> None: self.close_calls = 0 diff --git a/packages/smithy-core/tests/unit/test_interceptors.py b/packages/smithy-core/tests/unit/test_interceptors.py new file mode 100644 index 000000000..1f0cf3567 --- /dev/null +++ b/packages/smithy-core/tests/unit/test_interceptors.py @@ -0,0 +1,198 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 +import logging +from typing import Any, cast +from unittest.mock import Mock + +import pytest +from smithy_core.interceptors import ( + InputContext, + Interceptor, + InterceptorChain, + OutputContext, + RequestContext, + ResponseContext, +) +from smithy_core.types import TypedProperties + + +def _context() -> OutputContext[Any, Any, Any, Any]: + # Each field holds its own name, so a hook that returns the wrong field shows up. + return OutputContext( + request=cast(Any, "request"), + transport_request="transport_request", + transport_response="transport_response", + response=cast(Any, "response"), + properties=TypedProperties(), + ) + + +READ_HOOKS = [ + "read_before_execution", + "read_before_serialization", + "read_after_serialization", + "read_before_attempt", + "read_before_signing", + "read_after_signing", + "read_before_transmit", + "read_after_transmit", + "read_before_deserialization", + "read_after_deserialization", + "read_after_attempt", + "read_after_execution", +] + +# Each modify hook mapped to the context field it replaces. +MODIFY_HOOKS = { + "modify_before_serialization": "request", + "modify_before_retry_loop": "transport_request", + "modify_before_signing": "transport_request", + "modify_before_transmit": "transport_request", + "modify_before_deserialization": "transport_response", + "modify_before_attempt_completion": "response", + "modify_before_completion": "response", +} + + +class _Appender(Interceptor[Any, Any, Any, Any]): + """Appends a suffix to the value each modify hook receives.""" + + def __init__(self, suffix: str) -> None: + self._suffix = suffix + + def modify_before_serialization(self, context: InputContext[Any]) -> Any: + return context.request + self._suffix + + def modify_before_retry_loop(self, context: RequestContext[Any, Any]) -> Any: + return context.transport_request + self._suffix + + def modify_before_signing(self, context: RequestContext[Any, Any]) -> Any: + return context.transport_request + self._suffix + + def modify_before_transmit(self, context: RequestContext[Any, Any]) -> Any: + return context.transport_request + self._suffix + + def modify_before_deserialization( + self, context: ResponseContext[Any, Any, Any] + ) -> Any: + return context.transport_response + self._suffix + + def modify_before_attempt_completion( + self, context: OutputContext[Any, Any, Any, Any] + ) -> Any: + return cast(str, context.response) + self._suffix + + def modify_before_completion( + self, context: OutputContext[Any, Any, Any, Any] + ) -> Any: + return cast(str, context.response) + self._suffix + + +@pytest.mark.parametrize("hook,field", MODIFY_HOOKS.items()) +def test_chain_passes_each_modification_to_the_next_interceptor( + hook: str, field: str +) -> None: + # This also covers a default interceptor undoing earlier changes, since the base + # hooks return the context's value just as _Appender reads it. + chain = InterceptorChain([_Appender("a"), _Appender("b")]) + + assert getattr(chain, hook)(_context()) == f"{field}ab" + + +@pytest.mark.parametrize("hook", MODIFY_HOOKS) +def test_chain_reuses_the_context_when_nothing_changes(hook: str) -> None: + context = _context() + later = Mock(spec=Interceptor) + + getattr(InterceptorChain([Interceptor(), later]), hook)(context) + + assert getattr(later, hook).call_args.args[0] is context + + +@pytest.mark.parametrize("hook", READ_HOOKS + list(MODIFY_HOOKS)) +def test_chain_calls_only_the_matching_hook(hook: str) -> None: + interceptors = [Mock(spec=Interceptor), Mock(spec=Interceptor)] + + getattr(InterceptorChain(interceptors), hook)(_context()) + + for interceptor in interceptors: + assert [name for name, *_ in interceptor.method_calls] == [hook] + + +@pytest.mark.parametrize("hook", MODIFY_HOOKS) +def test_modify_hook_returning_none_raises(hook: str) -> None: + forgot_to_return = Mock(spec=Interceptor) + getattr(forgot_to_return, hook).return_value = None + after = Mock(spec=Interceptor) + + with pytest.raises(TypeError, match=hook): + getattr(InterceptorChain([forgot_to_return, after]), hook)(_context()) + + assert not after.method_calls + + +def _raising(name: str, calls: list[str]) -> Any: + """An interceptor whose every hook records its name and raises.""" + + def raise_error(context: Any) -> None: + calls.append(name) + raise RuntimeError(name) + + interceptor = Mock() + for hook in READ_HOOKS + list(MODIFY_HOOKS): + getattr(interceptor, hook).side_effect = raise_error + return interceptor + + +@pytest.mark.parametrize("hook", READ_HOOKS) +def test_read_hook_runs_every_interceptor_and_raises_the_last_error( + hook: str, caplog: pytest.LogCaptureFixture +) -> None: + caplog.set_level(logging.DEBUG, logger="smithy_core.interceptors") + calls: list[str] = [] + chain = InterceptorChain([_raising(name, calls) for name in ("1", "2", "3")]) + + with pytest.raises(RuntimeError, match="3"): + getattr(chain, hook)(_context()) + + assert calls == ["1", "2", "3"] + dropped = [str(record.exc_info[1]) for record in caplog.records if record.exc_info] + assert dropped == ["1", "2"] + + +@pytest.mark.parametrize("hook", READ_HOOKS) +def test_read_hook_raises_when_a_later_interceptor_succeeds(hook: str) -> None: + calls: list[str] = [] + succeeds = Mock(spec=Interceptor) + chain = InterceptorChain([_raising("1", calls), succeeds]) + + with pytest.raises(RuntimeError, match="1"): + getattr(chain, hook)(_context()) + + assert [name for name, *_ in succeeds.method_calls] == [hook] + + +@pytest.mark.parametrize("hook", ["read_after_attempt", "read_after_execution"]) +def test_completion_read_hook_passes_the_error_to_later_interceptors( + hook: str, +) -> None: + later = Mock(spec=Interceptor) + chain = InterceptorChain([_raising("1", []), later]) + + with pytest.raises(RuntimeError): + getattr(chain, hook)(_context()) + + response = getattr(later, hook).call_args.args[0].response + assert isinstance(response, RuntimeError) + assert str(response) == "1" + + +@pytest.mark.parametrize("hook", MODIFY_HOOKS) +def test_modify_hook_stops_at_the_first_error(hook: str) -> None: + calls: list[str] = [] + chain = InterceptorChain([_raising(name, calls) for name in ("1", "2")]) + + with pytest.raises(RuntimeError, match="1"): + getattr(chain, hook)(_context()) + + assert calls == ["1"]