Closed Bug 969318 Opened 6 years ago Closed 6 years ago

"A promise chain failed to handle a rejection" from the livemarks service (deprecate passing a callback)

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file)

The livemarks service is now accepting both callbacks and returning a promise, the problem is that consumers using the callback are not handling the promise, so when we reject it an error is printed to the error console stating "A promise chain failed to handle a rejection".

There are some possibilities here:
1. add at least one internal handler
Yoric suggested
promise.then(null, () => undefined); // Ensure that there is at least one consumer for this error

2. return the promise only if a callback is not provided

3. split the promise and the callback APIs

4. bug 896186 keep only the promise api and break all consumers
4. could even be done with a mozIAsyncLivemarks compatible shim (that only gets the callback)
Note that option 1. is actually

let promise = ...;
promise.then(null, () => undefined); // Ensure that there is at least one consumer for this error
return promise; // However, if anybody else does promise.then(), they will get the uncaught error warning
I think the best option for now is 2 (short term) followed by 4 (long term)
Mano, what do you think?
Flags: needinfo?(mano)
Duplicate of this bug: 976637
Duplicate of this bug: 978955
I'll go for option 2 and ask for review
Assignee: nobody → mak77
Flags: needinfo?(mano)
Summary: "A promise chain failed to handle a rejection" from the livemarks service → "A promise chain failed to handle a rejection" from the livemarks service (deprecate passing a callback)
Whiteboard: [triage]
Attached patch patch v1Splinter Review
It's mostly "automatic" refactoring to convert callbacks to .then, and I added the deprecate warnings.
From here converting the service to a module should be straight forward, we may then keep the service as a shim wrapping the module, for some releases.
Attachment #8386367 - Flags: review?(mano)
Blocks: fxdesktoptriage
No longer blocks: fxdesktopbacklog
Comment on attachment 8386367 [details] [diff] [review]
patch v1

Many many thanks for taking care of converting the consumers.
Attachment #8386367 - Flags: review?(mano) → review+
https://hg.mozilla.org/integration/fx-team/rev/60c1af78091a
OS: Windows 8.1 → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla30
Blocks: fxdesktopbacklog
No longer blocks: fxdesktoptriage
Whiteboard: [triage]
No longer blocks: fxdesktopbacklog
Duplicate of this bug: 999272
Depends on: 1003839
Depends on: 1166853
No longer blocks: 992656
You need to log in before you can comment on or make changes to this bug.