Closed Bug 675125 Opened 12 years ago Closed 12 years ago
.notify should pass observers the exception as subject
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.
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 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-
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 #549401 - Flags: review?(rnewman) → review+
Status: NEW → ASSIGNED
Whiteboard: [fixed in services][qa-]
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in services][qa-] → [qa-]
Target Milestone: --- → mozilla8
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.