Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

desktop playback error handling #10765

Merged
merged 11 commits into from
Mar 3, 2022
4 changes: 4 additions & 0 deletions .github/ISSUE_TEMPLATE/testplan.md
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,10 @@ and non interactive tsh bench loads.
- [ ] Verify async recording (`mode: node` or `mode: proxy`)
- [ ] Sessions show up in session recordings UI with desktop icon
- [ ] Sessions can be played back, including play/pause functionality
- [ ] A session that ends with a TDP error message can be played back, ends by displaying the error message,
and the progress bar progresses to the end.
- [ ] Attempting to play back a session that doesn't exist (i.e. by entering a non-existing session id in the url) shows
a relevant error message.
- [ ] RBAC for sessions: ensure users can only see their own recordings when
using the RBAC rule from our
[docs](../../docs/pages/access-controls/reference.mdx#rbac-for-sessions)
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/desktop/windows_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ func (s *WindowsService) makeTDPSendHandler(ctx context.Context, emitter events.
id *tlsca.Identity, sessionID, desktopAddr string) func(m tdp.Message, b []byte) {
return func(m tdp.Message, b []byte) {
switch b[0] {
case byte(tdp.TypePNGFrame):
case byte(tdp.TypePNGFrame), byte(tdp.TypeError):
e := &events.DesktopRecording{
Metadata: events.Metadata{
Type: libevents.DesktopRecordingEvent,
Expand Down
18 changes: 17 additions & 1 deletion lib/web/desktop/playback.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ package desktop
import (
"context"
"errors"
"fmt"
"net"
"os"
"sync"
"time"

apievents "github.com/gravitational/teleport/api/types/events"
"github.com/gravitational/teleport/lib/session"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"golang.org/x/net/websocket"
)
Expand Down Expand Up @@ -207,12 +210,21 @@ func (pp *Player) streamSessionEvents(ctx context.Context, cancel context.Cancel
// otherwise it just sits at the player UI
if err != nil && !errors.Is(err, context.Canceled) {
pp.log.WithError(err).Errorf("streaming session %v", pp.sID)
var errorText string
if os.IsNotExist(err) || trace.IsNotFound(err) {
errorText = "session not found"
} else {
errorText = "server error"
}
if _, err := pp.ws.Write([]byte(fmt.Sprintf(`{"message": "error", "errorText": "%v"}`, errorText))); err != nil {
pp.log.WithError(err).Error("failed to write \"error\" message over websocket")
}
}
return
case evt := <-eventsC:
if evt == nil {
pp.log.Debug("reached end of playback")
if _, err := pp.ws.Write([]byte(`{"message":"end"}`)); err != nil {
if _, err := pp.ws.Write([]byte(`{"message": "end"}`)); err != nil {
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
pp.log.WithError(err).Error("failed to write \"end\" message over websocket")
}
return
Expand All @@ -227,6 +239,10 @@ func (pp *Player) streamSessionEvents(ctx context.Context, cancel context.Cancel
msg, err := utils.FastMarshal(e)
if err != nil {
pp.log.WithError(err).Errorf("failed to marshal DesktopRecording event into JSON: %v", e)
if _, err := pp.ws.Write([]byte(`{"message": "error", "errorText": "server error"}`)); err != nil {
pp.log.WithError(err).Error("failed to write \"error\" message over websocket")
}
return
}
if _, err := pp.ws.Write(msg); err != nil {
// We expect net.ErrClosed to arise when another goroutine returns before
Expand Down
2 changes: 1 addition & 1 deletion lib/web/desktop/playback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestStreamsDesktopEvents(t *testing.T) {
b := make([]byte, 4096)
n, err := ws.Read(b)
require.NoError(t, err)
require.Equal(t, []byte(`{"message":"end"}`), b[:n])
require.JSONEq(t, `{"message":"end"}`, string(b[:n]))
}

func newServer(t *testing.T, streamInterval time.Duration, events []apievents.AuditEvent) *httptest.Server {
Expand Down