1100145 - Push Handling code needs logging

Status: RESOLVED DUPLICATE of bug 1028869

(Hello (Loop) :: Client, defect, P1)

Description

[Mark Banner (:standard8)]

Currently, the MozLoopPushHandler only has logging if something goes wrong. This makes it very difficult to determine what's happening in the case of e.g. push server failures (we have no idea if a notification has actually been received).

We need to add at least some debug logging so we can see what's happening.

Comment 1

[Mark Banner (:standard8)]

Attachment: Improve Loop's push handler code logging.

Working out if a push notification has actually been received, is a bit of a pain at the moment. Until we get something like bug 885508, we need to do our own logging.

Without the logging, you can only be sure you've got something if a call window pops up or the display changes, which isn't going to be the case with rooms, and if we get issues, then we really need to know what's happening.

This uses the existing loop.debug.loglevel (set to "Error" by default) from MozLoopService to log the websocket messages. Additionally, I've converted the existing console.* messages into logging, and I've adjusted the levels of a few of them to what I think is appropriate.

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8523813 [details] [diff] [review]
Improve Loop's push handler code logging.

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

::: browser/components/loop/MozLoopPushHandler.jsm
@@ +16,5 @@
>  XPCOMUtils.defineLazyModuleGetter(this, "console",
>                                    "resource://gre/modules/devtools/Console.jsm");
>  
> +// See LOG_LEVELS in Console.jsm. Common examples: "All", "Info", "Warn", & "Error".
> +const PREF_LOG_LEVEL = "loop.debug.loglevel";

Please use MozLoopService.log.warn and kin. (lazy getter and things)

Comment 3

[Maire Reavy [:mreavy]]

[Tracking Requested - why for this release]:
We believe this will help debugging if there is a problem connecting to the push server in the field.

Comment 4

[:shell escalante]

Hi Paul,  Is this a bug you're able to do this week so we can uplift into Beta Monday for testing?

Comment 5

[Paul Kerr [:pkerr]]

Yes.

Comment 6

[Paul Kerr [:pkerr]]

Added the additional logging in bug 1028869 in order to test and debug the new ping and ack handling code. Also added a debug only pref to allow the push server URL to be set to allow server/client debug.