Skip to content
This repository has been archived by the owner on Aug 30, 2024. It is now read-only.

Commit

Permalink
fix: Close conn if context deadline is exceeded (#395)
Browse files Browse the repository at this point in the history
* fix: Close conn if context deadline is exceeded

Previously the error occurred but never propogated due to blocking fromm
the read loop in negotiate. Now, connection timeouts can properly occur.

* Fixes
  • Loading branch information
kylecarbs authored Jul 22, 2021
1 parent 3e78729 commit 53e7c33
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 3 deletions.
4 changes: 3 additions & 1 deletion wsnet/dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,16 @@ func (d *Dialer) negotiate(ctx context.Context) (err error) {
// so it's better to buffer and process than fail.
pendingCandidates = []webrtc.ICECandidateInit{}
)

go func() {
defer close(errCh)
defer func() {
_ = d.conn.Close()
}()
err := waitForConnectionOpen(ctx, d.rtc)
if err != nil {
if errors.Is(err, context.DeadlineExceeded) {
_ = d.conn.Close()
}
errCh <- err
return
}
Expand Down
13 changes: 13 additions & 0 deletions wsnet/dial_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net"
"strconv"
"testing"
"time"

"cdr.dev/slog/sloggers/slogtest"
"github.com/pion/ice/v2"
Expand Down Expand Up @@ -51,6 +52,18 @@ func ExampleDial_basic() {
}

func TestDial(t *testing.T) {
t.Run("Timeout", func(t *testing.T) {
t.Parallel()

connectAddr, _ := createDumbBroker(t)

ctx, cancelFunc := context.WithTimeout(context.Background(), time.Millisecond*50)
defer cancelFunc()
dialer, err := DialWebsocket(ctx, connectAddr, nil, nil)
require.True(t, errors.Is(err, context.DeadlineExceeded))
require.Error(t, dialer.conn.Close(), "already wrote close")
})

t.Run("Ping", func(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 1 addition & 1 deletion wsnet/rtc.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ func waitForConnectionOpen(ctx context.Context, conn *webrtc.PeerConnection) err
})
<-ctx.Done()
if ctx.Err() == context.DeadlineExceeded {
return ctx.Err()
return context.DeadlineExceeded
}
return nil
}
Expand Down
4 changes: 3 additions & 1 deletion wsnet/wsnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ func createDumbBroker(t testing.TB) (connectAddr string, listenAddr string) {
mut.Lock()
defer mut.Unlock()
if sess == nil {
t.Error("listen not called")
// We discard inbound to emulate a pubsub where we don't know if anyone
// is listening on the other side.
_, _ = io.Copy(io.Discard, nc)
return
}
oc, err := sess.Open()
Expand Down

0 comments on commit 53e7c33

Please sign in to comment.