Unverified Commit a9e22ce1 authored by Zelda Hessler's avatar Zelda Hessler Committed by GitHub
Browse files

Fix: orchestrator flow (#2699)

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Necessary before we can implement retries.

## Description
<!--- Describe your changes in detail -->
I noticed that we weren't handling the flow quite right when errors
occurred. This PR fixes that and adds interceptor-based tests to ensure
things are working right. I still think we could use more tests but the
PR is already quite large.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
This PR contains tests

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent a514793f
Loading
Loading
Loading
Loading
+4 −3
Original line number Diff line number Diff line
@@ -5,8 +5,9 @@

#![allow(dead_code)]

use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
use aws_smithy_runtime_api::client::interceptors::{BoxError, Interceptor, InterceptorContext};
use aws_smithy_runtime_api::client::interceptors::{
    BeforeTransmitInterceptorContextMut, BoxError, Interceptor,
};
use aws_smithy_runtime_api::config_bag::ConfigBag;
use http::header::ACCEPT;
use http::HeaderValue;
@@ -18,7 +19,7 @@ pub(crate) struct AcceptHeaderInterceptor;
impl Interceptor for AcceptHeaderInterceptor {
    fn modify_before_signing(
        &self,
        context: &mut InterceptorContext<BeforeTransmit>,
        context: &mut BeforeTransmitInterceptorContextMut<'_>,
        _cfg: &mut ConfigBag,
    ) -> Result<(), BoxError> {
        context
+13 −12
Original line number Diff line number Diff line
@@ -3,9 +3,10 @@
 * SPDX-License-Identifier: Apache-2.0
 */

use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
use aws_smithy_runtime_api::client::interceptors::error::BoxError;
use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext};
use aws_smithy_runtime_api::client::interceptors::{
    BeforeTransmitInterceptorContextMut, Interceptor,
};
use aws_smithy_runtime_api::config_bag::ConfigBag;
use http::{HeaderName, HeaderValue};
use uuid::Uuid;
@@ -38,7 +39,7 @@ impl Default for InvocationIdInterceptor {
impl Interceptor for InvocationIdInterceptor {
    fn modify_before_retry_loop(
        &self,
        context: &mut InterceptorContext<BeforeTransmit>,
        context: &mut BeforeTransmitInterceptorContextMut<'_>,
        _cfg: &mut ConfigBag,
    ) -> Result<(), BoxError> {
        let headers = context.request_mut().headers_mut();
@@ -73,31 +74,31 @@ impl InvocationId {
mod tests {
    use crate::invocation_id::InvocationIdInterceptor;
    use aws_smithy_http::body::SdkBody;
    use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
    use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext};
    use aws_smithy_runtime_api::config_bag::ConfigBag;
    use aws_smithy_runtime_api::type_erasure::TypedBox;
    use http::HeaderValue;

    fn expect_header<'a>(
        context: &'a InterceptorContext<BeforeTransmit>,
        header_name: &str,
    ) -> &'a HeaderValue {
    fn expect_header<'a>(context: &'a InterceptorContext, header_name: &str) -> &'a HeaderValue {
        context.request().headers().get(header_name).unwrap()
    }

    #[test]
    fn test_id_is_generated_and_set() {
        let mut context = InterceptorContext::<()>::new(TypedBox::new("doesntmatter").erase())
            .into_serialization_phase();
        let mut context = InterceptorContext::new(TypedBox::new("doesntmatter").erase());
        context.enter_serialization_phase();
        context.set_request(http::Request::builder().body(SdkBody::empty()).unwrap());
        let _ = context.take_input();
        let mut context = context.into_before_transmit_phase();
        context.enter_before_transmit_phase();

        let mut config = ConfigBag::base();
        let interceptor = InvocationIdInterceptor::new();
        let mut ctx = Into::into(&mut context);
        interceptor
            .modify_before_signing(&mut ctx, &mut config)
            .unwrap();
        interceptor
            .modify_before_retry_loop(&mut context, &mut config)
            .modify_before_retry_loop(&mut ctx, &mut config)
            .unwrap();

        let header = expect_header(&context, "amz-sdk-invocation-id");
+10 −7
Original line number Diff line number Diff line
@@ -3,8 +3,9 @@
 * SPDX-License-Identifier: Apache-2.0
 */

use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
use aws_smithy_runtime_api::client::interceptors::{BoxError, Interceptor, InterceptorContext};
use aws_smithy_runtime_api::client::interceptors::{
    BeforeTransmitInterceptorContextMut, BoxError, Interceptor,
};
use aws_smithy_runtime_api::config_bag::ConfigBag;
use aws_types::os_shim_internal::Env;
use http::HeaderValue;
@@ -40,7 +41,7 @@ impl RecursionDetectionInterceptor {
impl Interceptor for RecursionDetectionInterceptor {
    fn modify_before_signing(
        &self,
        context: &mut InterceptorContext<BeforeTransmit>,
        context: &mut BeforeTransmitInterceptorContextMut<'_>,
        _cfg: &mut ConfigBag,
    ) -> Result<(), BoxError> {
        let request = context.request_mut();
@@ -73,6 +74,7 @@ mod tests {
    use super::*;
    use aws_smithy_http::body::SdkBody;
    use aws_smithy_protocol_test::{assert_ok, validate_headers};
    use aws_smithy_runtime_api::client::interceptors::InterceptorContext;
    use aws_smithy_runtime_api::type_erasure::TypedBox;
    use aws_types::os_shim_internal::Env;
    use http::HeaderValue;
@@ -146,15 +148,16 @@ mod tests {
            request = request.header(name, value);
        }
        let request = request.body(SdkBody::empty()).expect("must be valid");
        let mut context = InterceptorContext::<()>::new(TypedBox::new("doesntmatter").erase())
            .into_serialization_phase();
        let mut context = InterceptorContext::new(TypedBox::new("doesntmatter").erase());
        context.enter_serialization_phase();
        context.set_request(request);
        let _ = context.take_input();
        let mut context = context.into_before_transmit_phase();
        context.enter_before_transmit_phase();
        let mut config = ConfigBag::base();

        let mut ctx = Into::into(&mut context);
        RecursionDetectionInterceptor { env }
            .modify_before_signing(&mut context, &mut config)
            .modify_before_signing(&mut ctx, &mut config)
            .expect("interceptor must succeed");
        let mutated_request = context.request();
        for name in mutated_request.headers().keys() {
+10 −12
Original line number Diff line number Diff line
@@ -4,8 +4,9 @@
 */

use aws_smithy_runtime::client::orchestrator::interceptors::{RequestAttempts, ServiceClockSkew};
use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
use aws_smithy_runtime_api::client::interceptors::{BoxError, Interceptor, InterceptorContext};
use aws_smithy_runtime_api::client::interceptors::{
    BeforeTransmitInterceptorContextMut, BoxError, Interceptor,
};
use aws_smithy_runtime_api::config_bag::ConfigBag;
use aws_smithy_types::date_time::Format;
use aws_smithy_types::retry::RetryConfig;
@@ -79,7 +80,7 @@ impl RequestInfoInterceptor {
impl Interceptor for RequestInfoInterceptor {
    fn modify_before_transmit(
        &self,
        context: &mut InterceptorContext<BeforeTransmit>,
        context: &mut BeforeTransmitInterceptorContextMut<'_>,
        cfg: &mut ConfigBag,
    ) -> Result<(), BoxError> {
        let mut pairs = RequestPairs::new();
@@ -156,7 +157,6 @@ mod tests {
    use crate::request_info::RequestPairs;
    use aws_smithy_http::body::SdkBody;
    use aws_smithy_runtime::client::orchestrator::interceptors::RequestAttempts;
    use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
    use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext};
    use aws_smithy_runtime_api::config_bag::ConfigBag;
    use aws_smithy_runtime_api::type_erasure::TypedBox;
@@ -165,10 +165,7 @@ mod tests {
    use http::HeaderValue;
    use std::time::Duration;

    fn expect_header<'a>(
        context: &'a InterceptorContext<BeforeTransmit>,
        header_name: &str,
    ) -> &'a str {
    fn expect_header<'a>(context: &'a InterceptorContext, header_name: &str) -> &'a str {
        context
            .request()
            .headers()
@@ -180,8 +177,8 @@ mod tests {

    #[test]
    fn test_request_pairs_for_initial_attempt() {
        let context = InterceptorContext::<()>::new(TypedBox::new("doesntmatter").erase());
        let mut context = context.into_serialization_phase();
        let mut context = InterceptorContext::new(TypedBox::new("doesntmatter").erase());
        context.enter_serialization_phase();
        context.set_request(http::Request::builder().body(SdkBody::empty()).unwrap());

        let mut config = ConfigBag::base();
@@ -194,10 +191,11 @@ mod tests {
        config.put(RequestAttempts::new());

        let _ = context.take_input();
        let mut context = context.into_before_transmit_phase();
        context.enter_before_transmit_phase();
        let interceptor = RequestInfoInterceptor::new();
        let mut ctx = (&mut context).into();
        interceptor
            .modify_before_transmit(&mut context, &mut config)
            .modify_before_transmit(&mut ctx, &mut config)
            .unwrap();

        assert_eq!(
+19 −15
Original line number Diff line number Diff line
@@ -4,9 +4,10 @@
 */

use aws_http::user_agent::{ApiMetadata, AwsUserAgent};
use aws_smithy_runtime_api::client::interceptors::context::phase::BeforeTransmit;
use aws_smithy_runtime_api::client::interceptors::error::BoxError;
use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext};
use aws_smithy_runtime_api::client::interceptors::{
    BeforeTransmitInterceptorContextMut, Interceptor,
};
use aws_smithy_runtime_api::config_bag::ConfigBag;
use aws_types::app_name::AppName;
use aws_types::os_shim_internal::Env;
@@ -73,7 +74,7 @@ fn header_values(
impl Interceptor for UserAgentInterceptor {
    fn modify_before_signing(
        &self,
        context: &mut InterceptorContext<BeforeTransmit>,
        context: &mut BeforeTransmitInterceptorContextMut<'_>,
        cfg: &mut ConfigBag,
    ) -> Result<(), BoxError> {
        let api_metadata = cfg
@@ -113,10 +114,7 @@ mod tests {
    use aws_smithy_runtime_api::type_erasure::TypedBox;
    use aws_smithy_types::error::display::DisplayErrorContext;

    fn expect_header<'a>(
        context: &'a InterceptorContext<BeforeTransmit>,
        header_name: &str,
    ) -> &'a str {
    fn expect_header<'a>(context: &'a InterceptorContext, header_name: &str) -> &'a str {
        context
            .request()
            .headers()
@@ -126,12 +124,13 @@ mod tests {
            .unwrap()
    }

    fn context() -> InterceptorContext<BeforeTransmit> {
        let mut context = InterceptorContext::<()>::new(TypedBox::new("doesntmatter").erase())
            .into_serialization_phase();
    fn context() -> InterceptorContext {
        let mut context = InterceptorContext::new(TypedBox::new("doesntmatter").erase());
        context.enter_serialization_phase();
        context.set_request(http::Request::builder().body(SdkBody::empty()).unwrap());
        let _ = context.take_input();
        context.into_before_transmit_phase()
        context.enter_before_transmit_phase();
        context
    }

    #[test]
@@ -143,8 +142,9 @@ mod tests {
        config.put(ApiMetadata::new("unused", "unused"));

        let interceptor = UserAgentInterceptor::new();
        let mut ctx = Into::into(&mut context);
        interceptor
            .modify_before_signing(&mut context, &mut config)
            .modify_before_signing(&mut ctx, &mut config)
            .unwrap();

        let header = expect_header(&context, "user-agent");
@@ -166,8 +166,9 @@ mod tests {
        config.put(api_metadata.clone());

        let interceptor = UserAgentInterceptor::new();
        let mut ctx = Into::into(&mut context);
        interceptor
            .modify_before_signing(&mut context, &mut config)
            .modify_before_signing(&mut ctx, &mut config)
            .unwrap();

        let expected_ua = AwsUserAgent::new_from_environment(Env::real(), api_metadata);
@@ -195,8 +196,9 @@ mod tests {
        config.put(AppName::new("my_awesome_app").unwrap());

        let interceptor = UserAgentInterceptor::new();
        let mut ctx = Into::into(&mut context);
        interceptor
            .modify_before_signing(&mut context, &mut config)
            .modify_before_signing(&mut ctx, &mut config)
            .unwrap();

        let app_value = "app/my_awesome_app";
@@ -219,11 +221,13 @@ mod tests {
        let mut config = ConfigBag::base();

        let interceptor = UserAgentInterceptor::new();
        let mut ctx = Into::into(&mut context);

        let error = format!(
            "{}",
            DisplayErrorContext(
                &*interceptor
                    .modify_before_signing(&mut context, &mut config)
                    .modify_before_signing(&mut ctx, &mut config)
                    .expect_err("it should error")
            )
        );
Loading