Aws-sdk-js: createPresignedPost triggers the callback twice and crashes when credentials are not provided

[slifty]

Confirm by changing [ ] to [x] below to ensure that it's a bug:

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

Describe the bug
When calling createPresignedPost with a callback and invalid credentials, my callback is called and node also crashes.

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

If on Node.js, are you running this on AWS Lambda?
No

Details of the browser/Node.js version
v12.18.3

SDK version number
v2.763.0

To Reproduce (observed behavior)

In an environment with no configured credentials:

import { S3 } from 'aws-sdk';
try {
  const s3 = new S3();
  s3.createPresignedPost({}, (err, data) => {
    console.log('sup dog');
  });
} catch (err: Error) {
  console.log('KABOOM!');
}

Running this causes the following output:

sup dog
./node_modules/aws-sdk/lib/services/s3.js:1241
      throw new Error('Unable to create a POST object policy without a bucket,'
      ^

Error: Unable to create a POST object policy without a bucket, region, and credentials
    at features.constructor.preparePostFields (./node_modules/aws-sdk/lib/services/s3.js:1241:13)
    at finalizePost (./node_modules/aws-sdk/lib/services/s3.js:1204:22)
    at ./node_modules/aws-sdk/lib/services/s3.js:1221:24
    at finish (./node_modules/aws-sdk/lib/config.js:386:7)
    at ./node_modules/aws-sdk/lib/config.js:428:9
    at Object.<anonymous> (./node_modules/aws-sdk/lib/credentials/credential_provider_chain.js:111:13)
    at Object.arrayEach (./node_modules/aws-sdk/lib/util.js:516:32)
    at resolveNext (./node_modules/aws-sdk/lib/credentials/credential_provider_chain.js:110:20)
    at ./node_modules/aws-sdk/lib/credentials/credential_provider_chain.js:126:13
    at ./node_modules/aws-sdk/lib/credentials.js:124:23
error Command failed with exit code 1.

I looked into the code and what's happening is that my callback is being called twice. Specifically, it is being called once to pass the credential error, and then it is being called a second time.

The second call does not actually complete, because finalizePost calls preparePostFields which then throws an error due to invalid configuration.

Expected behavior
I would expect the error to be passed to the callback directly (using err) without terminating the process.

Screenshots
N/A

Additional context
The partial solution is to update L1080 to be return callback(err); instead of just callback(err);

The fully defensive solution would be to ALSO wrap L1084 in a try catch block:

try {
        callback(null, finalizePost());
} catch (err) {
        callback(err);
}

[slifty]

A note that I can submit a PR that fixes this but I'm having trouble writing a valid test.

The error itself is being triggered outside of the call stack / callback flow, which means I'm not able to actually detect it within mocha (in the same way I'm not able to detect it in my sample application). Unfortunately the unhandled error happens after a valid callback, which means I call done and the test runner moves along and apparently ignores the unhandled error.

I would love any guidance on how to nail down this one.

Side note

I tried catching unhandled errors, while doing some R&D for a potential test using something like:

      process.on('uncaughtException', function (err) {
        console.log(err)
      });

This uncovered another unhandled error that is triggered by this test. It looks like that test actually exposes a potential bug that it isn't testing: the handler seems to be called twice but the first call passes (and invokes done) and the second call creates an assertion failure which occurs after done() was called, and therefore is going unnoticed.

[slifty]

Following up here -- if I submit a patch without a failing test would that be helpful?

[ajredniwja]

@slifty I can have someone take a look at the PR if you would like to work on it.

[github-actions[bot]]

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.

[jasonaowen]

This is still an issue.

I work with @slifty, and while I don't mean to speak for him, it seems unlikely to me that we'll have bandwidth to work on fixing this in the near future. We ended up working around the bug, but it would be better if we didn't have to.

[slifty]

Just a note -- I'll have a PR for this very shortly.