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

S3 PutObject between regions fails after @smithy/node-http-handler 3.3.2 #6805

Closed
4 tasks done
Nevon opened this issue Jan 15, 2025 · 5 comments
Closed
4 tasks done
Assignees
Labels
bug This issue is a bug. closed-for-staleness p1 This is a high priority issue potential-regression Marking this issue as a potential regression to be checked by team member queued This issues is on the AWS team's backlog workaround-available This issue has a work around available.

Comments

@Nevon
Copy link

Nevon commented Jan 15, 2025

Checkboxes for prior research

Describe the bug

When doing a PutObject request using versions of @smithy/node-http-handler equal or higher than 3.3.2 from eu-north-1 to ap-southeast-2 the request fails with RequestTimeout: Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed. after a long period of doing nothing. Making the same request using version 3.3.1 or lower works.

Regression Issue

  • Select this option if this issue appears to be a regression.

SDK version number

@aws-sdk/[email protected]
@smithy/[email protected] (or higher)

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.11.1

Reproduction Steps

To reproduce, I simply created an S3 bucket in ap-southeast-2 and issued a PutObject request from my local computer (located in Stockholm):

const s3client = new S3Client({ region: 'ap-southeast-2', logger: console });
const response = await s3client.send(new PutObjectCommand({
  Bucket: process.env.BUCKET_NAME!,
  Body: randomBytes(1024 * 100),
  Key: randomUUID(),
  ContentType: 'text/plain',
}));

Observed Behavior

After quite some time, the resulting error is:

RequestTimeout: Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed.
    at throwDefaultError ... {
  '$fault': 'client',
  '$metadata': {
    httpStatusCode: 400,
    requestId: 'VT6TYR1KW11KX7TK',
    extendedRequestId: 'az0TnxKnm01z0ioaVq72g1p22izk6qK6a2chdPs/uYIcJVRT1Vzz4w7ZuxBE7fFdeD8sPsxqHL0=',
    cfId: undefined,
    attempts: 3,
    totalRetryDelay: 84
  },
  Code: 'RequestTimeout',
  RequestId: 'VT6TYR1KW11KX7TK',
  HostId: 'az0TnxKnm01z0ioaVq72g1p22izk6qK6a2chdPs/uYIcJVRT1Vzz4w7ZuxBE7fFdeD8sPsxqHL0='
}

Expected Behavior

If I change the version of @smithy/node-http-handler to be used to 3.3.1 using resolutions, the request works.

Possible Solution

Fixed in smithy-lang/smithy-typescript#1505

Additional Information/Context

The issue happens only if it takes more than 1000ms for the server to send the 100 Continue response after the client sends the initial Expect: 100-continue header. That's why this only occurred for us when making a request from Sweden to Sydney.

@Nevon Nevon added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 15, 2025
@github-actions github-actions bot added the potential-regression Marking this issue as a potential regression to be checked by team member label Jan 15, 2025
@kuhe
Copy link
Contributor

kuhe commented Jan 15, 2025

Thanks for submitting the report and the fix PR.

To work around this in the existing 3.x version, set a requestTimeout on the client requestHandler.

// example
new S3Client({
  requestHandler: { requestTimeout: 8_000 }
});

I've approved your fix PR and will publish it soon. However, the fix will only apply to SDK clients v3.723.0 and higher which have a 4.x dependency on @smithy/node-http-handler. Please use the workaround if staying on v3.x of node-http-handler.

@kuhe
Copy link
Contributor

kuhe commented Jan 15, 2025

Before I merge the fix, I am checking the intended behavior.

@Nevon
Copy link
Author

Nevon commented Jan 15, 2025

From reading about how Expect: 100-continue works, the behavior of sending the body after 1 second even if no 100 Continue response had been received did seem quite odd to me. But that was the previous behavior, and the change that broke it did not seem to do so intentionally since presumably the server did respond with a 100 Continue, just after longer than 1 second, and if the server didn't close the connection the client would just sit there forever after the 1 second timeout. My uneducated guess is that this behavior was modeled around how cURL does it, where indeed it continues anyway after waiting for 1 second.

I've also verified that increasing the request timeout works around the issue, in case someone else comes in here from searching the internet. It's a bit unfortunate to have the request timeout pull double-duty, I feel, since I probably don't want to wait 10 seconds for just a 100 Continue even if the actual file transfer request will take that long to complete. For my particular use-case it's not a dealbreaker, however.

@kuhe
Copy link
Contributor

kuhe commented Jan 15, 2025

I'm merging the PR but also increasing the default 100-continue timeout to 6s.

The intended behavior at the 1s mark is not to send the body but to keep waiting for the 100-continue event. At 6s, it's more likely that something is wrong (like the server isn't the real S3 and didn't implement 100-continue) so it's more acceptable to attempt to send the body after this waiting period.

@aBurmeseDev aBurmeseDev removed the needs-triage This issue or PR still needs to be triaged. label Jan 15, 2025
@aBurmeseDev aBurmeseDev added the p1 This is a high priority issue label Jan 15, 2025
@kuhe kuhe added queued This issues is on the AWS team's backlog pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Jan 15, 2025
@kuhe
Copy link
Contributor

kuhe commented Jan 15, 2025

The fix was released in https://www.npmjs.com/package/@smithy/node-http-handler/v/4.0.2, which can be picked up with a fresh install or modifying the lockfile for https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.723.0 and higher.

Previous versions of the SDK using 3.x of @smithy/node-http-handler should use the requestTimeout workaround above.

@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. workaround-available This issue has a work around available. and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Jan 15, 2025
@github-actions github-actions bot added closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness p1 This is a high priority issue potential-regression Marking this issue as a potential regression to be checked by team member queued This issues is on the AWS team's backlog workaround-available This issue has a work around available.
Projects
None yet
Development

No branches or pull requests

3 participants