Repository: pions/webrtc (go)

Issue:
Handle disconnected peers with track sharing
### Your environment.
  - Version: ab81640

### What did you do?
Using the basic SFU example with some custom signalling we connect a source(pions) and several consumers(browsers), and let them run. At least 1 in every 10 connections will transition to disconnected after approx 30 seconds which is fine but as soon as one of the peer connections using the sources track terminates all of the remaining peers stop receiving data. The Write method on the local track returns "No Valid Candidate Pairs Available" immediately and all consumers must manually reconnect after the source peer connection has been recreated from scratch (consumers enter an eternal connected state and cannot be closed, source also can't be cleaned up).

### What did you expect?
One transitioning ICE state should not down the whole SFU. Not sure of the best way to fix this, but it would be very useful if we had a "don't give up" parameter for this use case or provide an iterator for the senders that we can extract the offending peer connection from.

Something like
```
for _, s := range localTrack.Senders() {
	if err := s.SendRTP(rtpBuf[:i]); err != nil {
		// User defined
	}
}
or
localTrack.ForEachSender(func(s *webrtc.RTPSender) bool {
	if err := s.SendRTP(rtpBuf[:i]); err != nil {
		return true / false // Where true means break
	}

	return false
})
```

Pull request description:
Don't notify users via Track of ICE errors
Writing to a Track shouldn't return errors for an individual
RTPSender. This filters ErrNoCandidatePairs from being returned
and instead just returns nil

Resolves #523

----

@thetooth mind checking out this PR and seeing if it works for you? thanks!

Patch:
diff --git a/dtlstransport.go b/dtlstransport.go
index 8ee389c7..4f356ebe 100644
--- a/dtlstransport.go
+++ b/dtlstransport.go
@@ -27,9 +27,10 @@ import (
 type DTLSTransport struct {
 	lock sync.RWMutex
 
-	iceTransport     *ICETransport
-	certificates     []Certificate
-	remoteParameters DTLSParameters
+	iceTransport      *ICETransport
+	certificates      []Certificate
+	remoteParameters  DTLSParameters
+	remoteCertificate []byte
 	// State     DTLSTransportState
 
 	// OnStateChange func()
@@ -95,6 +96,14 @@ func (t *DTLSTransport) GetLocalParameters() DTLSParameters {
 	}
 }
 
+// GetRemoteCertificate returns the certificate chain in use by the remote side
+// returns an empty list prior to selection of the remote certificate
+func (t *DTLSTransport) GetRemoteCertificate() []byte {
+	t.lock.RLock()
+	defer t.lock.RUnlock()
+	return t.remoteCertificate
+}
+
 func (t *DTLSTransport) startSRTP() error {
 	t.lock.Lock()
 	defer t.lock.Unlock()
@@ -220,6 +229,7 @@ func (t *DTLSTransport) Start(remoteParameters DTLSParameters) error {
 		return fmt.Errorf("peer didn't provide certificate via DTLS")
 	}
 
+	t.remoteCertificate = remoteCert.Raw
 	return t.validateFingerPrint(remoteParameters, remoteCert)
 }
 
diff --git a/go.mod b/go.mod
index 3f8e09f0..0b23fec4 100644
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/pions/webrtc
 require (
 	github.com/pions/datachannel v1.2.0
 	github.com/pions/dtls v1.2.2
-	github.com/pions/ice v0.1.0
+	github.com/pions/ice v0.1.1
 	github.com/pions/logging v0.1.0
 	github.com/pions/quic v0.0.1
 	github.com/pions/rtcp v1.1.2
diff --git a/go.sum b/go.sum
index 55584013..29a72d2a 100644
--- a/go.sum
+++ b/go.sum
@@ -23,8 +23,8 @@ github.com/pions/datachannel v1.2.0 h1:N12qhHSRVlgBcaal2Hi4skdz7VI4yz6bNC5IJDMzC
 github.com/pions/datachannel v1.2.0/go.mod h1:MKPEKJRwX/a9/tyQvcVTUI9szyf8ZuUyZxSA9AVMSro=
 github.com/pions/dtls v1.2.2 h1:izn/74bIBxVuHb+NdirkmWMH/yFNd8PBVXJgeH/ofGU=
 github.com/pions/dtls v1.2.2/go.mod h1:5o0cLHyBEl8CvuA88enDDM6aBwn4SNo8md2dPhcynfc=
-github.com/pions/ice v0.1.0 h1:V3RaaUV7QEFUrNk2tHzRqNo3ldCwwnghEpDIQ8+m/eA=
-github.com/pions/ice v0.1.0/go.mod h1:vGnMrwYxOMmfR093luGSh2yMm1DDpPqj8lGRNsmo9VQ=
+github.com/pions/ice v0.1.1 h1:SZouAvl9RnrNnCHxSnurOfzqK2+oD0ZeURSoxuOs0tI=
+github.com/pions/ice v0.1.1/go.mod h1:vGnMrwYxOMmfR093luGSh2yMm1DDpPqj8lGRNsmo9VQ=
 github.com/pions/logging v0.1.0 h1:vr+vInmjaRI06CqPWErEQpywxaqoIZcUjQ2eN68jRbk=
 github.com/pions/logging v0.1.0/go.mod h1:duuz9/Se8ujqvq7OPzbnPpRlha6A0fk1Ba2wrbn4zew=
 github.com/pions/qtls-vendor-extracted v0.0.0-20190210024908-018998217c65 h1:skcEQZ2eUdm1WKlYu7y1y0HBzOwa1pgSAwvhG6PrI2s=
diff --git a/peerconnection.go b/peerconnection.go
index fe29d836..58b3c81c 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1328,6 +1328,9 @@ func (pc *PeerConnection) SendRTCP(pkt rtcp.Packet) error {
 	}
 
 	if _, err := writeStream.Write(raw); err != nil {
+		if err == ice.ErrNoCandidatePairs {
+			return nil
+		}
 		return fmt.Errorf("SendRTCP failed to write: %v", err)
 	}
 	return nil
diff --git a/rtpsender.go b/rtpsender.go
index 13c17c5b..901924de 100644
--- a/rtpsender.go
+++ b/rtpsender.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"sync"
 
+	"github.com/pions/ice"
 	"github.com/pions/rtcp"
 	"github.com/pions/rtp"
 	"github.com/pions/srtp"
@@ -149,7 +150,11 @@ func (r *RTPSender) sendRTP(header *rtp.Header, payload []byte) (int, error) {
 			return 0, err
 		}
 
-		return writeStream.WriteRTP(header, payload)
+		n, err := writeStream.WriteRTP(header, payload)
+		if err == ice.ErrNoCandidatePairs {
+			err = nil
+		}
+		return n, err
 	}
 }
 


Test patch:
diff --git a/peerconnection_media_test.go b/peerconnection_media_test.go
index 2ff18a1e..5cb6246e 100644
--- a/peerconnection_media_test.go
+++ b/peerconnection_media_test.go
@@ -183,12 +183,9 @@ func TestPeerConnection_Media_Shutdown(t *testing.T) {
 	report := test.CheckRoutines(t)
 	defer report()
 
-	pcOffer, err := NewPeerConnection(Configuration{})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	pcAnswer, err := NewPeerConnection(Configuration{})
+	api := NewAPI()
+	api.mediaEngine.RegisterDefaultCodecs()
+	pcOffer, pcAnswer, err := api.newPair()
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -266,3 +263,86 @@ func TestPeerConnection_Media_Shutdown(t *testing.T) {
 	}
 	onTrackFiredLock.Unlock()
 }
+
+/*
+Integration test for behavior around media and disconnected peers
+
+* Sending RTP and RTCP to a disconnected Peer shouldn't return an error
+
+*/
+func TestPeerConnection_Media_Disconnected(t *testing.T) {
+	lim := test.TimeOut(time.Second * 30)
+	defer lim.Stop()
+
+	report := test.CheckRoutines(t)
+	defer report()
+
+	s := SettingEngine{}
+	s.SetConnectionTimeout(time.Duration(1)*time.Second, time.Duration(250)*time.Millisecond)
+
+	api := NewAPI(WithSettingEngine(s))
+	api.mediaEngine.RegisterDefaultCodecs()
+
+	pcOffer, pcAnswer, err := api.newPair()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	vp8Track, err := pcOffer.NewTrack(DefaultPayloadTypeVP8, rand.Uint32(), "video", "pion2")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	vp8Sender, err := pcOffer.AddTrack(vp8Track)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	haveDisconnected := make(chan error)
+	pcOffer.OnICEConnectionStateChange(func(iceState ICEConnectionState) {
+		if iceState == ICEConnectionStateDisconnected {
+			close(haveDisconnected)
+		} else if iceState == ICEConnectionStateConnected {
+			// Assert that DTLS is done by pull remote certificate, don't tear down the PC early
+			for {
+				if len(vp8Sender.Transport().GetRemoteCertificate()) != 0 {
+					pcAnswer.sctpTransport.lock.RLock()
+					haveAssocation := pcAnswer.sctpTransport.association != nil
+					pcAnswer.sctpTransport.lock.RUnlock()
+
+					if haveAssocation {
+						break
+					}
+				}
+
+				time.Sleep(time.Second)
+			}
+
+			if pcCloseErr := pcAnswer.Close(); pcCloseErr != nil {
+				haveDisconnected <- pcCloseErr
+			}
+		}
+	})
+
+	err = signalPair(pcOffer, pcAnswer)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err, ok := <-haveDisconnected
+	if ok {
+		t.Fatal(err)
+	}
+	for i := 0; i <= 5; i++ {
+		if rtpErr := vp8Track.WriteSample(media.Sample{Data: []byte{0x00}, Samples: 1}); rtpErr != nil {
+			t.Fatal(rtpErr)
+		} else if rtcpErr := pcOffer.SendRTCP(&rtcp.PictureLossIndication{MediaSSRC: 0}); rtcpErr != nil {
+			t.Fatal(rtcpErr)
+		}
+	}
+
+	err = pcOffer.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+}