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

Fix createPresignedPost dual-callback error #3537

Merged
merged 1 commit into from
Dec 4, 2020

Conversation

slifty
Copy link
Contributor

@slifty slifty commented Nov 19, 2020

This fixes a bug where using the callback version of createPresignedPost without proper AWS credentials configured would result in an un-catchable error and crash the node process.

Note: as discussed in #3486, I was not able to figure out a way to write a test to capture this error due to the nature of the error. In the original code, the callback is called correctly at first, and only after that does the crash occur (when the callback is called a second time).

Resolves #3486

Checklist
  • npm run test passes
  • changelog is added, npm run add-change

@slifty slifty force-pushed the 3486-fix-createPresignedPost branch from 4b64673 to fd99318 Compare November 19, 2020 20:29
@aws-sdk-js-automation

This comment has been minimized.

@aws-sdk-js-automation

This comment has been minimized.

@ajredniwja
Copy link
Contributor

Can you please address the build failure @slifty ?

@slifty
Copy link
Contributor Author

slifty commented Nov 20, 2020

@ajredniwja I believe that failure is because I force pushed to correct a typo in my change log that I noticed right after opening the PR, which meant the branch didn't exist any more (the error in that failure is reference not found for primary source and source version 4b64673d0e391567ab0e3a37f7bc5a437a06378e)

The fd99318 commit passed -- which is indeed the actual commit in the current PR.

Is there a way for me to trigger a re-build, or is that good enough?

@ajredniwja
Copy link
Contributor

Hey @slifty we would need a successful build to review and merge this what you can do is rebase from master and push which would trigger rebuild.

git fetch upstream
git rebase upstream/master
git push --force-with-lease

This fixes a bug where using the callback version of createPresignedPost
without proper AWS credentials configured would result in an
un-catchable error and crash the node process.

It is not possible to write a test to capture this error due to the
nature of the error.  The callback is called, and only after that does
the crash occur.
@slifty slifty force-pushed the 3486-fix-createPresignedPost branch from fd99318 to e25d497 Compare November 23, 2020 17:43
@aws-sdk-js-automation
Copy link

AWS CodeBuild CI Report

  • CodeBuild project: sdk-v2-github
  • Commit ID: e25d497
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@slifty
Copy link
Contributor Author

slifty commented Dec 4, 2020

@ajredniwja Just following up on whether there are any other changes you would like?

@ajredniwja ajredniwja merged commit 71d6fa9 into aws:master Dec 4, 2020
@ajredniwja
Copy link
Contributor

Apologies for late reply, Thanks for your late reply.

@slifty slifty deleted the 3486-fix-createPresignedPost branch December 7, 2020 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createPresignedPost triggers the callback twice and crashes when credentials are not provided
3 participants