aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.09k stars 576 forks source link

The S3 Client swallows application errors that are outside its context. #3286

Open sellomkantjwa opened 2 years ago

sellomkantjwa commented 2 years ago

Describe the bug

The S3 Client swallows application errors that are not thrown by itself and have nothing to do with the client itself.

Your environment

SDK version number

@aws-sdk/client-s3@3.49.0

Is the issue in the browser/Node.js/ReactNative?

Node.js

Details of the browser/Node.js/ReactNative version

v14.18.1

Steps to reproduce

https://github.com/sellomkantjwa/aws-swallows-errors Note that the last line I attempt to throw an error, but this error is swallowed and never actually comes up.

Observed behavior

If you use the s3 client with callbacks instead of promises, and you do something like this:

client.send(command, (err, data) => {
    console.log('after send');
    throw new Error('I am an error');
});

Any errors thrown in the callback will just get mysteriously swallowed. Specifically, in the example above I am an error is never thrown

Expected behaviour

For errors to not get swallowed

Screenshots

After executing https://github.com/sellomkantjwa/aws-swallows-errors image. I expected to see my error thrown.

Additional context

Looks like the problem is caused by https://github.com/aws/aws-sdk-js-v3/blob/e98c52a68c3863003001bc8a751a6f8625de7771/packages/smithy-client/src/client.ts#L55. removing this line reveals the error as an unhandled rejection.

RanVaknin commented 2 years ago

@sellomkantjwa Thanks for reaching out with this issue.

I was able to confirm that it is in fact "swallowing" exceptions in V3.

V2 example code:

import AWS from 'aws-sdk';
AWS.config.update({region: 'us-east-1'});
const s3 = new AWS.S3();

const params = { Bucket: "testbucket-3650" };
s3.headBucket(params, function(err, data) {
    if(err) {
        console.log(err, err.stack);
    }
    else {
        console.log(data);
        throw new Error("im an error")
    }        
});

V2 logs:

Error: im an error
    at Response.<anonymous> (file://BASE_PATH/3286/sample.js:22:15)
    at Request.<anonymous> (BASE_PATH/3286/node_modules/aws-sdk/lib/request.js:367:18)
    at Request.callListeners (BASE_PATH/3286/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (BASE_PATH/3286/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (BASE_PATH/3286/node_modules/aws-sdk/lib/request.js:686:14)
    at Request.transition (BASE_PATH/3286/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (BASE_PATH/3286/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at BASE_PATH/3286/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (BASE_PATH/3286/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (BASE_PATH/3286/node_modules/aws-sdk/lib/request.js:688:12) {
  code: 'Error',
  time: 2022-06-22T02:20:40.492Z
}

V3 Example Code:

import { S3Client, HeadBucketCommand } from '@aws-sdk/client-s3'

const client = new S3Client({region: 'us-east-1'})
const command = new HeadBucketCommand({Bucket: "testbucket-3650"})

client.send(command, (err, data) => {
    console.log('after send');
    throw new Error('I am an error');
});

V3 Logs: (no exception)

after send

Will add it to the backlog. Thank you for bringing this to our attention!

yenfryherrerafeliz commented 1 year ago

Hi @sellomkantjwa, I also recommend you to take advantages of the use async / await features, since the SDK returns promises in almost all operations, and by doing that this behavior will not happen.

Thanks!