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: promise leak by capturing rejection from getVersion #584

Merged
merged 2 commits into from
Oct 31, 2023

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Oct 26, 2023

Acceptance Criteria

  • Resolve promise leak in the wallet start method when using the VersionApi

Related to: HathorNetwork/hathor-wallet-mobile#364

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (29ca737) 64.86% compared to head (1a1ef76) 64.86%.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #584   +/-   ##
=======================================
  Coverage   64.86%   64.86%           
=======================================
  Files          67       67           
  Lines        5254     5254           
  Branches     1108     1108           
=======================================
  Hits         3408     3408           
  Misses       1766     1766           
  Partials       80       80           
Files Coverage Δ
src/new/wallet.js 40.80% <50.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexruzenhack alexruzenhack force-pushed the fix/promise-leak-version-api branch from 7fc4c70 to 447c8d0 Compare October 26, 2023 21:37
@tuliomir
Copy link
Contributor

It's worth taking a look at #579 too, as it also proposes an async wrapper and unit tests around this same method. Link to the relevant code section.

@alexruzenhack alexruzenhack changed the title fix: promise leak by capturing rejection from axios fix: promise leak by capturing rejection from getVersion Oct 26, 2023
@alexruzenhack alexruzenhack merged commit 217797f into dev Oct 31, 2023
2 of 6 checks passed
@alexruzenhack alexruzenhack deleted the fix/promise-leak-version-api branch October 31, 2023 17:47
@alexruzenhack alexruzenhack mentioned this pull request Nov 9, 2023
1 task
@alexruzenhack alexruzenhack mentioned this pull request Sep 13, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants