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

Sync should handle library/client errors #2316

Closed
cezaraugusto opened this issue Nov 30, 2018 · 8 comments · Fixed by brave/brave-core#1076
Closed

Sync should handle library/client errors #2316

cezaraugusto opened this issue Nov 30, 2018 · 8 comments · Fixed by brave/brave-core#1076

Comments

@cezaraugusto
Copy link
Contributor

cezaraugusto commented Nov 30, 2018

Sync should handle library/client errors. We have this mechanism in the back-end but interface should be aware of it.

The error happens when sync client (browser) can't connect with the sync server for N reasons, so the message needs to be generic.

Worth mentioning that the offline error is already handled so this is different.

cc @rebron @bradleyrichter to cook a good phrase for this.

Test Plan:

  1. Create a sync chain and ensure you see two devices
  2. Quit Brave
  3. Launch Brave without internet
  4. You should see a Couldn't contact Sync servers error dialog
@rebron
Copy link
Collaborator

rebron commented Nov 30, 2018

#1149 related issue.

I think we can go with something like:

Title: Sync servers can't be reached
Body: The sync servers might temporarily be down. Please try again later.

cc: @tomlowenthal

@tildelowengrimm
Copy link
Contributor

tildelowengrimm commented Dec 4, 2018

[Always blame the user 😛]
Title: Couldn't contact Sync servers
Body: You might be having network trouble, or there could be a problem with the Sync servers.

@srirambv
Copy link
Contributor

Marking as QA/Yes as this requires text verification.

@rebron
Copy link
Collaborator

rebron commented Dec 12, 2018

@cezaraugusto can we get this in for 58.x too? Use toml text above/repeated here:
Title: Couldn't contact Sync servers
Body: You might be having network trouble, or there could be a problem with the Sync servers.

Can you also create a test plan for QA.

@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Dec 12, 2018
@cezaraugusto
Copy link
Contributor Author

sure I'll be addressing this in a quick follow-up once sync v2 lands

@LaurenWags
Copy link
Member

@cezaraugusto - in the test plan, should we see the error just by navigating to brave://sync or do we need to perform an action on the page (View Sync Code, Leave Sync Chain, Add Device, etc)?

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Dec 25, 2018

@cezaraugusto Verified the issue on 0.59.12. I am not seeing Couldn't contact Sync servers error dialog

2316

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jan 3, 2019

Verification passed on

Brave 0.59.14 Chromium: 72.0.3626.28 (Official Build) beta (64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Windows
  • Verified the STR from description

image

Verified passed with

Brave 0.59.14 Chromium: 72.0.3626.28 (Official Build) beta(64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Mac OS X
  • Verified test plan from description
    screen shot 2019-01-03 at 5 53 51 pm

Verification passed on

Brave 0.59.14 Chromium: 72.0.3626.28 (Official Build) beta(64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Linux

Used test plan from description
Logged #2814

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants