From 97fcb82a77856bf9a258c0ea12b3ebb2e5771097 Mon Sep 17 00:00:00 2001 From: "Soungmin Son (Eddy)" <41810556+happycastle114@users.noreply.github.com> Date: Fri, 21 Mar 2025 15:56:34 +0900 Subject: [PATCH] Fix: Return NotFoundErr instead of Unavailable when the participant does not exist in UpdateParticipant. (#3543) * Check if Participant exists when update metadata * Change Test cases * type smuggle oss participant check into roomstore * tidy --------- Co-authored-by: Paul Wells --- go.mod | 2 +- go.sum | 4 ++-- pkg/service/interfaces.go | 4 ++++ pkg/service/localstore.go | 5 +++++ pkg/service/redisstore.go | 5 +++++ pkg/service/roomservice.go | 9 +++++++++ test/singlenode_test.go | 4 +--- 7 files changed, 27 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 5d56a4e96..cd2c04e29 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/jxskiss/base62 v1.1.0 github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 github.com/livekit/mediatransportutil v0.0.0-20250310153736-45596af895b6 - github.com/livekit/protocol v1.35.1-0.20250320161708-6d044a0462b3 + github.com/livekit/protocol v1.35.1-0.20250321043517-157eda585c9a github.com/livekit/psrpc v0.6.1-0.20250205181828-a0beed2e4126 github.com/mackerelio/go-osstat v0.2.5 github.com/magefile/mage v1.15.0 diff --git a/go.sum b/go.sum index 519bba7c2..4d3109960 100644 --- a/go.sum +++ b/go.sum @@ -170,8 +170,8 @@ github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1 h1:jm09419p0lqTkD github.com/livekit/mageutil v0.0.0-20230125210925-54e8a70427c1/go.mod h1:Rs3MhFwutWhGwmY1VQsygw28z5bWcnEYmS1OG9OxjOQ= github.com/livekit/mediatransportutil v0.0.0-20250310153736-45596af895b6 h1:6ZhtnY9I9knfm3ieIPpznQSEU2rDECO8yliW/ANLQ7U= github.com/livekit/mediatransportutil v0.0.0-20250310153736-45596af895b6/go.mod h1:36s+wwmU3O40IAhE+MjBWP3W71QRiEE9SfooSBvtBqY= -github.com/livekit/protocol v1.35.1-0.20250320161708-6d044a0462b3 h1:RsVnIuxnj3wRzWILhpnguyrq3vrK6Cccb762GpLH3Xg= -github.com/livekit/protocol v1.35.1-0.20250320161708-6d044a0462b3/go.mod h1:WrT/CYRxtMNOVUjnIPm5OjWtEkmreffTeE1PRZwlRg4= +github.com/livekit/protocol v1.35.1-0.20250321043517-157eda585c9a h1:NmBLyIS7rW9Jd/8WXLbo/Lk7ZuMgkwJDJiE1+ixGyDE= +github.com/livekit/protocol v1.35.1-0.20250321043517-157eda585c9a/go.mod h1:WrT/CYRxtMNOVUjnIPm5OjWtEkmreffTeE1PRZwlRg4= github.com/livekit/psrpc v0.6.1-0.20250205181828-a0beed2e4126 h1:fzuYpAQbCid7ySPpQWWePfQOWUrs8x6dJ0T3Wl07n+Y= github.com/livekit/psrpc v0.6.1-0.20250205181828-a0beed2e4126/go.mod h1:X5WtEZ7OnEs72Fi5/J+i0on3964F1aynQpCalcgMqRo= github.com/mackerelio/go-osstat v0.2.5 h1:+MqTbZUhoIt4m8qzkVoXUJg1EuifwlAJSk4Yl2GXh+o= diff --git a/pkg/service/interfaces.go b/pkg/service/interfaces.go index e2a6978c5..0e5b5fa5c 100644 --- a/pkg/service/interfaces.go +++ b/pkg/service/interfaces.go @@ -52,6 +52,10 @@ type ServiceStore interface { ListParticipants(ctx context.Context, roomName livekit.RoomName) ([]*livekit.ParticipantInfo, error) } +type OSSServiceStore interface { + HasParticipant(context.Context, livekit.RoomName, livekit.ParticipantIdentity) (bool, error) +} + //counterfeiter:generate . EgressStore type EgressStore interface { StoreEgress(ctx context.Context, info *livekit.EgressInfo) error diff --git a/pkg/service/localstore.go b/pkg/service/localstore.go index 005386ce9..b6de883bd 100644 --- a/pkg/service/localstore.go +++ b/pkg/service/localstore.go @@ -153,6 +153,11 @@ func (s *LocalStore) LoadParticipant(_ context.Context, roomName livekit.RoomNam return participant, nil } +func (s *LocalStore) HasParticipant(ctx context.Context, roomName livekit.RoomName, identity livekit.ParticipantIdentity) (bool, error) { + p, err := s.LoadParticipant(ctx, roomName, identity) + return p != nil, utils.ScreenError(err, ErrParticipantNotFound) +} + func (s *LocalStore) ListParticipants(_ context.Context, roomName livekit.RoomName) ([]*livekit.ParticipantInfo, error) { s.lock.RLock() defer s.lock.RUnlock() diff --git a/pkg/service/redisstore.go b/pkg/service/redisstore.go index 45576d37f..990dba1d4 100644 --- a/pkg/service/redisstore.go +++ b/pkg/service/redisstore.go @@ -314,6 +314,11 @@ func (s *RedisStore) LoadParticipant(_ context.Context, roomName livekit.RoomNam return &pi, nil } +func (s *RedisStore) HasParticipant(ctx context.Context, roomName livekit.RoomName, identity livekit.ParticipantIdentity) (bool, error) { + p, err := s.LoadParticipant(ctx, roomName, identity) + return p != nil, utils.ScreenError(err, ErrParticipantNotFound) +} + func (s *RedisStore) ListParticipants(_ context.Context, roomName livekit.RoomName) ([]*livekit.ParticipantInfo, error) { key := RoomParticipantsPrefix + string(roomName) items, err := s.rc.HVals(s.ctx, key).Result() diff --git a/pkg/service/roomservice.go b/pkg/service/roomservice.go index 558855322..2ec41b228 100644 --- a/pkg/service/roomservice.go +++ b/pkg/service/roomservice.go @@ -238,6 +238,15 @@ func (s *RoomService) UpdateParticipant(ctx context.Context, req *livekit.Update return nil, twirpAuthError(err) } + if os, ok := s.roomStore.(OSSServiceStore); ok { + found, err := os.HasParticipant(ctx, livekit.RoomName(req.Room), livekit.ParticipantIdentity(req.Identity)) + if err != nil { + return nil, err + } else if !found { + return nil, ErrParticipantNotFound + } + } + res, err := s.participantClient.UpdateParticipant(ctx, s.topicFormatter.ParticipantTopic(ctx, livekit.RoomName(req.Room), livekit.ParticipantIdentity(req.Identity)), req) RecordResponse(ctx, res) return res, err diff --git a/test/singlenode_test.go b/test/singlenode_test.go index b11e5da77..91668123b 100644 --- a/test/singlenode_test.go +++ b/test/singlenode_test.go @@ -356,9 +356,7 @@ func TestSingleNodeUpdateParticipant(t *testing.T) { require.Error(t, err) var twErr twirp.Error require.True(t, errors.As(err, &twErr)) - // Note: for Cloud this would return 404, currently we are not able to differentiate between - // non-existent participant vs server being unavailable in OSS - require.Equal(t, twirp.Unavailable, twErr.Code()) + require.Equal(t, twirp.NotFound, twErr.Code()) }) }