Report WebRTC transport termination (e.g. iceConnectionState=disconnected)

RESOLVED FIXED in Firefox 52

Status

()

defect
P1
normal
Rank:
17
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: ekr, Assigned: drno)

Tracking

({dev-doc-complete})

unspecified
mozilla52
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [webrtc][blocking-webrtc-][spec-issue][webrtc-uplift])

Attachments

(1 attachment)

Reporter

Description

6 years ago
We need some way to report that transport has failed.
Not sure without more info if this is blocking, but I suspect it isn't
Whiteboard: [webrtc][blocking-webrtc?]
Assignee: nobody → ekr
Priority: -- → P2
Whiteboard: [webrtc][blocking-webrtc?] → [webrtc][blocking-webrtc-][spec-issue]
Assignee: ekr → adam
Whiteboard: [webrtc][blocking-webrtc-][spec-issue] → [webrtc][blocking-webrtc-][spec-issue][webrtc-uplift]

Comment 2

6 years ago
Justin's opinion appears to be that this should be communicated with an iceConnectionState change to "failed"; see http://lists.w3.org/Archives/Public/public-webrtc/2013Apr/0059.html

It is highly likely that Chrome will implement this behavior. Unless we have a reason to push for some other mechanism, I would propose that we do the same.

Comment 3

6 years ago
I have been following webrtc discussion group on this particular issue for sometime. iceconnectionstate report seems to be reasonable but it is not complete if the reason for failure is reported with granularity and unambigously. I am not sure what is the spec's plan on this though ...

Comment 4

6 years ago
i meant, if the failure "isn't" reported :)

Updated

6 years ago
Duplicate of this bug: 859761

Updated

6 years ago
Duplicate of this bug: 881294

Updated

6 years ago
Duplicate of this bug: 878562
Blocks: Talkilla

Updated

6 years ago
Summary: Report WebRTC transport failures somehow → Report WebRTC transport failures (iceConnectionState=failed)
Summary: Report WebRTC transport failures (iceConnectionState=failed) → Report WebRTC transport termination (e.g. iceConnectionState=failed|disconnected|closed|)

Comment 8

5 years ago
Is there any recommended way now to detect whether a PeerConnection has failed? As far as I can tell the PeerConnection state still never changes to "failed". Is there some other workaround? Right now in Chrome we wait for the "loadedmetadata" event on the Video Element and timeout. On Firefox this Event fires too soon and so we have no way of detecting when a PeerConnection has failed.
Reporter

Comment 9

5 years ago
Are you looking for negotiation failure or ICE failure?

Comment 10

5 years ago
I think ICE failure but I'm not clear on the difference, it's ICE negotiation right? I basically just want to be able to detect any time a connection between peers has failed and be able to provide a reasonable error message to the user.
Reporter

Comment 11

5 years ago
Well, so there are cases where we can't negotiate media, e.g., where the answer
contains more m-lines than the offer and other cases where the signaling is fine
but it turns out that we can't establish an ICE connection. The former isn't
really signaled by the API but if it were it would be by the PeerConnection state.
Errors in the SDP just cause error callbacks generally.

http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtcpeerstate-enum


The latter is indicated by the ICE state:
http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtciceconnectionstate-enum

Comment 12

5 years ago
Ah, I wasn't checking oniceconnectionstatechange, just onsignalingstatechange, thanks. I see that the iceConnectionState is changing to 'failed' when the PeerConnection fails. Does this mean that this bug should be resolved?
Assignee

Comment 13

5 years ago
(In reply to Adam Ullman from comment #12)
> Ah, I wasn't checking oniceconnectionstatechange, just
> onsignalingstatechange, thanks. I see that the iceConnectionState is
> changing to 'failed' when the PeerConnection fails. Does this mean that this
> bug should be resolved?

My understanding is that you wanted to know when an initial ICE connection never got established. This works by monitoring the iceConnectionState.
The second part, which I believe this ticket is about, is to get notified that an existing connection broke while you are using it.

Comment 14

5 years ago
It would be great to see this get resolved in a manner consistent with the chrome implementation.  At present when working with firefox we have no reliable way of determining when the remote party has disconnected using the RTCPeerConnection object alone (i.e. you can use your signalling server to provide information regarding peers leaving, but having to rely on that mechanism in a P2P networking situation is pretty poor).

Comment 15

5 years ago
I can confirm that oniceconnectionstatechange is not fired if iceConnectionState is disconnected. In Chrome browser works without problems (there's a delay of ~5 seconds, but it's ok).
Does somebody have a workaround for the moment until this gets solved?

Comment 16

5 years ago
Can the priority on this be increased somehow? We'd really appreciate correct iceConnectionState change events for disconnected and closed peer connections: http://w3c.github.io/webrtc-pc/#rtciceconnectionstate-enum

Comment 17

5 years ago
BUMP
Assignee

Comment 18

4 years ago
I think implementing ICE consent freshness (bug 929977) is the only reasonable approach to detect an ICE connection failure after a connection initially got established.
Depends on: 929977
Assignee

Comment 19

4 years ago
Important point in the discussion here: I assume that most comments here are actually interested in the use case where the far end of an established PeerConnection suddenly dis-appeared, e.g. laptop closed, device out of battery, device lost IP access, but locally everything is still working fine.
Assignee

Comment 20

4 years ago
As failed and closed are actually states which are reached I think we are talking here "only" about the disconnected state: http://mxr.mozilla.org/mozilla-central/source/dom/media/PeerConnection.js?mark=1146-1149#1119
Summary: Report WebRTC transport termination (e.g. iceConnectionState=failed|disconnected|closed|) → Report WebRTC transport termination (e.g. iceConnectionState=disconnected)

Comment 21

4 years ago
(In reply to Nils Ohlmeier [:drno] from comment #19)
> Important point in the discussion here: I assume that most comments here are
> actually interested in the use case where the far end of an established
> PeerConnection suddenly dis-appeared, e.g. laptop closed, device out of
> battery, device lost IP access, but locally everything is still working fine.

I don't think a local connection loss (e.g. wifi goes down) is currently reported either? API consumers would want to know about those too. Though of course they might be two separate cases to handle...
backlog: --- → webRTC+
Rank: 27

Comment 22

3 years ago
Any updates so far? when this will be fixed?
It is hard to say. We have a lot on our plate this year.

Comment 24

3 years ago
Hi Guies and Babes.

I am getting the following error in twilio integration instead i am having Live credential with that Twilio ****.

the error is : 
ICE negotiation with Twilio failed. Call will terminate.
could you please suggest me any solution for that coz i  think its webrtc problem on browser based.
i have tested on every browser though.

please help
It seems very unlikely that I'll have an opportunity to work on WebRTC platform development for the foreseeable future. I'm unassigning this bug so that someone else might pick it up.
Assignee: adam → nobody
Assignee

Updated

3 years ago
QA Contact: jsmith

Comment 26

3 years ago
In a case that nobody will work on this issue, Do you have any recommendations how to handle peer disconnected state in Firefox?
Once consent freshness (bug 929977, which drno is working on) is done, it should be fairly easy to implement this.
(In reply to Igor from comment #26)
> In a case that nobody will work on this issue, Do you have any
> recommendations how to handle peer disconnected state in Firefox?

One workaround is http://stackoverflow.com/questions/35577657/detect-offline-peer-in-webrtc-connection/35591205#35591205

Comment 29

3 years ago
This is really annoying, since it requires heavy workarounds. I was tempted to use a keep-alive ping until I gladly discovered Jan-Ivar's solution. Hope the bug will be fixed soon...
Consent freshness (bug 929977) will land early in Fx49 Nightly (in about 2 weeks).  As Byron says in Comment 27, once that lands, this is fairly easy to implement. So I'm bumping this to a P1.  I'd like to target landing this in Fx49 (before Fx49 uplifts to Dev Edition).

Nils -- Do you want to take this as a follow on to bug 929977?
Rank: 27 → 17
Flags: needinfo?(drno)
Priority: P2 → P1

Comment 31

3 years ago
Thanks for the quick reaction, highly appreciated! :-)

Comment 32

3 years ago
Great to see the progress here

I can confirm that Jan-Ivar's solution works and we already implemented it
But hope to see a true fix here soon
Assignee

Updated

3 years ago
Assignee: nobody → drno
Flags: needinfo?(drno)

Comment 33

3 years ago
When can we expect this issue to be resolved.
Assignee

Comment 34

3 years ago
Since bug 929977 has made it's way into Firefox 49 which has become the official Firefox release recently Firefox will switch the ICE connection state to 'failed' if ICE consent freshness fails. Note that per ICE consent refresh RFC the timeout is fairly high at 30 seconds.

As there is still work going on on the ICE state transition flows I don't think it makes sense to consider switching to other states right now.

I'm inclined to actually close this bug as Firefox now reports if a transport has failed.

Comment 35

3 years ago
It takes too long to get the "failed" state ~30 secs before that client has no way to know about the network issues, "disconnected" state is helpful to know the momentary network glitches (chrome usually takes 5 sec to detect and trigger disconnected state change) and this can is used to let the user know about brief network issues and can help in improving the user experience.
 
When you say you are going to close, are you not going to implement all the ice states mentioned in http://w3c.github.io/webrtc-pc/#rtciceconnectionstate-enum or deferring it for now ?
Comment hidden (mozreview-request)

Comment 38

3 years ago
mozreview-review
Comment on attachment 8806172 [details]
Bug 852665: add support for ICE disconnected state.

https://reviewboard.mozilla.org/r/89672/#review89282

::: media/mtransport/nricectx.cpp:627
(Diff revision 1)
>    // Create the handler objects
>    ice_handler_vtbl_ = new nr_ice_handler_vtbl();
>    ice_handler_vtbl_->select_pair = &NrIceCtx::select_pair;
>    ice_handler_vtbl_->stream_ready = &NrIceCtx::stream_ready;
>    ice_handler_vtbl_->stream_failed = &NrIceCtx::stream_failed;
> -  ice_handler_vtbl_->ice_completed = &NrIceCtx::ice_completed;
> +  ice_handler_vtbl_->ice_completed = &NrIceCtx::ice_connected;

Should we rename this in the vtbl too?

::: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1289
(Diff revision 1)
> +void nr_ice_component_consent_calc_consent_timer(nr_ice_component *comp)
> +  {
> +    uint16_t trange, trand, tval;
> +    void *buf = &trand;
> +
> +    trange = NR_ICE_CONSENT_TIMER_DEFAULT / 100 * 20;

I guess rounding error will be smaller if you multiply by 20 first.

::: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1291
(Diff revision 1)
> +    uint16_t trange, trand, tval;
> +    void *buf = &trand;
> +
> +    trange = NR_ICE_CONSENT_TIMER_DEFAULT / 100 * 20;
> +    tval = NR_ICE_CONSENT_TIMER_DEFAULT - trange;
> +    if (!nr_crypto_random_bytes(buf, sizeof(trand)))

I feel like ((UCHAR*)&trand, sizeof(trand)) would be easier to read.

::: media/mtransport/third_party/nICEr/src/ice/ice_component.c:1346
(Diff revision 1)
> -      tval += (trand % (trange * 2));
> -
> -    if (comp->ctx->test_timer_divider)
> -      tval = tval / comp->ctx->test_timer_divider;
>  
> -    NR_ASYNC_TIMER_SET(tval, nr_ice_component_consent_timer_cb, comp,
> +    NR_ASYNC_TIMER_SET(comp->consent_ctx->maximum_transmits_timeout_ms + 100,

What's the + 100 for?
Attachment #8806172 - Flags: review?(docfaraday) → review+
Assignee

Comment 39

3 years ago
mozreview-review-reply
Comment on attachment 8806172 [details]
Bug 852665: add support for ICE disconnected state.

https://reviewboard.mozilla.org/r/89672/#review89282

> What's the + 100 for?

Originally I thought I could leverage the ICE transaction timer to kick off the next round of consent tries and wanted to avoid that the order of the two timers would get swapped. But we need an external timer any way to kick off the first round of consent, and then later we would have to calculate the difference between our calculated random time out value and the actual time it took until we got a reply or things timed out. So I left it with the extra timer. I'm going to remove the extra 100ms.
Comment hidden (mozreview-request)

Comment 41

3 years ago
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/dec79837f590
add support for ICE disconnected state. r=bwc

Comment 42

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dec79837f590
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Assignee

Updated

3 years ago
Blocks: 1320150
You need to log in before you can comment on or make changes to this bug.