Closed Bug 888058 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, firefox23 wontfix, firefox24 wontfix, firefox25 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

RESOLVED FIXED
1.1 QE4 (15jul)
blocking-b2g leo+
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: jsmith, Assigned: nsm)

References

Details

(Whiteboard: QARegressExclude, [LeoVB+])

Attachments

(1 file, 1 obsolete file)

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.
Depends on: simple-push-b2g
Attached patch Wrap sendMsg in try (obsolete) — Splinter Review
Jason, can you try this patch please?
Assignee: nobody → nsm.nikhil
(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/
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.
Removed debug
Attachment #768657 - Attachment is obsolete: true
Attachment #769800 - Flags: review?(doug.turner)
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?
(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.
Attachment #769800 - Flags: review?(doug.turner) → review+
https://hg.mozilla.org/mozilla-central/rev/c4f7516f02f9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-birch]
Asking for leo+ blocker because as jsmith mentioned, after this error, further push registrations do not work.
blocking-b2g: --- → leo?
Blocking since this sounds like a regression within the 1.1 dev timeframe, please uplift & verify on branch.
blocking-b2g: leo? → leo+
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.
Flags: needinfo?(ryanvm)
b2g18 gets merged to v1.1hd, so this will end up there eventually :)
Flags: needinfo?(ryanvm)
Whiteboard: QARegressExclude
Whiteboard: QARegressExclude → QARegressExclude, [LeoVB+]
You need to log in before you can comment on or make changes to this bug.