Skip to content

Commit

Permalink
Fix paginator bug where None was returned immediately
Browse files Browse the repository at this point in the history
The escape hatch added by aws-sdk-rust#391 did not properly handle the case where the first response was `None` and _not_ the empty string. This diff:
- Checks for emptiness for both maps and strings
- Fixes the check so that an initial `None` input does not cause an incorrect paginator error
  • Loading branch information
rcoh committed Jan 18, 2022
1 parent 171d1a4 commit fd52ddc
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
36 changes: 36 additions & 0 deletions aws/sdk/integration-tests/ec2/tests/paginators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,39 @@ async fn paginators_handle_empty_tokens() {
assert_eq!(first_item, None);
conn.assert_requests_match(&[]);
}

/// See https://github.com/awslabs/aws-sdk-rust/issues/405
///
/// EC2 can also reply with the token truly unset which will be interpreted as `None`
#[tokio::test]
async fn paginators_handle_unset_tokens() {
let request= "Action=DescribeSpotPriceHistory&Version=2016-11-15&AvailabilityZone=eu-north-1a&InstanceType.1=g5.48xlarge&ProductDescription.1=Linux%2FUNIX";
let response = r#"<?xml version="1.0" encoding="UTF-8"?>
<DescribeSpotPriceHistoryResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">
<requestId>edf3e86c-4baf-47c1-9228-9a5ea09542e8</requestId>
<spotPriceHistorySet/>
</DescribeSpotPriceHistoryResponse>"#;
let conn = TestConnection::<&str>::new(vec![(
http::Request::builder()
.uri("https://ec2.us-east-1.amazonaws.com/")
.body(request.into())
.unwrap(),
http::Response::builder()
.status(200)
.body(response)
.unwrap(),
)]);
let client = Client::from_conf_conn(stub_config(), conn.clone());
let instance_type = InstanceType::from("g5.48xlarge");
let mut paginator = client
.describe_spot_price_history()
.instance_types(instance_type)
.product_descriptions("Linux/UNIX")
.availability_zone("eu-north-1a")
.into_paginator()
.items()
.send();
let first_item = paginator.try_next().await.expect("success");
assert_eq!(first_item, None);
conn.assert_requests_match(&[]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import software.amazon.smithy.model.Model
import software.amazon.smithy.model.knowledge.PaginatedIndex
import software.amazon.smithy.model.shapes.OperationShape
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.traits.IdempotencyTokenTrait
import software.amazon.smithy.model.traits.PaginatedTrait
import software.amazon.smithy.rust.codegen.rustlang.CargoDependency
Expand Down Expand Up @@ -175,12 +174,13 @@ class PaginatorGenerator private constructor(
let done = match resp {
Ok(ref resp) => {
let new_token = #{output_token}(resp);
if new_token == input.$inputTokenMember.as_ref() {
let is_empty = ${nextTokenEmpty("new_token")};
if !is_empty && new_token == input.$inputTokenMember.as_ref() {
let _ = tx.send(Err(#{SdkError}::ConstructionFailure("next token did not change, aborting paginator. This indicates an SDK or AWS service bug.".into()))).await;
return;
}
input.$inputTokenMember = new_token.cloned();
${nextTokenEmpty("input.$inputTokenMember")}
is_empty
},
Err(_) => true,
};
Expand All @@ -192,7 +192,6 @@ class PaginatorGenerator private constructor(
return
}
}
}))
}
}
Expand Down Expand Up @@ -276,12 +275,7 @@ class PaginatorGenerator private constructor(
}

private fun nextTokenEmpty(token: String): String {
val tokenType = model.expectShape(paginationInfo.outputTokenMemberPath.last().target)
if (tokenType is StringShape) {
return "$token.as_deref().unwrap_or_default().is_empty()"
} else {
return "$token.is_none()"
}
return "$token.map(|token|token.is_empty()).unwrap_or(true)"
}

private fun pageSizeSetter() = writable {
Expand Down
11 changes: 10 additions & 1 deletion tools/ci-cdk/canary-lambda/src/paginator_canary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ pub async fn paginator_canary(client: ec2::Client, page_size: usize) -> anyhow::
bail!("should be ~60 of pages of results")
}

// https://github.com/awslabs/aws-sdk-rust/issues/405
let _ = client
.describe_vpcs()
.into_paginator()
.items()
.send()
.collect::<Result<Vec<_>, _>>()
.await?;

Ok(())
}

Expand All @@ -54,6 +63,6 @@ mod test {
async fn test_paginator() {
let conf = aws_config::load_from_env().await;
let client = aws_sdk_ec2::Client::new(&conf);
paginator_canary(client).await.unwrap()
paginator_canary(client, 20).await.unwrap()
}
}

0 comments on commit fd52ddc

Please sign in to comment.