From dabbfaa8e395907ff1e68b271f3d16069abac8b4 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Thu, 8 Jun 2023 13:31:52 -0400 Subject: [PATCH] Add the ability to disable interceptors via the config bag (#2757) This replaces the existing custom interceptor disable logic with shared logic to allow generally disabling any public interceptors from Runtime plugins. The previous behavior was very brittle because it relied heavily on runtime plugin execution order. ## Motivation and Context - simplify presigning behavior - generalize logic to disable interceptors ## Description Create `disable_interceptor` struct, which, when inserted into the configuration bag can disable an interceptor via the `Interceptors` execution interface. ## Testing - ` (cd aws/sdk/build/aws-sdk/sdk/s3 && cargo test --test presigning)` ## Checklist - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the smithy-rs codegen or runtime crates - [ ] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../src/presigning_interceptors.rs | 17 +- .../aws-runtime/src/invocation_id.rs | 17 -- .../aws-runtime/src/request_info.rs | 17 -- .../aws-runtime/src/user_agent.rs | 17 -- .../smithy/rustsdk/InvocationIdDecorator.kt | 11 +- .../RetryInformationHeaderDecorator.kt | 26 +- .../smithy/rustsdk/UserAgentDecorator.kt | 16 +- design/src/client/orchestrator.md | 2 +- .../src/client/interceptors.rs | 237 ++++++++++++++---- 9 files changed, 209 insertions(+), 151 deletions(-) diff --git a/aws/rust-runtime/aws-inlineable/src/presigning_interceptors.rs b/aws/rust-runtime/aws-inlineable/src/presigning_interceptors.rs index af26912ddd..18b94723a2 100644 --- a/aws/rust-runtime/aws-inlineable/src/presigning_interceptors.rs +++ b/aws/rust-runtime/aws-inlineable/src/presigning_interceptors.rs @@ -8,13 +8,14 @@ use crate::presigning::PresigningConfig; use crate::serialization_settings::HeaderSerializationSettings; use aws_runtime::auth::sigv4::{HttpSignatureType, SigV4OperationSigningConfig}; -use aws_runtime::invocation_id::DisableInvocationIdInterceptor; -use aws_runtime::request_info::DisableRequestInfoInterceptor; -use aws_runtime::user_agent::DisableUserAgentInterceptor; +use aws_runtime::invocation_id::InvocationIdInterceptor; +use aws_runtime::request_info::RequestInfoInterceptor; +use aws_runtime::user_agent::UserAgentInterceptor; use aws_smithy_async::time::{SharedTimeSource, StaticTimeSource}; use aws_smithy_runtime_api::client::interceptors::{ - BeforeSerializationInterceptorContextMut, BeforeTransmitInterceptorContextMut, BoxError, - Interceptor, InterceptorRegistrar, SharedInterceptor, + disable_interceptor, BeforeSerializationInterceptorContextMut, + BeforeTransmitInterceptorContextMut, BoxError, Interceptor, InterceptorRegistrar, + SharedInterceptor, }; use aws_smithy_runtime_api::client::orchestrator::ConfigBagAccessors; use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; @@ -92,9 +93,9 @@ impl RuntimePlugin for SigV4PresigningRuntimePlugin { interceptors: &mut InterceptorRegistrar, ) -> Result<(), BoxError> { // Disable some SDK interceptors that shouldn't run for presigning - cfg.put(DisableInvocationIdInterceptor::new("presigning")); - cfg.put(DisableRequestInfoInterceptor::new("presigning")); - cfg.put(DisableUserAgentInterceptor::new("presigning")); + cfg.put(disable_interceptor::("presigning")); + cfg.put(disable_interceptor::("presigning")); + cfg.put(disable_interceptor::("presigning")); // Register the presigning interceptor interceptors.register(self.interceptor.clone()); diff --git a/aws/rust-runtime/aws-runtime/src/invocation_id.rs b/aws/rust-runtime/aws-runtime/src/invocation_id.rs index 2ff073f822..420cedb73d 100644 --- a/aws/rust-runtime/aws-runtime/src/invocation_id.rs +++ b/aws/rust-runtime/aws-runtime/src/invocation_id.rs @@ -18,23 +18,6 @@ pub use test_util::{NoInvocationIdGenerator, PredefinedInvocationIdGenerator}; #[allow(clippy::declare_interior_mutable_const)] // we will never mutate this const AMZ_SDK_INVOCATION_ID: HeaderName = HeaderName::from_static("amz-sdk-invocation-id"); -/// Config marker that disables the invocation ID interceptor. -#[doc(hidden)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct DisableInvocationIdInterceptor { - why: &'static str, -} - -impl DisableInvocationIdInterceptor { - /// Creates a new `DisableInvocationIdInterceptor`. - /// - /// Takes a human readable string for the `Debug` impl to state why it is being disabled. - /// This is to assist with debugging issues with requests. - pub fn new(why: &'static str) -> Self { - Self { why } - } -} - /// A generator for returning new invocation IDs on demand. pub trait InvocationIdGenerator: Debug + Send + Sync { /// Call this function to receive a new [`InvocationId`] or an error explaining why one couldn't diff --git a/aws/rust-runtime/aws-runtime/src/request_info.rs b/aws/rust-runtime/aws-runtime/src/request_info.rs index 706872dd29..0b41310536 100644 --- a/aws/rust-runtime/aws-runtime/src/request_info.rs +++ b/aws/rust-runtime/aws-runtime/src/request_info.rs @@ -20,23 +20,6 @@ use std::time::{Duration, SystemTime}; #[allow(clippy::declare_interior_mutable_const)] // we will never mutate this const AMZ_SDK_REQUEST: HeaderName = HeaderName::from_static("amz-sdk-request"); -/// Config marker that disables the invocation ID interceptor. -#[doc(hidden)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct DisableRequestInfoInterceptor { - why: &'static str, -} - -impl DisableRequestInfoInterceptor { - /// Creates a new `DisableRequestInfoInterceptor`. - /// - /// Takes a human readable string for the `Debug` impl to state why it is being disabled. - /// This is to assist with debugging issues with requests. - pub fn new(why: &'static str) -> Self { - Self { why } - } -} - /// Generates and attaches a request header that communicates request-related metadata. /// Examples include: /// diff --git a/aws/rust-runtime/aws-runtime/src/user_agent.rs b/aws/rust-runtime/aws-runtime/src/user_agent.rs index 12469fa13d..52c6cdb4da 100644 --- a/aws/rust-runtime/aws-runtime/src/user_agent.rs +++ b/aws/rust-runtime/aws-runtime/src/user_agent.rs @@ -49,23 +49,6 @@ impl From for UserAgentInterceptorError { } } -/// Config marker that disables the user agent interceptor. -#[doc(hidden)] -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub struct DisableUserAgentInterceptor { - why: &'static str, -} - -impl DisableUserAgentInterceptor { - /// Creates a new `DisableUserAgentInterceptor`. - /// - /// Takes a human readable string for the `Debug` impl to state why it is being disabled. - /// This is to assist with debugging issues with requests. - pub fn new(why: &'static str) -> Self { - Self { why } - } -} - /// Generates and attaches the AWS SDK's user agent to a HTTP request #[non_exhaustive] #[derive(Debug, Default)] diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt index 2b52c2b33a..e26ddbec76 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/InvocationIdDecorator.kt @@ -10,7 +10,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.customize.ClientCodegen import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection import software.amazon.smithy.rust.codegen.core.rustlang.Writable -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.util.letIf @@ -38,14 +37,8 @@ private class InvocationIdRuntimePluginCustomization( override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.AdditionalConfig) { - val invocationId = AwsRuntimeType.awsRuntime(codegenContext.runtimeConfig).resolve("invocation_id") - rustBlockTemplate( - "if cfg.get::<#{DisableInvocationIdInterceptor}>().is_none()", - "DisableInvocationIdInterceptor" to invocationId.resolve("DisableInvocationIdInterceptor"), - ) { - section.registerInterceptor(codegenContext.runtimeConfig, this) { - rustTemplate("#{InvocationIdInterceptor}::new()", *codegenScope) - } + section.registerInterceptor(codegenContext.runtimeConfig, this) { + rustTemplate("#{InvocationIdInterceptor}::new()", *codegenScope) } } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt index f984608b41..69cf7f9b15 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/RetryInformationHeaderDecorator.kt @@ -11,7 +11,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRunti import software.amazon.smithy.rust.codegen.client.smithy.generators.ServiceRuntimePluginSection import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.util.letIf @@ -37,22 +36,17 @@ private class AddRetryInformationHeaderInterceptors(codegenContext: ClientCodege override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.AdditionalConfig) { - rustBlockTemplate( - "if cfg.get::<#{DisableRequestInfoInterceptor}>().is_none()", - "DisableRequestInfoInterceptor" to awsRuntime.resolve("request_info::DisableRequestInfoInterceptor"), - ) { - // Track the latency between client and server. - section.registerInterceptor(runtimeConfig, this) { - rust( - "#T::new()", - smithyRuntime.resolve("client::orchestrator::interceptors::ServiceClockSkewInterceptor"), - ) - } + // Track the latency between client and server. + section.registerInterceptor(runtimeConfig, this) { + rust( + "#T::new()", + smithyRuntime.resolve("client::orchestrator::interceptors::ServiceClockSkewInterceptor"), + ) + } - // Add request metadata to outgoing requests. Sets a header. - section.registerInterceptor(runtimeConfig, this) { - rust("#T::new()", awsRuntime.resolve("request_info::RequestInfoInterceptor")) - } + // Add request metadata to outgoing requests. Sets a header. + section.registerInterceptor(runtimeConfig, this) { + rust("#T::new()", awsRuntime.resolve("request_info::RequestInfoInterceptor")) } } } diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt index c3595d3a32..7ead51d18b 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/UserAgentDecorator.kt @@ -18,7 +18,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.config.Confi import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ServiceConfig import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.rust -import software.amazon.smithy.rust.codegen.core.rustlang.rustBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig @@ -105,16 +104,11 @@ class UserAgentDecorator : ClientCodegenDecorator { override fun section(section: ServiceRuntimePluginSection): Writable = writable { if (section is ServiceRuntimePluginSection.AdditionalConfig) { - rustBlockTemplate( - "if cfg.get::<#{DisableUserAgentInterceptor}>().is_none()", - "DisableUserAgentInterceptor" to awsRuntime.resolve("user_agent::DisableUserAgentInterceptor"), - ) { - section.putConfigValue(this) { - rust("#T.clone()", ClientRustModule.Meta.toType().resolve("API_METADATA")) - } - section.registerInterceptor(runtimeConfig, this) { - rust("#T::new()", awsRuntime.resolve("user_agent::UserAgentInterceptor")) - } + section.putConfigValue(this) { + rust("#T.clone()", ClientRustModule.Meta.toType().resolve("API_METADATA")) + } + section.registerInterceptor(runtimeConfig, this) { + rust("#T::new()", awsRuntime.resolve("user_agent::UserAgentInterceptor")) } } } diff --git a/design/src/client/orchestrator.md b/design/src/client/orchestrator.md index d23088dcc5..d0330905bd 100644 --- a/design/src/client/orchestrator.md +++ b/design/src/client/orchestrator.md @@ -52,7 +52,7 @@ The orchestrator's work is divided into four phases: *NOTE: If an interceptor fails, then the other interceptors for that lifecycle event are still run. All resulting errors are collected and emitted together.* 0. **Building the `ConfigBag` and mounting interceptors**. - - *This phase is infallible.* + - *This phase is fallible.* - An interceptor context is created. This will hold request and response objects, making them available to interceptors. - All runtime plugins set at the client-level are run. These plugins can set config and mount interceptors. Any _"read before execution"_ interceptors that have been set get run. - All runtime plugins set at the operation-level are run. These plugins can also set config and mount interceptors. Any new _"read before execution"_ interceptors that have been set get run. diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs index 12722aef6c..fe1f7e535d 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs @@ -22,6 +22,8 @@ pub use context::{ }; use context::{Error, Input, Output}; pub use error::{BoxError, InterceptorError}; +use std::fmt::{Debug, Formatter}; +use std::marker::PhantomData; use std::ops::Deref; use std::sync::Arc; @@ -578,38 +580,66 @@ pub trait Interceptor: std::fmt::Debug { } /// Interceptor wrapper that may be shared -#[derive(Debug, Clone)] -pub struct SharedInterceptor(Arc); +#[derive(Clone)] +pub struct SharedInterceptor { + interceptor: Arc, + check_enabled: Arc bool + Send + Sync>, +} + +impl Debug for SharedInterceptor { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("SharedInterceptor") + .field("interceptor", &self.interceptor) + .finish() + } +} impl SharedInterceptor { /// Create a new `SharedInterceptor` from `Interceptor` - pub fn new(interceptor: impl Interceptor + Send + Sync + 'static) -> Self { - Self(Arc::new(interceptor)) + pub fn new(interceptor: T) -> Self { + Self { + interceptor: Arc::new(interceptor), + check_enabled: Arc::new(|conf: &ConfigBag| { + conf.get::>().is_none() + }), + } + } + + fn enabled(&self, conf: &ConfigBag) -> bool { + (self.check_enabled)(conf) } } -impl AsRef for SharedInterceptor { - fn as_ref(&self) -> &(dyn Interceptor + 'static) { - self.0.as_ref() +/// A interceptor wrapper to conditionally enable the interceptor based on [`DisableInterceptor`] +struct ConditionallyEnabledInterceptor<'a>(&'a SharedInterceptor); +impl ConditionallyEnabledInterceptor<'_> { + fn if_enabled(&self, cfg: &ConfigBag) -> Option<&dyn Interceptor> { + if self.0.enabled(cfg) { + Some(self.0.as_ref()) + } else { + None + } } } -impl From> for SharedInterceptor { - fn from(interceptor: Arc) -> Self { - SharedInterceptor(interceptor) +impl AsRef for SharedInterceptor { + fn as_ref(&self) -> &(dyn Interceptor + 'static) { + self.interceptor.as_ref() } } impl Deref for SharedInterceptor { type Target = Arc; fn deref(&self) -> &Self::Target { - &self.0 + &self.interceptor } } /// Collection of [`SharedInterceptor`] that allows for only registration #[derive(Debug, Clone, Default)] -pub struct InterceptorRegistrar(Vec); +pub struct InterceptorRegistrar { + interceptors: Vec, +} impl InterceptorRegistrar { /// Register an interceptor with this `InterceptorRegistrar`. @@ -617,7 +647,7 @@ impl InterceptorRegistrar { /// When this `InterceptorRegistrar` is passed to an orchestrator, the orchestrator will run the /// registered interceptor for all the "hooks" that it implements. pub fn register(&mut self, interceptor: SharedInterceptor) { - self.0.push(interceptor); + self.interceptors.push(interceptor); } } @@ -645,11 +675,13 @@ macro_rules! interceptor_impl_fn { let mut result: Result<(), BoxError> = Ok(()); let mut ctx = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.$interceptor(&mut ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.$interceptor(&mut ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::$interceptor) @@ -664,11 +696,13 @@ macro_rules! interceptor_impl_fn { let mut result: Result<(), BoxError> = Ok(()); let ctx = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.$interceptor(&ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.$interceptor(&ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::$interceptor) @@ -676,18 +710,47 @@ macro_rules! interceptor_impl_fn { }; } +/// Generalized interceptor disabling interface +/// +/// RuntimePlugins can disable interceptors by inserting [`DisableInterceptor`](DisableInterceptor) into the config bag +#[must_use] +#[derive(Debug)] +pub struct DisableInterceptor { + _t: PhantomData, + #[allow(unused)] + cause: &'static str, +} + +/// Disable an interceptor with a given cause +pub fn disable_interceptor(cause: &'static str) -> DisableInterceptor { + DisableInterceptor { + _t: PhantomData::default(), + cause, + } +} + impl Interceptors { pub fn new() -> Self { Self::default() } - fn interceptors(&self) -> impl Iterator { - // Since interceptors can modify the interceptor list (since its in the config bag), copy the list ahead of time. - // This should be cheap since the interceptors inside the list are Arcs. + fn interceptors(&self) -> impl Iterator> { + self.client_interceptors() + .chain(self.operation_interceptors()) + } + + fn client_interceptors(&self) -> impl Iterator> { self.client_interceptors - .0 + .interceptors + .iter() + .map(ConditionallyEnabledInterceptor) + } + + fn operation_interceptors(&self) -> impl Iterator> { + self.operation_interceptors + .interceptors .iter() - .chain(self.operation_interceptors.0.iter()) + .map(ConditionallyEnabledInterceptor) } pub fn client_interceptors_mut(&mut self) -> &mut InterceptorRegistrar { @@ -705,12 +768,14 @@ impl Interceptors { ) -> Result<(), InterceptorError> { let mut result: Result<(), BoxError> = Ok(()); let ctx: BeforeSerializationInterceptorContextRef<'_> = ctx.into(); - for interceptor in self.client_interceptors.0.iter() { - if let Err(new_error) = interceptor.read_before_execution(&ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + for interceptor in self.client_interceptors() { + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.read_before_execution(&ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::read_before_execution) @@ -723,12 +788,14 @@ impl Interceptors { ) -> Result<(), InterceptorError> { let mut result: Result<(), BoxError> = Ok(()); let ctx: BeforeSerializationInterceptorContextRef<'_> = ctx.into(); - for interceptor in self.operation_interceptors.0.iter() { - if let Err(new_error) = interceptor.read_before_execution(&ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + for interceptor in self.operation_interceptors() { + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.read_before_execution(&ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::read_before_execution) @@ -757,11 +824,14 @@ impl Interceptors { let mut result: Result<(), BoxError> = Ok(()); let mut ctx: FinalizerInterceptorContextMut<'_> = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.modify_before_attempt_completion(&mut ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.modify_before_attempt_completion(&mut ctx, cfg) + { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::modify_before_attempt_completion) @@ -775,11 +845,13 @@ impl Interceptors { let mut result: Result<(), BoxError> = Ok(()); let ctx: FinalizerInterceptorContextRef<'_> = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.read_after_attempt(&ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.read_after_attempt(&ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::read_after_attempt) @@ -793,11 +865,13 @@ impl Interceptors { let mut result: Result<(), BoxError> = Ok(()); let mut ctx: FinalizerInterceptorContextMut<'_> = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.modify_before_completion(&mut ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.modify_before_completion(&mut ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::modify_before_completion) @@ -811,11 +885,13 @@ impl Interceptors { let mut result: Result<(), BoxError> = Ok(()); let ctx: FinalizerInterceptorContextRef<'_> = ctx.into(); for interceptor in self.interceptors() { - if let Err(new_error) = interceptor.read_after_execution(&ctx, cfg) { - if let Err(last_error) = result { - tracing::debug!("{}", DisplayErrorContext(&*last_error)); + if let Some(interceptor) = interceptor.if_enabled(cfg) { + if let Err(new_error) = interceptor.read_after_execution(&ctx, cfg) { + if let Err(last_error) = result { + tracing::debug!("{}", DisplayErrorContext(&*last_error)); + } + result = Err(new_error); } - result = Err(new_error); } } result.map_err(InterceptorError::read_after_execution) @@ -824,7 +900,12 @@ impl Interceptors { #[cfg(test)] mod tests { - use crate::client::interceptors::{Interceptor, InterceptorRegistrar, SharedInterceptor}; + use crate::client::interceptors::context::Input; + use crate::client::interceptors::{ + disable_interceptor, BeforeTransmitInterceptorContextRef, BoxError, Interceptor, + InterceptorContext, InterceptorRegistrar, Interceptors, SharedInterceptor, + }; + use aws_smithy_types::config_bag::ConfigBag; #[derive(Debug)] struct TestInterceptor; @@ -834,7 +915,7 @@ mod tests { fn register_interceptor() { let mut registrar = InterceptorRegistrar::default(); registrar.register(SharedInterceptor::new(TestInterceptor)); - assert_eq!(1, registrar.0.len()); + assert_eq!(1, registrar.interceptors.len()); } #[test] @@ -843,6 +924,52 @@ mod tests { let number_of_interceptors = 3; let interceptors = vec![SharedInterceptor::new(TestInterceptor); number_of_interceptors]; registrar.extend(interceptors); - assert_eq!(number_of_interceptors, registrar.0.len()); + assert_eq!(number_of_interceptors, registrar.interceptors.len()); + } + + #[test] + fn test_disable_interceptors() { + #[derive(Debug)] + struct PanicInterceptor; + impl Interceptor for PanicInterceptor { + fn read_before_transmit( + &self, + _context: &BeforeTransmitInterceptorContextRef<'_>, + _cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + Err("boom".into()) + } + } + let mut interceptors = Interceptors::new(); + let interceptors_vec = vec![ + SharedInterceptor::new(PanicInterceptor), + SharedInterceptor::new(TestInterceptor), + ]; + interceptors + .client_interceptors_mut() + .extend(interceptors_vec); + let mut cfg = ConfigBag::base(); + assert_eq!( + interceptors + .interceptors() + .filter(|i| i.if_enabled(&cfg).is_some()) + .count(), + 2 + ); + interceptors + .read_before_transmit(&mut InterceptorContext::new(Input::new(5)), &mut cfg) + .expect_err("interceptor returns error"); + cfg.put(disable_interceptor::("test")); + assert_eq!( + interceptors + .interceptors() + .filter(|i| i.if_enabled(&cfg).is_some()) + .count(), + 1 + ); + // shouldn't error because interceptors won't run + interceptors + .read_before_transmit(&mut InterceptorContext::new(Input::new(5)), &mut cfg) + .expect("interceptor is now disabled"); } }