Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't error on duplicate pagination token #1748

Merged
merged 6 commits into from
Sep 27, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,15 @@ message = "Implement support for pure Python request middleware. Improve idiomat
references = ["smithy-rs#1734"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server" }
author = "crisidev"

[[aws-sdk-rust]]
message = "Paginators now stop on encountering a duplicate token by default rather than panic. This behavior can be customized by toggling the `stop_on_duplicate_token` property on the paginator before calling `send`."
references = ["aws-sdk-rust#620", "smithy-rs#1748"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "jdisanti"

[[smithy-rs]]
message = "Paginators now stop on encountering a duplicate token by default rather than panic. This behavior can be customized by toggling the `stop_on_duplicate_token` property on the paginator before calling `send`."
references = ["aws-sdk-rust#620", "smithy-rs#1748"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"}
author = "jdisanti"
83 changes: 76 additions & 7 deletions aws/sdk/integration-tests/dynamodb/tests/paginators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ async fn paginators_handle_errors() {
}

#[tokio::test]
async fn paginators_error_on_repeated_token() {
async fn paginators_stop_on_duplicate_token_by_default() {
let response = r#"{
"Count": 1,
"Items": [{
Expand Down Expand Up @@ -212,11 +212,80 @@ async fn paginators_error_on_repeated_token() {
.get("PostedBy"),
Some(&AttributeValue::S("[email protected]".to_string()))
);
let err = rows.try_next().await.expect_err("failure");
assert!(
format!("{}", err).contains("next token did not change"),
"{}",
err
assert_eq!(
rows.try_next()
.await
.expect("no error")
.expect("not EOS")
.get("PostedBy"),
Some(&AttributeValue::S("[email protected]".to_string()))
);
assert_eq!(None, rows.try_next().await.expect("success"));
}

#[tokio::test]
async fn paginators_can_continue_on_duplicate_token() {
let response = r#"{
"Count": 1,
"Items": [{
"PostedBy": {
"S": "[email protected]"
}
}],
"LastEvaluatedKey": {
"PostedBy": { "S": "[email protected]" }
}
}"#;
// send the same response twice with the same pagination token
let conn = TestConnection::new(vec![
(
mk_request(r#"{"TableName":"test-table","Limit":32}"#),
mk_response(response),
),
(
mk_request(
r#"{"TableName":"test-table","Limit":32,"ExclusiveStartKey":{"PostedBy":{"S":"[email protected]"}}}"#,
),
mk_response(response),
),
(
mk_request(
r#"{"TableName":"test-table","Limit":32,"ExclusiveStartKey":{"PostedBy":{"S":"[email protected]"}}}"#,
),
mk_response(response),
),
]);
let client = Client::from_conf_conn(stub_config(), conn.clone());
let mut rows = client
.scan()
.table_name("test-table")
.into_paginator()
.stop_on_duplicate_token(false)
.page_size(32)
.items()
.send();
assert_eq!(
rows.try_next()
.await
.expect("no error")
.expect("not EOS")
.get("PostedBy"),
Some(&AttributeValue::S("[email protected]".to_string()))
);
assert_eq!(
rows.try_next()
.await
.expect("no error")
.expect("not EOS")
.get("PostedBy"),
Some(&AttributeValue::S("[email protected]".to_string()))
);
assert_eq!(
rows.try_next()
.await
.expect("no error")
.expect("not EOS")
.get("PostedBy"),
Some(&AttributeValue::S("[email protected]".to_string()))
);
assert_eq!(rows.try_next().await.expect("ok"), None);
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ class PaginatorGenerator private constructor(
/// Paginator for #{operation:D}
pub struct $paginatorName#{generics:W} {
handle: std::sync::Arc<crate::client::Handle${generics.inst}>,
builder: #{Builder}
builder: #{Builder},
stop_on_duplicate_token: bool,
}

impl${generics.inst} ${paginatorName}${generics.inst} #{bounds:W} {
Expand All @@ -139,13 +140,25 @@ class PaginatorGenerator private constructor(
Self {
handle,
builder,
stop_on_duplicate_token: true,
}
}

#{page_size_setter:W}

#{items_fn:W}

/// Stop paginating when the service returns the same pagination token twice in a row.
///
Comment on lines +153 to +154
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include docs on when this would be helpful to set to false:

When running status-checking operations or operations that you want to continually send in order to "watch" for the latest data (like AWS CloudWatch's GetLogEvents), set this to false. That way, you can continually call the paginator in order to receive new data as it becomes available.

/// Defaults to true.
///
/// For certain operations, it may be useful to continue on duplicate token. For example,
/// if an operation is for tailing a log file in real-time, then continuing may be desired.
/// This option can be set to `false` to accommodate these use cases.
pub fn stop_on_duplicate_token(mut self, stop_on_duplicate_token: bool) -> Self {
self.stop_on_duplicate_token = stop_on_duplicate_token;
self
}

/// Create the pagination stream
///
Expand Down Expand Up @@ -177,12 +190,12 @@ class PaginatorGenerator private constructor(
Ok(ref resp) => {
let new_token = #{output_token}(resp);
let is_empty = new_token.map(|token| token.is_empty()).unwrap_or(true);
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;
if !is_empty && new_token == input.$inputTokenMember.as_ref() && self.stop_on_duplicate_token {
true
} else {
input.$inputTokenMember = new_token.cloned();
is_empty
}
input.$inputTokenMember = new_token.cloned();
is_empty
},
Err(_) => true,
};
Expand Down