createBufferedReadable fails to buffer properly if chunks are mixed Buffer/Uint8Array instances

[abustany]

Version: @smithy/util-stream@4.2.0

when buffering a stream, createBufferedReadable treats Uint8Array and Buffer chunks differently. Switching from one to the other inside a stream makes it flush its buffer - which can lead to an InvalidChunkSizeError when this function is used by S3Client. I'm not sure if mixing both Buffer and Uint8Array chunks in a same stream is "legal" or not, but it's an easy mistake to do if not, especially since on NodeJS Buffer is a base of Uint8Array (so Typescript won't complain if you mix them up).

Minimal reproducer:

import * as streams from "node:stream";

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

async function main() {
  const client = new S3Client({ requestStreamBufferSize: 32 * 1024 });

  async function* mixedData(): AsyncIterable<Uint8Array> {
    yield new Uint8Array(2000); // too small chunk, should get buffered with the following one
    yield Buffer.alloc(128_000);
  }

  // Triggers an InvalidChunkSizeError
  await client.send(
    new PutObjectCommand({
      Bucket: "test-bucket",
      Key: "test",
      ContentLength: 130_000,
      Body: streams.Readable.from(mixedData()),
    })
  );
}

main();

[kuhe]

The automatic buffering mechanism is provided as a convenience and already not ideal for performance.

We would prefer that the stream owner maintains responsibility for keeping the byteArray class consistent for the duration of the stream. Allowing it to both be buffered into chunks and then change between byteArray implementations, leaving the chunks and their type with possibly very little resemblance to the input data, is not a use-case we should support.

Please set your AsyncIterable to normalize the output byteArray type.

[abustany]

yup, to be honest I filed this issue mostly to document the behavior, in case it helps someone googling the same error in the future. I would totally be fine with the buffering logic requiring that chunks are of a uniform type.

One question though is whether the buffering logic should emit a clearer error in case of mixed payloads. The current situation is really confusing for end users, since on the one hand you set requestStreamBufferSize, but on the other you get a InvalidChunkSizeError 😅 It took me quite a bit of time in the debugger to get to the root cause of the issue.

[kuhe]

Yeah, a warning would help