1265914 - PushServiceWebSocket:getNetworkInformation: Error recovering mobile network information. Error: UDP disabled

Status: RESOLVED FIXED

(Core :: DOM: Push Subscriptions, defect)

Description

[Chris Peterson [:cpeterson]]

I see multiple instances of the following PushServiceWebSocket error stack trace in the browser console (Nightly 48):

PushServiceWebSocket:getNetworkInformation: Error recovering mobile network information. Error: UDP disabled

Stack trace:
PushNetworkInfo.getNetworkInformation@resource://gre/modules/PushServiceWebSocket.jsm:1393:15
PushNetworkInfo.getNetworkState@resource://gre/modules/PushServiceWebSocket.jsm:1449:23
this.PushServiceWebSocket._wsOnStart@resource://gre/modules/PushServiceWebSocket.jsm:1187:5
PushWebSocketListener.prototype.onStart@resource://gre/modules/PushServiceWebSocket.jsm:98:5

Comment 1

[Lina Butler [:lina]]

Ugh, that's annoying for sure, especially since the error is expected behavior.

I think we can remove UDP wake-up entirely. It was used in the older B2G-only Simple Push implementation for a third-party server, but Web Push isn't supported on B2G.

Ben, JR: I know we use the UDP close code as a way to tell misbehaving clients to go away. Once the adaptive pings are removed (bug 1265915), do you see a reason to keep the close code around, or can we remove it from the client, too?

Comment 2

[Lina Butler [:lina]]

Attachment: MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana

Review commit: https://reviewboard.mozilla.org/r/47543/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47543/

Comment 3

[Lina Butler [:lina]]

I kept the special close code because it's the only way we can tell a client, "go away and don't come back for a while." Experience with Sync and Simple Push suggests this is a useful failsafe. But I removed the listener and everything else that mentions "UDP."

Comment 4

[Dragana Damjanovic [:dragana]]

Comment on attachment 8743076 [details]
MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana

https://reviewboard.mozilla.org/r/47543/#review45459

looks good

Comment 5

[Lina Butler [:lina]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=02252e5dd398

Comment 6

[Dragana Damjanovic [:dragana]]

Comment on attachment 8743076 [details]
MozReview Request: Bug 1265914 - Remove Push UDP wake-up. r?dragana

https://reviewboard.mozilla.org/r/47543/#review45561

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ae1e9a43c07

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ae1e9a43c07

Comment 9

[Kazumasa Hasegawa (Kazz)]

Will this fix be uplifted to 48?

Comment 10

[Lina Butler [:lina]]

I think it's OK to let this and bug 1265915 ride the trains. The only side effect is benign logspam, though it's definitely annoying because the "error" is expected. (The removed code only worked on Firefox OS, but, even though it's reported as an error, Push runs fine without it). Setting the "dom.push.loglevel" pref to "off" should silence the logging.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

This is going to affect the release?
I have this warning from time to time in beta and it is confusing.

Comment 12

[Lina Butler [:lina]]

We'd want to uplift this and bug 1265915 to Aurora and Beta. Most of it just removes a lot of (unused) code, but it's a moderately-sized patch.

Sylvestre, would you approve uplifts for both patches if I requested it? If you'd rather let both ride the trains, we can instead flip "dom.push.loglevel" to "off" and uplift that instead.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

This is a lot of changes to silent a warning :)
Flipping the pref is probably better if you don't foresee potential side effects.