@aws-sdk/client-s3 Passing a stream Body to PutObjectCommand will hang if a retry happens

[jeanbmar]

Checkboxes for prior research

• I've gone through Developer Guide and API reference
• I've checked AWS Forums and StackOverflow.
• I've searched for previous similar issues and didn't find any solution.

Describe the bug

I work on a project that needs to upload a few hundred files to S3 a few times per day. This happens in Lambda.

I'm using a single PutObjectCommand for each file and pass bodies as Readable streams. The ContentLength property of PutObjectCommand is properly set.

I noticed Lambda timeouts on PutObjectCommand send calls, that I couldn't explain, happening randomly.

I monkey patched @smithy/node-http-handler, and figured it happened when I was receiving 503 Slow Down errors from AWS. Fine? No, because the 15s timeout from my Lambda didn't match the retry delay which is between 100-1000ms. It should have been more than enough.

After more monkey patching, I found the cause:

When a Readable is passed to PutObjectCommand, the stream will be consumed by the http request through:

https://github.com/awslabs/smithy-typescript/blob/a4b58b32ac2ae778917e276ba381527f551c2d3d/packages/node-http-handler/src/write-request-body.ts#L60C7-L60C7

function writeBody(
  httpRequest: ClientRequest | ClientHttp2Stream,
  body?: string | ArrayBuffer | ArrayBufferView | Readable | Uint8Array
) {
  if (body instanceof Readable) {
    // pipe automatically handles end
    body.pipe(httpRequest); // <---------------- here
  } else if (body) {
    httpRequest.end(Buffer.from(body as Parameters<typeof Buffer.from>[0]));
  } else {
    httpRequest.end();
  }
}

Now what happens when there's some throttling involved and the request has to be retried? The stream is already consumed and ended. The end event from the Readable will never be emitted again. The http request will hang instead of creating a response:

https://github.com/awslabs/smithy-typescript/blob/a4b58b32ac2ae778917e276ba381527f551c2d3d/packages/node-http-handler/src/node-http-handler.ts#L148-L156

const req = requestFunc(nodeHttpsOptions, (res) => {
    // <--------------- this block will never happen on retry because the body stream can't end again, preventing the request itself from ending
    const httpResponse = new HttpResponse({
        statusCode: res.statusCode || -1,
        reason: res.statusMessage,
        headers: getTransformedHeaders(res.headers),
        body: res,
    });
    resolve({ response: httpResponse });
});

I'm surprised such a major issue was never noticed. It makes streams impossible to use with PutObjectCommand (and possibly others) as soon as there's some volume involved since S3 sends 503 when scaling.

SDK version number

@aws-sdk/client-s3@3.435.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.17.0

Reproduction Steps

Loop on this until S3 replies with a 503 Slow down error, the send promise won't resolve when retrying.

const buffer = Buffer.alloc(10000, 'a');
const stream = Readable.from(buffer);
const s3Client = new S3Client();
const input = {
  Bucket: bucketName,
  Key: key,
  Body: stream,
  ContentLength: buffer.length,
};
await s3Client.send(new PutObjectCommand(input));

Observed Behavior

The send promise is hanging.

Expected Behavior

The send promise should either properly resolve after the retry delay or reject with a message stating than body streams cannot be retried.

Possible Solution

Use a PassThrough stream between the body and the http request to copy flowing data in a buffer, if ContentLength is reasonably small. Use the buffer when a retry happens (or throw if ContentLength was too large).

Additional Information/Context

No response

[RanVaknin]

Hi @jeanbmar,
Thanks for reaching out. I see how this could be an issue and I admit I dont remember seeing this before.

Im not sure about your use case here, but in your repro example you are creating the stream from a buffer. In that case you can supply the buffer directly to the Body parameter, this would make it so that the SDK could seek back to the beginning of the buffer. Would that work for your case?

Thanks,
Ran~

[jeanbmar]

Hi,

This was for the purpose of repro, I don't create the Readable from a buffer in real code.
I read the entire Readable into a buffer though in order to bypass the issue (https://nodejs.org/api/webstreams.html#streamconsumersbufferstream).

But I'm afraid this issue is a big flaw for any command that uses a Readable body. There's a chance the process will hang and it's very hard for developers to pinpoint the origin of the problem.

[RanVaknin]

Hi @jeanbmar ,

Yeah thats what I thought.

Unfortunately I was not able to artificially reproduce this.

I've tried your approach of iterating over that list over and over but it did not cause the service to throw a slow down 503 error.
I tried concurrently sending thousands of requests and it didnt slow down:

import { S3Client, PutObjectCommand } from "@aws-sdk/client-s3";
import { Readable } from "stream";

const s3Client = new S3Client();

async function uploadStream(id){

    const buffer = Buffer.alloc(10000, 'a');
    const stream = Readable.from(buffer);

    const input = {
        Bucket: "foo-bucket",
        Key: `my key${id}`,
        Body: stream,
        ContentLength: buffer.length,
    };
    
    try {
        const response = await s3Client.send(new PutObjectCommand(input));
        console.log(response.$metadata.httpStatusCode)
    } catch (error) {
        console.log(error)
    }
}

const promises = []

for (let i = 0; i < 20000; i++) {
    promises.push(new Promise((resolve, reject) =>{
        return uploadStream(i)
    }))
}
Promise.all(promises).then(() => {
    console.log(`Batch ${i + 1} of ${batches} completed.`);
});

I also tried unplugging my wifi to simulate a connection error, but that didnt work either. The request was not retried.

This will need to get a deeper look. Adding to our backlog.
Thanks again,
Ran

[kuhe]

@RanVaknin sample repro code in smithy-lang/smithy-typescript#1092

@jeanbmar a fix is queued for release sometime next week. We are not going to attempt to buffer the request at this time due to complexity and unpredictable behavior. Any failures submitting a stream will be thrown instead of retried.

[Mathie01]

Same problem, and the fix does not fix the bug
My S3Client is blocked in all send() call

[kuhe]

@Mathie01 provide a reproduction code sample.

[github-actions]

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

[github-actions]

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.