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

[ck] Proper promise treatment to avoid UnhandledPromises #1219

Merged
merged 2 commits into from
Oct 4, 2019

Conversation

mcortesi
Copy link
Contributor

@mcortesi mcortesi commented Oct 4, 2019

Description

When a sendTransaction is reject, we need to make sure every rejects promise is handled.

Usually, a user would do:

const res = await kit.sendTransaction(...)

await res.getHash()
await res.waitReceipt()

Others, may directly call res.waitReceipt() and don't call res.getHash().

The problem is when sendTransaction() fails, and thus both promises (hash & receipt) are rejected. The promises are created when the txResult is created, not when the methods are called. So we need to make sure we catch those errors.

This PR address that, by adding catch logic in both methods.

Tested

Manually, when factoring e2e transfer tests

@codecov
Copy link

codecov bot commented Oct 4, 2019

Codecov Report

Merging #1219 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1219   +/-   ##
======================================
  Coverage    66.1%   66.1%           
======================================
  Files         261     261           
  Lines        7618    7618           
  Branches      508     508           
======================================
  Hits         5036    5036           
  Misses       2485    2485           
  Partials       97      97
Flag Coverage Δ
#mobile 66.1% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b70e840...0f71b65. Read the comment docs.

@mcortesi mcortesi added the automerge Have PR merge automatically when checks pass label Oct 4, 2019
@ashishb ashishb merged commit fc3bfad into master Oct 4, 2019
@ashishb ashishb deleted the mc/ck/handle-rejected-promises branch October 4, 2019 20:52
aaronmgdr added a commit that referenced this pull request Oct 8, 2019
* master: (35 commits)
  [Wallet] Fix top of emojis cut off in the activity feed (#1243)
  Adding a contract to store minimum required client version (#1081)
  Revert "Feature #909 proxy delegatecall (#1152)" (#1241)
  Use ContractKit to get addresses for Blockchain API (#1175)
  Feature #909 proxy delegatecall (#1152)
  Fix Faucet done message (#1217)
  Updated SETUP.md with new yarn process (#1224)
  Adding `increaseAllowance` and `decreaseAllowance` methods (#1196)
  extracting function signatures (#1061)
  Fix integration hardcode (#1208)
  Fixing flaky governance test (#1155)
  Restore CI branch (#1223)
  [wallet] e2e back to green (#1210)
  [Wallet] Implement new import wallet flow designs (#1209)
  [Wallet] Fix disable conditions for butons on Enter Invite screen (#1214)
  [protocol] Rename infrastructureFraction to proposerFraction (#1174)
  [ck] Proper promise treatment to avoid UnhandledPromises (#1219)
  [ck] Transform StableToken parameters from fixidity format (#1218)
  [wallet]Store encrypted local signing key (#1188)
  2019-10-03 alfajores deployment (#1200)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants