Closed Bug 1439777 Opened 7 years ago Closed 7 years ago

Simplify sync's ErrorHandler

Categories

(Firefox :: Sync, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox60 --- fixed

People

(Reporter: tcsc, Assigned: eoger)

Details

Attachments

(2 files)

It's a somewhat complex piece of code (with very complex tests) that is primarially concerned with whether or not we should be displaying errors in the UI. Since we never do this anymore, it should be simplified.
Assignee: nobody → eoger
Status: NEW → ASSIGNED
Priority: -- → P1
Note that we will also need to update the about:sync addon which relies on weave:ui:* [0] [0] https://github.com/mhammond/aboutsync/blob/77ef1595df68ed71d7f55ae68018d650061ca3ed/bootstrap.js#L178-L179
We could instead consider emitting weave:ui:*:error events when we'd otherwise write a sync error log. I spent some time looking into how to convert about:sync to not depend on those events, and the best I could come up with (without changing the sync code) was to listen for when we wrote an log file, and see if doing so caused a new file with error in its name to appear in our sync log directory. A simpler solution would be to let about:sync know here https://searchfox.org/mozilla-central/source/services/sync/modules/policies.js#854 somehow (using weave:ui:* would be backwards compatible, admittedly the name is bad)
(FWIW, reporting an error in this case will catch cases where an error is logged outside of a sync -- at the moment we completely miss those)
Comment on attachment 8952983 [details] Bug 1439777 p2 - Remove weave:ui:* related code. https://reviewboard.mozilla.org/r/222246/#review228490 nice cleanup, thanks!
Attachment #8952983 - Flags: review?(markh) → review+
Attachment #8952982 - Flags: review?(markh) → review+
Comment on attachment 8952983 [details] Bug 1439777 p2 - Remove weave:ui:* related code. https://reviewboard.mozilla.org/r/222246/#review229398 It's great to see this gone.
Attachment #8952983 - Flags: review?(tchiovoloni) → review+
Pushed by eoger@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8856efbc78a7 p1 - Remove X-Weave-Alert support. r=markh https://hg.mozilla.org/integration/autoland/rev/6e83f07e8f2f p2 - Remove weave:ui:* related code. r=markh,tcsc
Backed out 2 changesets (bug 1439777) for browser chrome in browser/components/customizableui/test/browser_remote_tabs_button.js and xpc-shell failures in services/sync/tests/unit/test_errorhandler_2.js on a CLOSED TREE https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=166018817 https://hg.mozilla.org/integration/autoland/rev/f00eccec1c8378c6468215ccfe1cd3d8a252b640
Flags: needinfo?(eoger)
Backout by shindli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25507e523aec Backed out 2 changesets for browser chrome in browser/components/customizableui/test/browser_remote_tabs_button.js and xpc-shell failures in services/sync/tests/unit/test_errorhandler_2.js on a CLOSED TREE
Pushed by eoger@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1af3426dc956 p1 - Remove X-Weave-Alert support. r=markh https://hg.mozilla.org/integration/autoland/rev/d99402ad9ecc p2 - Remove weave:ui:* related code. r=markh,tcsc
Backout by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fea7978f3af6 Backed out 2 changesets for xpcshell failures on /test_errorhandler_2.js
Pushed by eoger@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/76c0ebfa6f08 p1 - Remove X-Weave-Alert support. r=markh https://hg.mozilla.org/integration/autoland/rev/157f93b585b1 p2 - Remove weave:ui:* related code. r=markh,tcsc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: