-
Notifications
You must be signed in to change notification settings - Fork 246
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
Retry after HTTP2 GOAWAY errors #738
Comments
@ozgrakkurt Thanks for submitting this. Are you actually encountering this error in production right now? |
Hey, yes. We get this regularly |
I can reproduce it easily with the following code. Version use aws_sdk_cognitoidentityprovider as cognitoidentityprovider;
use tokio;
use std::thread;
use std::time::Duration;
#[tokio::main(flavor = "current_thread")]
async fn main() {
let config = aws_config::load_from_env().await;
let client = cognitoidentityprovider::Client::new(&config);
let username = "fake_user";
let password ="fake_password";
let client_id = "fake_client";
for i in 1..4 {
let cognito_result = client.initiate_auth().set_client_id(Some(client_id.to_string())).auth_flow(cognitoidentityprovider::types::AuthFlowType::UserPasswordAuth).auth_parameters("USERNAME", username).auth_parameters("PASSWORD", password).send().await;
println!("Result {:}: {:?}", i, cognito_result);
thread::sleep(Duration::from_secs(60));
}
} Results:
|
ah perfect! thanks for the reproducer. it's a quick fix but I wasn't able to reproduce it in tests so I wasn't able to figure out if my fix actually worked. We'll get this prioritized and fixed—or we're happy to accept a PR, the relevant code is here: If we correctly classify that error is |
This PR adds two additional classes of retries tested: 1. GO_AWAY: awslabs/aws-sdk-rust#738 2. REFUSED_STREAM: awslabs/aws-sdk-rust#858 I tested 1 by using the example helpfully provided. The fix for 2 is untested and difficult to reproduce but since my fix for 1 worked, I'm confident that we're detecting the correct error class here.
This PR adds two additional classes of retries tested: 1. GO_AWAY: awslabs/aws-sdk-rust#738 2. REFUSED_STREAM: awslabs/aws-sdk-rust#858 I tested 1 by using the example helpfully provided. The fix for 2 is untested and difficult to reproduce but since my fix for 1 worked, I'm confident that we're detecting the correct error class here.
I get a similar error when trying to invoke dynamodb in Rust AWS SDK: |
This is a different error—no matching auth scheme. Can you open a separate
issue and include some logs?
…On Tue, Sep 12, 2023, 11:04 AM Harrison Kaiser ***@***.***> wrote:
I get a similar error when trying to invoke dynamodb in Rust AWS SDK:
DispatchFailure(DispatchFailure { source: ConnectorError { kind:
Other(None), source: NoMatchingAuthScheme, connection: Unknown } })
Not sure what this error means...
—
Reply to this email directly, view it on GitHub
<#738 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADYKZ4SEETLOGKTYJ5O76DX2B2XRANCNFSM6AAAAAAU2EGWYA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
This PR adds two additional classes of retries tested: 1. GO_AWAY: awslabs/aws-sdk-rust#738 2. REFUSED_STREAM: awslabs/aws-sdk-rust#858 I tested 1 by using the example helpfully provided. The fix for 2 is untested and difficult to reproduce but since my fix for 1 worked, I'm confident that we're detecting the correct error class here.
Draft pull request pending merge of #2970 ## Motivation and Context - awslabs/aws-sdk-rust#738 - awslabs/aws-sdk-rust#858 <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here --> ## Description <!--- Describe your changes in detail --> This PR adds two additional classes of retries tested: 1. GO_AWAY: awslabs/aws-sdk-rust#738 2. REFUSED_STREAM: awslabs/aws-sdk-rust#858 I tested 1 by using the example helpfully provided. The fix for 2 is untested and difficult to reproduce but since my fix for 1 worked, I'm confident that we're detecting the correct error class here. ## Testing I used the example provided by the customer and validated the H2 GO_AWAY fix. ## Checklist <!--- If a checkbox below is not applicable, then please DELETE it rather than leaving it unchecked --> - [ ] 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._
It looks like the fix for this was merged but then accidentally removed as part of a merge. We'll get a fix out soon |
The fix has been merged and will go out in a future release: smithy-lang/smithy-rs#3250 |
@jdisanti thx for the update. curiously, we stopped receiving this error, and are not sure what changed. one theory is that it was an AWS service bug that got fixed. we also wonder whether this error should be retried, after reading this particular post (we use ELB) |
Oh — this was fixed in the runtime libraries so if you ran cargo update
you'd get the new library even without a new sdk version.
…On Mon, Dec 4, 2023, 1:55 PM Fero ***@***.***> wrote:
@jdisanti <https://github.com/jdisanti> thx for the update. curiously, we
stopped receiving this error, and are not sure what changed. one theory is
that it was an AWS service bug that got fixed. we also wonder whether this
error should be retried, after reading this particular post
<https://stackoverflow.com/a/42682185> (we use ELB)
—
Reply to this email directly, view it on GitHub
<#738 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADYKZ3TDH2QPVVVFQGTGGLYHYMBNAVCNFSM6AAAAAAU2EGWYCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZZGI3TGMZSHE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
@rcoh we did run cargo update on two occasions, Nov 8th (bump from 0.56.1 to 0.57.1), and Nov 30th (bump from 0.57.1 to 1.0.1), so this is possible. |
This fix for this went out in the December 5, 2023 release. |
|
Describe the bug
[2023-02-13T11:10:36Z WARN aws_smithy_client::hyper_ext] unrecognized error from Hyper. If this error should be retried, please file an issue. err=http2 error: connection error received: not a result of an error: connection error received: not a result of an error (hyper::Error(Http2, Error { kind: GoAway(b"", NO_ERROR, Remote) }))
Expected Behavior
Should retry
Current Behavior
Error out
Reproduction Steps
none
Possible Solution
No response
Additional Information/Context
No response
Version
Environment details (OS name and version, etc.)
docker ubuntu:latest
Logs
[2023-02-13T11:10:36Z WARN aws_smithy_client::hyper_ext] unrecognized error from Hyper. If this error should be retried, please file an issue. err=http2 error: connection error received: not a result of an error: connection error received: not a result of an error (hyper::Error(Http2, Error { kind: GoAway(b"", NO_ERROR, Remote) }))
The text was updated successfully, but these errors were encountered: