Closed Bug 1793754 Opened 2 years ago Closed 2 years ago

We're having trouble syncing error is displayed after the account is disconnected from the FxA web page

Categories

(Firefox :: Firefox View, defect, P1)

Desktop
Unspecified
defect

Tracking

()

VERIFIED FIXED
107 Branch
Tracking Status
firefox-esr102 --- unaffected
firefox105 --- disabled
firefox106 --- wontfix
firefox107 --- verified

People

(Reporter: atrif, Assigned: sclements)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fidefe-2022-mr1-firefox-view])

Attachments

(4 files)

Attached image try_again_00.gif (deleted) —

Found in

  • 106.0b8 (20221004185850)

Affected versions

  • 106.0b8 (20221004185850)
  • 107.0a1 (20220929014928)

Tested platforms

  • Affected platforms: Windows 10x64, Ubuntu 20.04, macOS 11.6
  • Unaffected platform: none

Steps to reproduce

  1. Sign in to FxA account inside Firefox.
  2. Open the Firefox App Menu and select the connected FxA Account > Manage Account.
  3. Sign out the device that was connected.
  4. Return to Firefox View and observe the Tab Pickup area.

Expected result

  • Not sure about the correct expected result here but I think that the Switch seamlessly between devices message should be displayed asking the user to connect to Sync.

Actual result

  • Switch seamlessly between devices message is displayed for a split second and then the We’re having trouble syncing error is displayed. Clicking the Try again button does nothing.

Regression range

  • This started with bug 1768695 when the sync error states were introduced. Also, before bug 1784969 there can be seen a page refresh.

Additional notes

  • Attached a screen recording.
  • Restarting the browser after step4 shows the Switch seamlessly between devices message.
Has STR: --- → yes

I was able to reproduce this, but I had 7 or so devices connected (some of which were old), and it wasn't until most of them were removed that I also saw this error. And what is weird is that when I clicked sign out sometimes I saw the "Disconnect from sync" modal pop up, same as in Alexandru's gif, and other times it immediately removed that device from the "Connected Services" section.

But when I try to sync a new device and try those steps all over again, I see the appropriate "Switch seamlessly between devices" screen once I've removed that connected device. So this doesn't seem to reproduce exactly all the time (or maybe only when there are multiple devices connected?), and perhaps has to do with timing of the weave:engine:sync:error.

Its a good question about what should be shown here, and I'm not entirely sure what's going on. If all devices have been signed out, then it should show the 'switch seamlessly between devices' screen. It didn't seem like sync was being disconnected per say, despite the misleading modal, but rather just devices removed. Unfortunately I accidentally killed the tab with the sync error the first time I attempted the STR.

Alexandru, could you try reproducing again and paste the sync log if you get the same error? You'll need to install this addon and you should see the sync logs in your terminal when you run the build locally.

Flags: needinfo?(alexandru.trif)
Attached file aboutsync.zip (deleted) —

Hello Sarah!
I tried to get some logs using only this addon but after the issue is reproduced there are no log files inside about:sync or about:sync-log pages. I attached the ones from the Browser Console. The console log is named console-export_only_about_sync_addon. If there is a step that I miss here please let me know and I can try again to attach the logs.

Also, I reproduced the issue with the above addon enabled and with browser.tabs.firefox-view.logLevel:All. This log file is named console_export with browser.tabs.firefox-view.logLevel. The tryToClearError: unable to sync, isReady: true, fxaSignedIn: false error is displayed when pressing the Try Again button after the issue was reproduced.

I can almost consistently reproduce the issue while having only two accounts connected and disconnecting one. If more information is needed please let me know.

Flags: needinfo?(alexandru.trif)
Whiteboard: [fidefe-2022-mr1-firefox-view]

Does this issue still occur when there are more than 2 devices synced? I just tried to reproduce by signing out of my mobile device and didn't get this same result on MacOS 12.6. Instead, my tabs from the other devices are still shown in the "Tab pickup" section, but now I also see the "Get Firefox for mobile" prompt.

In terms of the message we should see if sync is disconnected, it should be the "Turn on syncing to continue" error message.

Priority: -- → P2
Attached image sync_00.gif (deleted) —

I can see this issue with multiple devices as well but it seems that is more intermitent. In this video, I had a Phone,a MacOS,a Windows 7, and Win10 machine connected to sync and I could reproduce the issue but not on the first try.
The issue was reproduced after signing in all devices to Sync including the one that will be disconnected and then restarting Firefox. After restart, I followed the comment 0 steps and I could reproduce it. I could also reproduce the issue by disconnecting the account inside FxA web page from the same machine or from another machine. If more information is needed please let me know. Thank you!

Set release status flags based on info from the regressing bug 1768695

Assignee: nobody → sclements
Status: NEW → ASSIGNED
Keywords: regression
No longer regressed by: 1768695

Hey Sammy, would you be able to help investigate what's happening on the sync side? Firefox View observes several sync notifications and its hitting the observer for "weave:service:sync:error" here, hence that message. Is there a way to avoid showing a message for this particular situation?

Edit: I'm also wondering why its not hitting the case where we check if sync is enabled here. When someone disconnects sync from the about:preferences tab, that's what's triggered but its not in this situation.

Flags: needinfo?(skhamis)
Flags: needinfo?(skhamis)
Flags: needinfo?(skhamis)

Hey Sarah,

