888058 - Simple Push - NS_ERROR_UNEXPECTED error thrown during execution of navigator.push.register, resulting in no onsuccess or onerror callback firing

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Jason Smith [:jsmith]]

Had this happen once when I called navigator.push.register under Mozilla Guest wifi on an unagi b2g18 6/27 build:

06-27 15:31:51.971: E/GeckoConsole(109): [JavaScript Error: "NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIWebSocketChannel.sendMsg]" {file: "resource://gre/modules/PushService.jsm" line: 884}]

As a result of this error firing, no onsuccess or onerror callback fired on the DOM Request from navigator.push.register.

Comment 1

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Wrap sendMsg in try

Jason, can you try this patch please?

Comment 2

[Jason Smith [:jsmith]]

(In reply to Nikhil Marathe from comment #1)
> Created attachment 768657 [details] [diff] [review]
> Wrap sendMsg in try
> 
> Jason, can you try this patch please?

Can you generate a try build with this patch?

https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/try-builds/

Comment 3

[Jason Smith [:jsmith]]

https://tbpl.mozilla.org/?tree=Try&rev=7631a4c747a7

Comment 4

[Jason Smith [:jsmith]]

Tried testing the attached patch with the try build. I'm definitely hitting the code path here, as I'm seeing "Sent ping" in many parts of my logcat. However, I have not managed to get the exception to get thrown again to cause "Sending the ping failed" to be seen in the logcat after testing registration under various networking conditions.

Comment 5

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: Wrap sendMsg in try

Removed debug

Comment 6

[Doug Turner (:dougt)]

Comment on attachment 769800 [details] [diff] [review]
Wrap sendMsg in try

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

::: dom/push/src/PushService.jsm
@@ +684,5 @@
> +      // being reset.
> +      try {
> +        this._ws.sendMsg('{}');
> +      } catch (e) {
> +      }

if this happens, don't you want to move to a different state?  Like  STATE_SHUT_DOWN?

Comment 7

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

(In reply to Doug Turner (:dougt) from comment #6)
> Comment on attachment 769800 [details] [diff] [review]
> Wrap sendMsg in try
> 
> Review of attachment 769800 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/push/src/PushService.jsm
> @@ +684,5 @@
> > +      // being reset.
> > +      try {
> > +        this._ws.sendMsg('{}');
> > +      } catch (e) {
> > +      }
> 
> if this happens, don't you want to move to a different state?  Like 
> STATE_SHUT_DOWN?

Since the ping send will fail, the pong timer will timeout after 10sec (no pong received), so the connection will be reset at that point.

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

http://hg.mozilla.org/projects/birch/rev/c4f7516f02f9

Comment 9

[Phil Ringnalda (:philor)]

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

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Asking for leo+ blocker because as jsmith mentioned, after this error, further push registrations do not work.

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

Blocking since this sounds like a regression within the 1.1 dev timeframe, please uplift & verify on branch.

Comment 12

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/20395e0e8294

I don't know what the hd branch is for, if it integrates 1.1 changes, it should land on hd too.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

b2g18 gets merged to v1.1hd, so this will end up there eventually :)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/20395e0e8294