Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Ledger recovery should be aware of network status and give appropriate errors when the connection is down. #4720

Closed
alexwykoff opened this issue Oct 12, 2016 · 15 comments · Fixed by #5924 or #6838
Labels
bug feature/rewards needs-STR This bug needs steps to reproduce.

Comments

@alexwykoff
Copy link
Contributor

alexwykoff commented Oct 12, 2016

Did you search for similar issues before submitting this one?
Yes

Describe the issue you encountered:
While trying to recover a wallet, my network connection went down. The error message from the system indicated that my keys were somehow incorrect. Once the network connection was restored, the keys worked without issue.

Expected behavior:
The system should display appropriate error messages.

  • Platform (Win7, 8, 10? macOS? Linux distro?):
    Tested on OS X
  • Brave Version:
    0.12.5
  • Steps to reproduce:
    1. Generate recovery keys
    2. Remove your ~brave folder and create a new wallet
    3. Open the recovery dialog, enter keys but do not submit the form
    4. Disable your network connection (or just happen to have a really poor one)
    5. Submit the form
    6. Observe the error message
  • Screenshot if needed:
    screen shot 2016-10-13 at 1 01 40 am
  • Any related issues:

Test Plan

  1. Go to Preferences > Payments
  2. Click Advanced Settings
  3. Click Recover Your Wallet
  4. Try to recover with the wrong keys
  5. You should see the "Recovery Failed" overlay
  6. Repeat steps 1-3
  7. Turn off network connection
  8. Try to recover with right or wrong keys
  9. You should see the "Network Error" overlay
@mrose17 mrose17 modified the milestones: 1.0.0, 0.12.7dev, 0.12.8dev Oct 17, 2016
@mrose17 mrose17 modified the milestones: 1.1.0, 0.12.8dev Oct 25, 2016
mrose17 added a commit that referenced this issue Nov 30, 2016
1. Fixes #5788
2. Partially addresses
#4720
3. Needs help for #4114
mrose17 added a commit that referenced this issue Nov 30, 2016
1. Fixes #5788
2. Fixes #4114
3. Partially addresses #4720
@mrose17 mrose17 removed their assignment Nov 30, 2016
mrose17 referenced this issue Nov 30, 2016
- only perform check if payments are enabled
- check using Array.prototype.some (which returns on first truthy result)

Auditors: @mrose17
@bsclifton
Copy link
Member

Groundwork laid by @mrose17 w/ #5924

This is ready for the UI work. Marking as 0.13.1 and assigning over to @jkup

@bsclifton bsclifton modified the milestones: 0.13.1, 1.1.0 Nov 30, 2016
@jkup
Copy link
Contributor

jkup commented Nov 30, 2016

@bsclifton did you mean to close this?

@mrose17 mrose17 reopened this Nov 30, 2016
@mrose17
Copy link
Member

mrose17 commented Nov 30, 2016

it shouldn't have been closed. i did the first part, but now it's up to you!!!

@bsclifton
Copy link
Member

bsclifton commented Nov 30, 2016

Looks like the self-closing keywords from the commit unintentionally closed it 😄

...
For the person who fixes #4720 - when...

@mrose17
Copy link
Member

mrose17 commented Nov 30, 2016

rather amusing...

@mrose17
Copy link
Member

mrose17 commented Dec 15, 2016

@bsclifton - can we close this one too?

@luixxiul
Copy link
Contributor

@mrose17 I don't think so. #6047 focuses on the UI change.

@jkup
Copy link
Contributor

jkup commented Dec 16, 2016

@mrose17 reading through your commit I'm still not sure how I can differentiate between a network error and a failed recovery (bad keys). Can you help me understand how to do this UI work?

Thanks!!

@bsclifton
Copy link
Member

@mrose17 looks like @jkup needs some more info (he should be back next week)

I'm still not sure how I can differentiate between a network error and a failed recovery (bad keys). Can you help me understand how to do this UI work?

@mrose17
Copy link
Member

mrose17 commented Dec 21, 2016

here is the answer: look at ledgerInfo.error.err if it is a STRING, then there is a data input or server error; if it is an object, e.g., { errorCode: -109 } then there is a network error.

@luixxiul
Copy link
Contributor

luixxiul commented Feb 4, 2017

It seems this is not fixed. I followed the same steps after restarting the browser but it did not work either

@luixxiul luixxiul reopened this Feb 4, 2017
@alexwykoff
Copy link
Contributor Author

In my case, the breakage was a bit nuanced.
Without a network connection, the UI did nothing. No warning, no error, it just took my clicks and walked away.

When the network connection was restored, somewhere in the middle of my machine-gunning the control, the recovery keys were accepted.

cc @bsclifton

@bbondy
Copy link
Member

bbondy commented Feb 5, 2017

Moved to 0.13.3

@bbondy bbondy modified the milestones: 0.13.3, 0.13.2 Feb 5, 2017
@mrose17 mrose17 modified the milestones: 0.13.5, 0.13.4 Feb 14, 2017
@bsclifton bsclifton modified the milestones: 0.13.7, 0.13.6 Mar 12, 2017
@bsclifton
Copy link
Member

Removing milestone for now and putting on backlog- we're tracking in Asana. Will update once we have prioritized

@bsclifton bsclifton modified the milestones: Backlog, 0.14.2 Apr 3, 2017
@mrose17 mrose17 modified the milestones: 1.1.0, Backlog Apr 20, 2017
@ghost ghost removed this from the 1.1.0 milestone Sep 26, 2017
@ghost ghost added needs-STR This bug needs steps to reproduce. suggestion and removed suggestion labels Sep 26, 2017
@ghost
Copy link

ghost commented Sep 26, 2017

Closing at @mrose17 direction.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug feature/rewards needs-STR This bug needs steps to reproduce.
Projects
None yet
6 participants