Skip to content

fix: wallet-service network error #444

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

Merged
merged 10 commits into from
Mar 22, 2024

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Mar 16, 2024

Closes: https://github.com/HathorNetwork/internal-issues/issues/257

Acceptance Criteria

  • Should fix a network error when processing handleTx
  • Add a saga/helper method progressiveRetryRequest which is an abstraction to apply a progressive retry strategy to async function calls; useful when making requests to wallet-service API
  • This fix is the second part of the related issue; the first part is already fixed in fix: missing transaction after reload #442.

The following error happens on handlerTx when calling wallet.checkAddressMine:

 ERROR  [Error: Network Error]
 ERROR  The above error occurred in task handleTx
    created by takeEvery(WALLET_NEW_TX, handleTx)
    created by saga
    created by defaultSaga
Tasks cancelled due to error:
takeEvery(WALLET_NEW_TX, handleTx)
takeLatest(START_WALLET_REQUESTED, handleAction)
takeLatest(WALLET_CONN_STATE_UPDATE, onWalletConnStateUpdate)
takeLatest(WALLET_RELOADING, onWalletReloadData)
takeLatest(START_WALLET_FAILED, onStartWalletFailed)
takeLatest(RESET_WALLET, onResetWallet)
takeEvery(WALLET_UPDATE_TX, handleTx)
takeEvery(WALLET_BEST_BLOCK_UPDATE, bestBlockUpdate)
takeEvery(WALLET_PARTIAL_UPDATE, loadPartialUpdate)
takeEvery(WALLET_REFRESH_SHARED_ADDRESS, refreshSharedAddress)

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.

@alexruzenhack alexruzenhack self-assigned this Mar 16, 2024
@alexruzenhack alexruzenhack changed the title fix: wallet-service netowrk error fix: wallet-service network error Mar 16, 2024
Copy link
Member

@pedroferreira1 pedroferreira1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue you mention in the PR description has two bugs, one in the wallet service and the other in the full node facade. Which of the bugs does this PR fixes?

One other thing, after reading the issue again, I think the wallet service bug is not clear to me, I'm missing some details of it in the issue. Any received tx causes this bug? Or just if the app is in background? In which conditions does the bug happen?

I have some more questions regarding the implementation itself but it depends on the answers above.

@alexruzenhack
Copy link
Contributor Author

alexruzenhack commented Mar 19, 2024

The issue you mention in the PR description has two bugs, one in the wallet service and the other in the full node facade. Which of the bugs does this PR fixes?

One other thing, after reading the issue again, I think the wallet service bug is not clear to me, I'm missing some details of it in the issue. Any received tx causes this bug? Or just if the app is in background? In which conditions does the bug happen?

I have some more questions regarding the implementation itself but it depends on the answers above.

I answer it here: https://github.com/HathorNetwork/internal-issues/issues/257#issuecomment-2007626717

But in summary, this issue solves an unhandled network error, that can happen as a normal consequence of an HTTP request. It is not related to any transaction type or the background condition of the app.

@pedroferreira1 pedroferreira1 self-requested a review March 21, 2024 23:44
@alexruzenhack alexruzenhack merged commit 54aa3e8 into master Mar 22, 2024
@alexruzenhack alexruzenhack deleted the fix/wallet-service-network-error branch March 22, 2024 14:52
@alexruzenhack alexruzenhack mentioned this pull request Mar 22, 2024
6 tasks
alexruzenhack added a commit that referenced this pull request Mar 22, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
@alexruzenhack alexruzenhack mentioned this pull request May 9, 2024
alexruzenhack added a commit that referenced this pull request May 14, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
alexruzenhack added a commit that referenced this pull request May 15, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
alexruzenhack added a commit that referenced this pull request May 15, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
@alexruzenhack alexruzenhack mentioned this pull request May 15, 2024
alexruzenhack added a commit that referenced this pull request May 15, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
alexruzenhack added a commit that referenced this pull request May 15, 2024
* feat: add an abstraction method to call checkAddressMine with retry strategy

* review: apply suggestions

* lint: resolve rules

* refactor: extract progressiveRetryRequest to helper

* lint: comply with rules

* review: apply suggestions

* chore: add docstring to progressive retry mechanism constants

* refactor: progressiveRetryRequest

* lint: comply with rule

* chore: remove custom maxRetries
@alexruzenhack alexruzenhack mentioned this pull request Aug 12, 2024
1 task
@tuliomir tuliomir mentioned this pull request Oct 31, 2024
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.

3 participants