From b8c9e816b35651ce0cd874850052dbb5451e4946 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 14:57:04 +0200 Subject: [PATCH 1/8] Remove unnecessary assignments to the blank identifier. --- src/signaling/hub.go | 4 ++-- src/signaling/mcu_janus.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/signaling/hub.go b/src/signaling/hub.go index 8545121..d840346 100644 --- a/src/signaling/hub.go +++ b/src/signaling/hub.go @@ -456,13 +456,13 @@ func (h *Hub) GetSessionByPublicId(sessionId string) Session { } h.mu.Lock() - session, _ := h.sessions[data.Sid] + session := h.sessions[data.Sid] h.mu.Unlock() return session } func (h *Hub) checkExpiredSessions(now time.Time) { - for s, _ := range h.expiredSessions { + for s := range h.expiredSessions { if s.IsExpired(now) { h.mu.Unlock() log.Printf("Closing expired session %s (private=%s)", s.PublicId(), s.PrivateId()) diff --git a/src/signaling/mcu_janus.go b/src/signaling/mcu_janus.go index f8f921d..0acd282 100644 --- a/src/signaling/mcu_janus.go +++ b/src/signaling/mcu_janus.go @@ -245,7 +245,7 @@ func (m *mcuJanus) doReconnect() { m.mu.Unlock() m.muClients.Lock() - for client, _ := range m.clients { + for client := range m.clients { go client.NotifyReconnected() } m.muClients.Unlock() From 3f4693dca4d8b7c794bf87d88d7154be59a6490d Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 14:58:23 +0200 Subject: [PATCH 2/8] Remove unnecessary sub-loop. --- src/signaling/backend_configuration.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/signaling/backend_configuration.go b/src/signaling/backend_configuration.go index ffc547c..c99cefe 100644 --- a/src/signaling/backend_configuration.go +++ b/src/signaling/backend_configuration.go @@ -180,9 +180,7 @@ func (b *BackendConfiguration) GetBackend(u *url.URL) *Backend { func (b *BackendConfiguration) GetBackends() []*Backend { var result []*Backend for _, entries := range b.backends { - for _, entry := range entries { - result = append(result, entry) - } + result = append(result, entries...) } return result } From 67c5bebdcb860d834f7655def72fb5c3ff512734 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:00:39 +0200 Subject: [PATCH 3/8] Remove unused members. --- src/signaling/hub.go | 1 - src/signaling/room.go | 1 - 2 files changed, 2 deletions(-) diff --git a/src/signaling/hub.go b/src/signaling/hub.go index d840346..1747081 100644 --- a/src/signaling/hub.go +++ b/src/signaling/hub.go @@ -100,7 +100,6 @@ type Hub struct { upgrader websocket.Upgrader cookie *securecookie.SecureCookie info *HelloServerMessageServer - version string stopped int32 stopChan chan bool diff --git a/src/signaling/room.go b/src/signaling/room.go index ee520ed..aebe118 100644 --- a/src/signaling/room.go +++ b/src/signaling/room.go @@ -53,7 +53,6 @@ type Room struct { backend *Backend properties *json.RawMessage - roomType int closeChan chan bool mu *sync.RWMutex From efc232fb9c3013eebf675bfb4be834f704a82df7 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:19:11 +0200 Subject: [PATCH 4/8] Don't call "Fatal" from inside a goroutine. --- src/signaling/backend_server_test.go | 15 +++++++++++---- src/signaling/natsclient_loopback_test.go | 15 ++++++++++----- src/signaling/testclient_test.go | 3 ++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/signaling/backend_server_test.go b/src/signaling/backend_server_test.go index c800906..3403573 100644 --- a/src/signaling/backend_server_test.go +++ b/src/signaling/backend_server_test.go @@ -1068,11 +1068,13 @@ func TestBackendServer_ParticipantsUpdateTimeout(t *testing.T) { data, err := json.Marshal(msg) if err != nil { - t.Fatal(err) + t.Error(err) + return } res, err := performBackendRequest(server.URL+"/api/v1/room/"+roomId, data) if err != nil { - t.Fatal(err) + t.Error(err) + return } defer res.Body.Close() body, err := ioutil.ReadAll(res.Body) @@ -1119,11 +1121,13 @@ func TestBackendServer_ParticipantsUpdateTimeout(t *testing.T) { data, err := json.Marshal(msg) if err != nil { - t.Fatal(err) + t.Error(err) + return } res, err := performBackendRequest(server.URL+"/api/v1/room/"+roomId, data) if err != nil { - t.Fatal(err) + t.Error(err) + return } defer res.Body.Close() body, err := ioutil.ReadAll(res.Body) @@ -1136,6 +1140,9 @@ func TestBackendServer_ParticipantsUpdateTimeout(t *testing.T) { }() wg.Wait() + if t.Failed() { + return + } msg1_a, err := client1.RunUntilMessage(ctx) if err != nil { diff --git a/src/signaling/natsclient_loopback_test.go b/src/signaling/natsclient_loopback_test.go index 95bc2bd..680efed 100644 --- a/src/signaling/natsclient_loopback_test.go +++ b/src/signaling/natsclient_loopback_test.go @@ -88,7 +88,8 @@ func TestLoopbackNatsClient_Subscribe(t *testing.T) { if total == max { err := sub.Unsubscribe() if err != nil { - t.Fatal("Unsubscribe failed with err:", err) + t.Errorf("Unsubscribe failed with err: %s", err) + return } ch <- true } @@ -135,10 +136,12 @@ func TestLoopbackNatsClient_Request(t *testing.T) { go func() { msg := <-dest if err := client.Publish(msg.Reply, []byte("world")); err != nil { - t.Fatal(err) + t.Error(err) + return } if err := sub.Unsubscribe(); err != nil { - t.Fatal("Unsubscribe failed with err:", err) + t.Error("Unsubscribe failed with err:", err) + return } }() reply, err := client.Request("foo", []byte("hello"), 1*time.Second) @@ -182,10 +185,12 @@ func TestLoopbackNatsClient_RequestTimeout(t *testing.T) { msg := <-dest time.Sleep(200 * time.Millisecond) if err := client.Publish(msg.Reply, []byte("world")); err != nil { - t.Fatal(err) + t.Error(err) + return } if err := sub.Unsubscribe(); err != nil { - t.Fatal("Unsubscribe failed with err:", err) + t.Error("Unsubscribe failed with err:", err) + return } }() reply, err := client.Request("foo", []byte("hello"), 100*time.Millisecond) diff --git a/src/signaling/testclient_test.go b/src/signaling/testclient_test.go index 99393d9..4ae5681 100644 --- a/src/signaling/testclient_test.go +++ b/src/signaling/testclient_test.go @@ -233,7 +233,8 @@ func NewTestClient(t *testing.T, server *httptest.Server, hub *Hub) *TestClient readErrorChan <- err return } else if messageType != websocket.TextMessage { - t.Fatalf("Expect text message, got %d", messageType) + t.Errorf("Expect text message, got %d", messageType) + return } messageChan <- data From 5964a98218a1e07d4738299ee9546ac9bd2617e2 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:22:18 +0200 Subject: [PATCH 5/8] Remove unnecessary assignment to "err". --- src/signaling/backend_server_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/signaling/backend_server_test.go b/src/signaling/backend_server_test.go index 3403573..36a7834 100644 --- a/src/signaling/backend_server_test.go +++ b/src/signaling/backend_server_test.go @@ -1183,8 +1183,7 @@ func TestBackendServer_ParticipantsUpdateTimeout(t *testing.T) { ctx2, cancel2 := context.WithTimeout(context.Background(), time.Second+100*time.Millisecond) defer cancel2() - msg1_c, err := client1.RunUntilMessage(ctx2) - if msg1_c != nil { + if msg1_c, _ := client1.RunUntilMessage(ctx2); msg1_c != nil { if in_call_2, err := checkMessageParticipantsInCall(msg1_c); err != nil { t.Error(err) } else if len(in_call_2.Users) != 2 { @@ -1194,8 +1193,7 @@ func TestBackendServer_ParticipantsUpdateTimeout(t *testing.T) { ctx3, cancel3 := context.WithTimeout(context.Background(), time.Second+100*time.Millisecond) defer cancel3() - msg2_c, err := client2.RunUntilMessage(ctx3) - if msg2_c != nil { + if msg2_c, _ := client2.RunUntilMessage(ctx3); msg2_c != nil { if in_call_2, err := checkMessageParticipantsInCall(msg2_c); err != nil { t.Error(err) } else if len(in_call_2.Users) != 2 { From 6ffed30ddcc66ac1b474f74ad1023dd5be4b3218 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:23:41 +0200 Subject: [PATCH 6/8] Remove unused error result (was always "nil"). --- src/signaling/natsclient_loopback.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/signaling/natsclient_loopback.go b/src/signaling/natsclient_loopback.go index ef3bfc0..2c2f10f 100644 --- a/src/signaling/natsclient_loopback.go +++ b/src/signaling/natsclient_loopback.go @@ -66,14 +66,13 @@ func (s *loopbackNatsSubscription) Unsubscribe() error { return nil } -func (s *loopbackNatsSubscription) queue(msg *nats.Msg) error { +func (s *loopbackNatsSubscription) queue(msg *nats.Msg) { s.cond.L.Lock() s.incoming = append(s.incoming, msg) if len(s.incoming) == 1 { s.cond.Signal() } s.cond.L.Unlock() - return nil } func (s *loopbackNatsSubscription) run() { From 7d874ac1a711835de1be02e1e238903ae52f6c95 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:26:18 +0200 Subject: [PATCH 7/8] Don't cast bytes to string but use "String()" method. --- src/signaling/janus_client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/signaling/janus_client.go b/src/signaling/janus_client.go index 4d98358..5a47ff9 100644 --- a/src/signaling/janus_client.go +++ b/src/signaling/janus_client.go @@ -430,13 +430,13 @@ func (gateway *JanusGateway) recv() { decoder := json.NewDecoder(data) decoder.UseNumber() if err := decoder.Decode(&base); err != nil { - log.Printf("json.Unmarshal of %s: %s", string(decodeBuffer.Bytes()), err) + log.Printf("json.Unmarshal of %s: %s", decodeBuffer.String(), err) continue } typeFunc, ok := msgtypes[base.Type] if !ok { - log.Printf("Unknown message type received: %s", string(decodeBuffer.Bytes())) + log.Printf("Unknown message type received: %s", decodeBuffer.String()) continue } @@ -445,7 +445,7 @@ func (gateway *JanusGateway) recv() { decoder = json.NewDecoder(data) decoder.UseNumber() if err := decoder.Decode(&msg); err != nil { - log.Printf("json.Unmarshal of %s: %s", string(decodeBuffer.Bytes()), err) + log.Printf("json.Unmarshal of %s: %s", decodeBuffer.String(), err) continue // Decode error } @@ -461,7 +461,7 @@ func (gateway *JanusGateway) recv() { session := gateway.Sessions[base.Session] gateway.Unlock() if session == nil { - log.Printf("Unable to deliver message %s. Session %d gone?", string(decodeBuffer.Bytes()), base.Session) + log.Printf("Unable to deliver message %s. Session %d gone?", decodeBuffer.String(), base.Session) continue } @@ -470,7 +470,7 @@ func (gateway *JanusGateway) recv() { handle := session.Handles[base.Handle] session.Unlock() if handle == nil { - log.Printf("Unable to deliver message %s. Handle %d gone?", string(decodeBuffer.Bytes()), base.Handle) + log.Printf("Unable to deliver message %s. Handle %d gone?", decodeBuffer.String(), base.Handle) continue } From 68477474f89a7e7931ee2973e2d201d8427e8526 Mon Sep 17 00:00:00 2001 From: Joachim Bauch Date: Fri, 31 Jul 2020 15:32:42 +0200 Subject: [PATCH 8/8] Don't use "select" with a single case that reads from a channel but read channel directly. --- src/server/main.go | 6 ++---- src/signaling/natsclient_loopback.go | 12 +++++------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/server/main.go b/src/server/main.go index 48b1b44..468b899 100644 --- a/src/server/main.go +++ b/src/server/main.go @@ -288,8 +288,6 @@ func main() { } } - select { - case <-interrupt: - log.Println("Interrupted") - } + <-interrupt + log.Println("Interrupted") } diff --git a/src/signaling/natsclient_loopback.go b/src/signaling/natsclient_loopback.go index 2c2f10f..42476e5 100644 --- a/src/signaling/natsclient_loopback.go +++ b/src/signaling/natsclient_loopback.go @@ -148,13 +148,11 @@ func (c *LoopbackNatsClient) Request(subject string, data []byte, timeout time.D c.mu.Unlock() ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - select { - case <-ctx.Done(): - if ctx.Err() == context.DeadlineExceeded { - err = nats.ErrTimeout - } else { - err = ctx.Err() - } + <-ctx.Done() + if ctx.Err() == context.DeadlineExceeded { + err = nats.ErrTimeout + } else { + err = ctx.Err() } c.mu.Lock() return nil, err