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

Unhandled 'error' event caused by ChecksumStream #6845

Open
3 of 4 tasks
agaevaslan opened this issue Jan 23, 2025 · 4 comments
Open
3 of 4 tasks

Unhandled 'error' event caused by ChecksumStream #6845

agaevaslan opened this issue Jan 23, 2025 · 4 comments
Assignees
Labels
bug This issue is a bug. closing-soon This issue will automatically close in 4 days unless further comments are made. p1 This is a high priority issue

Comments

@agaevaslan
Copy link

Checkboxes for prior research

Describe the bug

                const inputData = {
			Bucket: bucketName,
			Key: fileKey,
			Range: _range
		}

		try {
			const getObjectCommand = new GetObjectCommand(inputData)
                         const s3Client = new S3Client({
			    region,
			    credentials,
		         })
			const response = await s3Client.send(getObjectCommand)
                } catch (e) {
                         console.log(e)
                }

when getting a filestream from s3 with GetObjectCommand, if there's a checksum mismatch in @aws-sdk/middleware-flexible-checksums/node_modules/@smithy/util-stream/dist-cjs/checksum/ChecksumStream.js the error is thrown:

`Checksum mismatch: expected "${this.expectedChecksum}" but received "${received}"` +
                    ` in response header "${this.checksumSourceLocation}".`

this error causes from Unhandled 'error' event in ChecksumStream.

It's not catched in catch block around s3Client.send(getObjectCommand) (since it happens after receiving the result after the response stream (response.Body starts producing data) and causes an unhandled exception.
There seems to be no way to catch/handle it (except using global unhandled expection handlers).

Regression Issue

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

SDK version number

@aws-sdk/client-s3 v3.732.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.18.1

Reproduction Steps

                const inputData = {
			Bucket: this._config.bucketName,
			Key: fileKey,
			Range: _range
		}

		try {
			const getObjectCommand = new GetObjectCommand(inputData)
                         const s3Client = new S3Client({
			    region,
			    credentials,
		         })
			const response = await s3Client.send(getObjectCommand)
                } catch (e) {
                         console.log(e)
                }

in order to simulate checksum mismatch you can change this.expectedChecksum !== received to this.expectedChecksum === received or smth trueish in @aws-sdk/middleware-flexible-checksums/node_modules/@smithy/util-stream/dist-cjs/checksum/ChecksumStream.js.

Observed Behavior

Checksum mismatch error causes unhandled error event

Expected Behavior

Checksum mismatch error should not cause unhandled error event

Possible Solution

Expose ChecksumStream outside to allow setting error handlers

Additional Information/Context

No response

@agaevaslan agaevaslan added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 23, 2025
@zshzbh zshzbh self-assigned this Jan 23, 2025
@zshzbh
Copy link
Contributor

zshzbh commented Jan 23, 2025

Hey @agaevaslan ,

I can't reproduce this issue.

I'm using "@aws-sdk/client-s3": "3.732",
The code I have

 import {S3Client, GetObjectCommand} from "@aws-sdk/client-s3"
 import {NodeHttpHandler} from "@smithy/node-http-handler"
 const client = new S3Client({
	region : "us-east-1",
	requestHandler: new NodeHttpHandler({
	  connectionTimeout: 5000, // 5 seconds
	  requestTimeout: 5000
	})
  });
  const res = await client.send(new GetObjectCommand({
	Bucket: "new-bucket-maggie-ma",
	Key: "0.txt",
	Range: "bytes=1-"
  }))
  const bodyContents = await res.Body.transformToString();
  console.log('File contents:', bodyContents);
console.log("res", res.$metadata, res.$metadata.response);

The result I got -

➜  OSDS-4611 node index.js
File contents: ello 0!
res {
  httpStatusCode: 206,
  requestId: 'XXXXXXX',
  extendedRequestId: 'cEnsohC2YgBF1XQqgO+Yr68/XXXXX/OxzNF6PZUu7jHoc9BOg=',
  cfId: undefined,
  attempts: 1,
  totalRetryDelay: 0
} undefined

Could you please provide more details with minimal reprod?

@zshzbh zshzbh added response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jan 23, 2025
@agaevaslan
Copy link
Author

hey @zshzbh ,
the issue appears when there is a checksum mismatch.
I don't know how to reproduce a checksum mismatch.

But you can simulate the mismatch by always returning an error in @aws-sdk/middleware-flexible-checksums/node_modules/@smithy/util-stream/dist-cjs/checksum/ChecksumStream.js in

 if (this.expectedChecksum !== received) {
                return callback(new Error(`Checksum mismatch: expected "${this.expectedChecksum}" but received "${received}"` 
                    ` in response header "${this.checksumSourceLocation}".`));
            }

I'm pretty sure this is where the error is generated since it's the only place in the package with such error message.

@kuhe kuhe self-assigned this Jan 24, 2025
@kuhe
Copy link
Contributor

kuhe commented Jan 24, 2025

We are planning to release a fix for this issue today in a few hours.

@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Jan 24, 2025
@kuhe
Copy link
Contributor

kuhe commented Jan 24, 2025

@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. p1 This is a high priority issue and removed response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. pending-release This issue will be fixed by an approved PR that hasn't been released yet. p2 This is a standard priority issue labels Jan 24, 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. closing-soon This issue will automatically close in 4 days unless further comments are made. p1 This is a high priority issue
Projects
None yet
Development

No branches or pull requests

3 participants