Closed
Bug 675125
Opened 13 years ago
Closed 13 years ago
Utils.notify should pass observers the exception as subject
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: philikon, Assigned: philikon)
Details
(Whiteboard: [qa-])
Attachments
(1 file, 1 obsolete file)
5.95 KB,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
Our ErrorHandler policy (bug 659067) and Syncorro will most definitely want to inspect the error thrown by various parts of Sync. We should pass them to the appropriate observers.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → philipp
Assignee | ||
Comment 1•13 years ago
|
||
This changes the API of Utils.notify a bit. Instead of using 'subject' as a pre-determined value, we now use 'data'. That way 'subject' can be used to either pass the return value or the exception of the function that's wrapped. Using 'data' to track, say, the engine name also matches other observers out there, e.g. weave:engine:applied.
Attachment #549311 -
Flags: review?(rnewman)
Comment 2•13 years ago
|
||
Comment on attachment 549311 [details] [diff] [review] v1 Review of attachment 549311 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, and is a sane change. However, it causes two test failures on my machine -- did you run xpcshell-tests before submitting for review? test_interval_triggers.js test_engine.js Let me know if you can't repro locally, and I'll provide the logs. While you're touching these tests, it occurs to me to change the obscure magic values -- "one", "baz" -- to something slightly more meaningful, like "bar_data" and "bad_data". This would make the tests a little more comprehensible at first glance.
Attachment #549311 -
Flags: review?(rnewman) → review-
Assignee | ||
Comment 3•13 years ago
|
||
Yeah, it was late. In fact, I went to bed thinking that I had better run all tests in the morning to be sure. I also had this indescribable feeling that we actually used the subject somewhere, and of course my gut was right. All good now.
Attachment #549311 -
Attachment is obsolete: true
Attachment #549401 -
Flags: review?(rnewman)
Updated•13 years ago
|
Attachment #549401 -
Flags: review?(rnewman) → review+
Assignee | ||
Comment 4•13 years ago
|
||
http://hg.mozilla.org/services/services-central/rev/ced4ba4f15e9
Status: NEW → ASSIGNED
Whiteboard: [fixed in services][qa-]
Assignee | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ced4ba4f15e9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][qa-] → [qa-]
Target Milestone: --- → mozilla8
Updated•6 years ago
|
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•