1205610 - Log the string associated with OT's UNABLE_TO_PUBLISH exception

Status: RESOLVED FIXED

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

Description

[Mark Banner (:standard8)]

As part of investigating bug 1199231, we're seeing a number of UNABLE_TO_PUBLISH exceptions that we don't understand.

We want to add logging of the error message to the 1500 event, so that we can hopefully better understand where exactly the error is coming from.

In otSdkDriver#_onOTException we need to extend the _notifyMetricsEvent call to add the event.message string to the passed string, but only when the error code is UNABLE_TO_PUBLISH (1500).

Comment 1

[Mark Banner (:standard8)]

Attachment: Log the string associated with OT's UNABLE_TO_PUBLISH exception to help understand where it is coming from.

This adds logging of the string, we expect this to potentially be temporary depending on what we find (though it might be a few releases).

Comment 2

[Ed Lee :Mardak]

Comment on attachment 8664470 [details] [diff] [review]
Log the string associated with OT's UNABLE_TO_PUBLISH exception to help understand where it is coming from.

>+++ b/browser/components/loop/content/shared/js/otSdkDriver.js
>+      } else if (event.code === OT.ExceptionCodes.UNABLE_TO_PUBLISH) {
I suppose the runtime optimizer could reuse the previous check for UNABLE_TO_PUBLISH although this code probably won't be hot anyway. ;)

>+        // We need to log the message so that we can understand where the exception
>+        // is coming from. Potentially a temporary addition.
>+        this._notifyMetricsEvent("sdk.exception." + event.code + "." + event.message);
Are we analyzing events with spaces correctly? It seems that many messages are full strings, e.g.,

https://dxr.mozilla.org/mozilla-central/source/browser/components/loop/content/shared/libs/sdk.js#23486
This token does not allow publishing. The role must be at least `publisher` to enable this functionality

Although if this is just for logging for visual inspection/reports, then it's probably not an issue.

Comment 3

[Mark Banner (:standard8)]

(In reply to Ed Lee :Mardak from comment #2)
> Comment on attachment 8664470 [details] [diff] [review]
> Log the string associated with OT's UNABLE_TO_PUBLISH exception to help
> understand where it is coming from.
> 
> >+++ b/browser/components/loop/content/shared/js/otSdkDriver.js
> >+      } else if (event.code === OT.ExceptionCodes.UNABLE_TO_PUBLISH) {
> I suppose the runtime optimizer could reuse the previous check for
> UNABLE_TO_PUBLISH although this code probably won't be hot anyway. ;)

I should have commented - I decided to not incorporate it into the previous check as it is likely to be temporary.

> >+        // We need to log the message so that we can understand where the exception
> >+        // is coming from. Potentially a temporary addition.
> >+        this._notifyMetricsEvent("sdk.exception." + event.code + "." + event.message);
> Are we analyzing events with spaces correctly? It seems that many messages
> are full strings, e.g.,
> 
> https://dxr.mozilla.org/mozilla-central/source/browser/components/loop/
> content/shared/libs/sdk.js#23486
> This token does not allow publishing. The role must be at least `publisher`
> to enable this functionality
> 
> Although if this is just for logging for visual inspection/reports, then
> it's probably not an issue.

Yeah, we're aware of that. This is intended to be temporary, and the code that is processing this we're controlling, so we can easily take account of the spaces.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/cf45b8a01ff0

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cf45b8a01ff0