So a couple of things here from what I can see:

  1. Yes it looks like because Firefox View is observing: "weave:service:sync:error" the error state pops up. From the logs the process:
1665178020350	Sync.ErrorHandler	INFO	Attempting to schedule another sync.
1665178020350	Sync.SyncScheduler	TRACE	Requested sync should happen right away. (why=reschedule)
1665178020350	Sync.Service	ERROR	Aborting sync: failed to get collections.
1665178020350	Sync.Service	TRACE	Event: weave:service:sync:error
1665178020351	Sync.ErrorHandler	TRACE	Handling weave:service:sync:error
1665178020351	Sync.ErrorHandler	ERROR	Sync encountered an error

Because we're syncing when it gets signed out. This feels intentional from sync perspective because we're no longer signed in even though a sync was requested. Now as for the Firefox View portion it looks like:

1664970340141	FirefoxView.TabsSetup	DEBUG	Handling weave:service:sync:error
1664970340141	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : error-state
1664970340141	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: true
1664970340141	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, in error state:: sync-error
1664970340143	FirefoxView.TabsSetup	DEBUG	Handling UIState update
1664970340143	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : error-state
1664970340143	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: true
1664970340143	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, in error state:: sync-error
1664970340147	FirefoxView.TabsSetup	DEBUG	Handling UIState update
1664970340147	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, conditions not met to exit state: : error-state
1664970340147	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, will notify update?:: true
1664970340147	FirefoxView.TabsSetup	DEBUG	maybeUpdateUI, in error state:: sync-error

it seems like it does try to update the UI but hits the error-state of sync and when the user tries to press "try again" it hits here: https://searchfox.org/mozilla-central/source/browser/components/firefoxview/firefox-view-tabs-setup-manager.sys.mjs#548

which produces:

Skipping device list refresh; not signed in browser-sync.js:621:15
1664970343492	FirefoxView.TabsSetup	DEBUG	tryToClearError: unable to sync, isReady: true, fxaSignedIn: false
1664970344118	FirefoxView.TabsSetup	DEBUG	tryToClearError: unable to sync, isReady: true, fxaSignedIn: false

It seems the two options are:

  1. Check in the SYNC_SERVICE_ERROR case if we're signed in to sync, and if not then kick to the part of the flow that corresponds to what we need the user to do

  2. In tryToClearError we know if the user is signed in or not fxaSignedIn, so when clicking "try again", we know we can kick back to the sign in part of the flow

Hopefully this was helpful! We can try see if there are things we can try to improve on in the sync side of things but this might be hard to catch since Firefox View watches for the weave:service:sync:error and we might solve more edge cases by checking in Firefox View itself.

Flags: needinfo?(skhamis)
  • Only show sync error if user is not signed in and give them a way out when try again button is clicked

Thanks Sammy, this is very helpful. So this is another transient sync error that occurs when just the right situations occurs.

It seems that a problem with firefox view observing the weave:service:sync:error is that there isn't a reliable way to recover from it besides having the user click "try again" (and have that resetting some things). When I initially implemented the error states logic, I think it was suggested that "weave:service:sync:finish" would help to recover from the sync error so as not to end up in the position we're at now. But that doesn't seem to get triggered in all circumstances (certainly not this one afaict).

It's worth considering getting rid of this weave:service:sync:error error handling in Firefox View entirely. We've had numerous bugs that land users into this error state, sometimes they can't get out of and that we keep adding more and more conditions to handle these edge cases. But if weave:service:sync:error is considered a transient error, how would we deal with actual sync breakage? Is there a different notification we should be observing?

Since this will be a longer discussion, and the 107 nightly deadline is looming, I've submitted a patch for the immediately actionable changes mentioned.

(In reply to Sarah Clements [:sclements] from comment #9)

It's worth considering getting rid of this weave:service:sync:error error handling in Firefox View entirely.

Yep, I've been advocating that since the start :)

But if weave:service:sync:error is considered a transient error, how would we deal with actual sync breakage? Is there a different notification we should be observing?

I don't think we should bother. Firefox itself used to, but removed that support because it was flakey and caused user anxiety by telling them about problems they have literally no control over. Service downtime has been so tiny that I think you will never get this working correctly, mainly because it will be impossible to test (because the service is so rarely down).

Local environment issues (eg, the network being down) is a little gray, but I'd be inclined to even ignore that because (a) Firefox itself does in most cases and (b) the user, sitting in front of a browser, is going to become aware their network is down very soon if they don't already - and while that's easier to test and rationalize about, it's still imperfect (eg, Sync often sees the network link adaptor report the wrong state for periods of time, we had to remote support for captive portal detection due to too many false-positives, etc)

Pushed by sclements@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cbbf6a7e34a3 Fix sync error when user disconnects a device r=sfoster
Severity: S3 → S2
Priority: P2 → P1
Severity: S2 → S3
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

The patch landed in nightly and beta is affected.
:sclements, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox106 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(sclements)

Verified the issue today with Firefox 107.0a1 (20221013224755) while connected to sync on macOS 10.15, Ubuntu 20.04 and Windows 10x64. After disconnecting a device the Switch seamlessly between devices message is displayed giving the user a message to connect back to Sync. I could not see the We’re having trouble syncing error by following the steps from comment 0 multiple times with two or more devices connected. Closing this as verified. Thank you!

Status: RESOLVED → VERIFIED
Flags: needinfo?(sclements)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: