Closed Bug 1262618 Opened 4 years ago Closed 4 years ago

Fix an unchained promise and a couple of non-promise returns in the push service

Categories

(Core :: DOM: Push Notifications, defect)

Product:
Core
Component:
DOM: Push Notifications
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox48 --- fixed

People

(Reporter: markh, Assigned: markh)

Details

Attachments

(1 file)

0001-Bug-XXXXXXX-Fix-an-unchained-promise-and-a-couple-of.patch
2.53 KB, patch
wchen
: review+
Details | Diff | Splinter Review 
There seem to be a couple of edge-cases that aren't handled quite correctly in the push service:

* when handling the 'idle-daily' notification, we call a function that returns a promise, but ignore the result. We should .catch and log to ensure the error is seen and to prevent "unhandled promise rejection" warnings.

* _changeServerURL is expected to return a promise, but will not do so if an unknown event is passed.

* _stopService() is expected to return a promise, but will not do so if the service is not initialized.

These seem minor and probably unlikely to be hit, but it seems to make sense to fix them. William, what do you think?
Attachment #8738750 - Flags: review?(wchen)
Comment on attachment 8738750 [details] [diff] [review]
0001-Bug-XXXXXXX-Fix-an-unchained-promise-and-a-couple-of.patch

Review of attachment 8738750 [details] [diff] [review]:
-----------------------------------------------------------------

looks good.
Attachment #8738750 - Flags: review?(wchen) → review+
Assignee: nobody → markh
https://hg.mozilla.org/mozilla-central/rev/04e4fbe0584e
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.