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

[node-http-handler 100-continue] When an error occurs in the request, there is no waiting time for the cotinue event #4799

Closed
3 tasks done
bcshmily opened this issue Jun 7, 2023 · 2 comments
Assignees
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. p2 This is a standard priority issue

Comments

@bcshmily
Copy link

bcshmily commented Jun 7, 2023

Checkboxes for prior research

Describe the bug

When an error occurs in the request, there is no waiting time for the cotinue event,
If the socketTimeout set is relatively long, Lambda will timeout

SDK version number

@aws-sdk/node-http-handler": "3.344.0"

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.16.0

Reproduction Steps

set socketTimeout of S3Client  to 15 minute
s3 = new S3Client({
      apiVersion: "2006-03-01",
      requestHandler: new NodeHttpHandler({
        socketTimeout: 15 * 60 * 1000
      }),
    });

Usually, uploading files only takes one minute, but sometimes Lambda times out

    const parallelUploadS3 = new Upload({
      client: s3,
      params: {
        Bucket: bucket,
        Key: key,
        Body: body
      }
    });
    await parallelUploadS3.done();

Observed Behavior

Lambda timeout

Expected Behavior

The file can be uploaded successfully

Possible Solution

Possible reason: When an error occurs in the request, there is no waiting time for the cotinue event, so the configured 15 minute timeout will be applied

packages/node-http-handler/src/write-request-body.ts

export async function writeRequestBody(
  httpRequest: ClientRequest | ClientHttp2Stream,
  request: HttpRequest,
  maxContinueTimeoutMs = MIN_WAIT_TIME
): Promise<void> {
  const headers = request.headers ?? {};
  const expect = headers["Expect"] || headers["expect"];

  let timeoutId = -1;
  let hasError = false;
  if (expect === "100-continue") {
    await Promise.race<void>([
      new Promise((resolve) => {
        timeoutId = Number(setTimeout(resolve, Math.max(MIN_WAIT_TIME, maxContinueTimeoutMs)));
      }),
      new Promise((resolve) => {
        httpRequest.on("continue", () => {
          clearTimeout(timeoutId);
          resolve();
        });
        httpRequest.on("error", () => {
          hasError = true;
          clearTimeout(timeoutId);
          resolve();
        });
      }),
    ]);
  }
  if(!hasError) {
      writeBody(httpRequest, request.body);
  }
}

Additional Information/Context

v3.329.0 is work fine.

@bcshmily bcshmily added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 7, 2023
@yenfryherrerafeliz yenfryherrerafeliz self-assigned this Jun 7, 2023
@kuhe kuhe self-assigned this Jun 7, 2023
@kuhe kuhe added queued This issues is on the AWS team's backlog p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jun 7, 2023
@kuhe kuhe added pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed queued This issues is on the AWS team's backlog labels Jun 8, 2023
@kuhe
Copy link
Contributor

kuhe commented Jun 9, 2023

released in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.350.0

@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Jun 9, 2023
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

3 participants