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

desktop playback error handling #638

Merged
merged 7 commits into from
Mar 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 119 additions & 46 deletions packages/teleport/src/Player/DesktopPlayer.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';

import styled from 'styled-components';

import { Indicator, Box } from 'design';
import { Indicator, Box, Alert } from 'design';

import useAttempt from 'shared/hooks/useAttemptNext';

import cfg from 'teleport/config';
import { PlayerClient } from 'teleport/lib/tdp';
import { PlayerClient, PlayerClientEvent } from 'teleport/lib/tdp';
import { PngFrame, ClientScreenSpec } from 'teleport/lib/tdp/codec';
import { getAccessToken, getHostName } from 'teleport/services/api';
import TdpClientCanvas from 'teleport/components/TdpClientCanvas';
Expand All @@ -25,44 +27,53 @@ export const DesktopPlayer = ({
playerClient,
tdpCliOnPngFrame,
tdpCliOnClientScreenSpec,
showCanvas,
tdpCliOnWsClose,
tdpCliOnTdpError,
attempt,
} = useDesktopPlayer({
sid,
clusterId,
});

const displayCanvas = attempt.status === 'success' || attempt.status === '';
const displayProgressBar = attempt.status !== 'processing';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the progress bar be visible on error? also wondering if the progress bar can still be moved around on error and if that would cause errors

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided that for now it should be, since sometimes the error is actually part of the playback (i.e. if the session itself ended with a tdp error). Currently moving the progress bar doesn't actually change anything, so it isn't a problem. When it does become active, it will need to be accounted for.


return (
<>
<StyledPlayer>
{!showCanvas && (
<Box textAlign="center" m={10}>
<Indicator />
</Box>
)}

<TdpClientCanvas
tdpCli={playerClient}
tdpCliOnPngFrame={tdpCliOnPngFrame}
tdpCliOnClientScreenSpec={tdpCliOnClientScreenSpec}
onContextMenu={() => true}
// overflow: 'hidden' is needed to prevent the canvas from outgrowing the container due to some weird css flex idiosyncracy.
// See https://gaurav5430.medium.com/css-flex-positioning-gotchas-child-expands-to-more-than-the-width-allowed-by-the-parent-799c37428dd6.
style={{
alignSelf: 'center',
overflow: 'hidden',
display: showCanvas ? 'flex' : 'none',
}}
/>
<ProgressBarDesktop
playerClient={playerClient}
durationMs={durationMs}
style={{
display: showCanvas ? 'flex' : 'none',
}}
id="progressBarDesktop"
/>
</StyledPlayer>
</>
<StyledPlayer>
{attempt.status === 'processing' && (
<Box textAlign="center" m={10}>
<Indicator />
</Box>
)}

{attempt.status === 'failed' && (
<DesktopPlayerAlert my={4} children={attempt.statusText} />
)}

<TdpClientCanvas
tdpCli={playerClient}
tdpCliOnPngFrame={tdpCliOnPngFrame}
tdpCliOnClientScreenSpec={tdpCliOnClientScreenSpec}
tdpCliOnWsClose={tdpCliOnWsClose}
tdpCliOnTdpError={tdpCliOnTdpError}
onContextMenu={() => true}
// overflow: 'hidden' is needed to prevent the canvas from outgrowing the container due to some weird css flex idiosyncracy.
// See https://gaurav5430.medium.com/css-flex-positioning-gotchas-child-expands-to-more-than-the-width-allowed-by-the-parent-799c37428dd6.
style={{
alignSelf: 'center',
overflow: 'hidden',
display: displayCanvas ? 'flex' : 'none',
}}
/>
<ProgressBarDesktop
playerClient={playerClient}
durationMs={durationMs}
style={{
display: displayProgressBar ? 'flex' : 'none',
}}
id="progressBarDesktop"
/>
</StyledPlayer>
);
};

Expand All @@ -73,15 +84,21 @@ const useDesktopPlayer = ({
sid: string;
clusterId: string;
}) => {
const playerClient = new PlayerClient(
cfg.api.desktopPlaybackWsAddr
.replace(':fqdn', getHostName())
.replace(':clusterId', clusterId)
.replace(':sid', sid)
.replace(':token', getAccessToken())
);

const [showCanvas, setShowCanvas] = useState(false);
const [playerClient, setPlayerClient] = useState<PlayerClient | null>(null);
// attempt.status === '' means the playback ended gracefully
const { attempt, setAttempt } = useAttempt('processing');

useEffect(() => {
setPlayerClient(
new PlayerClient(
cfg.api.desktopPlaybackWsAddr
.replace(':fqdn', getHostName())
.replace(':clusterId', clusterId)
.replace(':sid', sid)
.replace(':token', getAccessToken())
)
);
}, [clusterId, sid]);

const tdpCliOnPngFrame = (
ctx: CanvasRenderingContext2D,
Expand Down Expand Up @@ -113,14 +130,61 @@ const useDesktopPlayer = ({
canvas.width = spec.width;
canvas.height = spec.height;

setShowCanvas(true);
setAttempt({ status: 'success' });
};

useEffect(() => {
if (playerClient) {
playerClient.addListener(PlayerClientEvent.SESSION_END, () => {
setAttempt({ status: '' });
});

playerClient.addListener(
PlayerClientEvent.PLAYBACK_ERROR,
(err: Error) => {
setAttempt({
status: 'failed',
statusText: `There was an error while playing this session: ${err.message}`,
});
}
);

return () => {
playerClient.nuke();
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
};
}
}, [playerClient]);

// If the websocket closed for some reason other than the session playback ending,
// as signaled by the server (which sets prevAttempt.status = '' in
// the PlayerClientEvent.SESSION_END event handler), or a TDP message from the server
// signalling an error, assume some sort of network or playback error and alert the user.
const tdpCliOnWsClose = () => {
setAttempt(prevAttempt => {
if (prevAttempt.status !== '' && prevAttempt.status !== 'failed') {
return {
status: 'failed',
statusText: 'connection to the server failed for an unknown reason',
};
}
return prevAttempt;
});
};

const tdpCliOnTdpError = (err: Error) => {
setAttempt({
status: 'failed',
statusText: err.message,
});
};

return {
playerClient,
tdpCliOnPngFrame,
tdpCliOnClientScreenSpec,
showCanvas,
tdpCliOnWsClose,
tdpCliOnTdpError,
attempt,
};
};

Expand All @@ -131,3 +195,12 @@ const StyledPlayer = styled.div`
width: 100%;
height: 100%;
`;

const DesktopPlayerAlert = styled(Alert)`
align-self: center;
min-width: 450px;
// Overrides StyledPlayer container's justify-content
// https://stackoverflow.com/a/34063808/6277051
margin-bottom: auto;
`;
82 changes: 48 additions & 34 deletions packages/teleport/src/Player/ProgressBar/ProgressBarDesktop.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { throttle } from 'lodash';
import { dateToUtc } from 'shared/services/loc';
import { format } from 'date-fns';

import { PlayerClient, PlayerClientEvent } from 'teleport/lib/tdp';
import {
PlayerClient,
PlayerClientEvent,
TdpClientEvent,
} from 'teleport/lib/tdp';

import ProgressBar from './ProgressBar';

Expand All @@ -29,47 +33,57 @@ export const ProgressBarDesktop = (props: {
});

useEffect(() => {
playerClient.addListener(PlayerClientEvent.TOGGLE_PLAY_PAUSE, () => {
// setState({...state, isPlaying: !state.isPlaying}) doesn't work because
// the listener is added when state == initialState, and that initialState
// value is effectively hardcoded into its logic.
setState(prevState => {
return { ...prevState, isPlaying: !prevState.isPlaying };
if (playerClient) {
playerClient.addListener(PlayerClientEvent.TOGGLE_PLAY_PAUSE, () => {
// setState({...state, isPlaying: !state.isPlaying}) doesn't work because
// the listener is added when state == initialState, and that initialState
// value is effectively hardcoded into its logic.
setState(prevState => {
return { ...prevState, isPlaying: !prevState.isPlaying };
});
});
});

const throttledUpdateCurrentTime = throttle(
currentTimeMs => {
const throttledUpdateCurrentTime = throttle(
currentTimeMs => {
setState(prevState => {
return {
...prevState,
current: currentTimeMs,
time: toHuman(currentTimeMs),
};
});
},
// Magic number to throttle progress bar updates so that the playback is smoother.
50
);

playerClient.addListener(
PlayerClientEvent.UPDATE_CURRENT_TIME,
currentTimeMs => throttledUpdateCurrentTime(currentTimeMs)
);

const progressToEnd = () => {
throttledUpdateCurrentTime.cancel();
// TODO(isaiah): Make this smoother
// https://github.com/gravitational/webapps/issues/579
setState(prevState => {
return {
...prevState,
current: currentTimeMs,
time: toHuman(currentTimeMs),
};
return { ...prevState, current: durationMs };
});
},
// Magic number to throttle progress bar updates so that the playback is smoother.
50
);
};

playerClient.addListener(
PlayerClientEvent.UPDATE_CURRENT_TIME,
currentTimeMs => throttledUpdateCurrentTime(currentTimeMs)
);
playerClient.addListener(PlayerClientEvent.SESSION_END, () => {
progressToEnd();
});

playerClient.addListener(PlayerClientEvent.SESSION_END, () => {
throttledUpdateCurrentTime.cancel();
// TODO(isaiah): Make this smoother
// https://github.com/gravitational/webapps/issues/579
setState(prevState => {
return { ...prevState, current: durationMs };
playerClient.addListener(TdpClientEvent.TDP_ERROR, () => {
progressToEnd();
});
});

return () => {
throttledUpdateCurrentTime.cancel();
playerClient.nuke();
};
return () => {
throttledUpdateCurrentTime.cancel();
playerClient.nuke();
};
}
}, [playerClient]);

return (
Expand Down
2 changes: 2 additions & 0 deletions packages/teleport/src/lib/tdp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ export default class Client extends EventEmitterWebAuthnSender {
// Ensures full cleanup of this object.
// Note that it removes all listeners first and then cleans up the socket,
// so don't call this if your calling object is relying on listeners.
// It's safe to call this multiple times, calls subsequent to the first call
// will simply do nothing.
nuke() {
this.removeAllListeners();
this.socket?.close();
Expand Down
12 changes: 8 additions & 4 deletions packages/teleport/src/lib/tdp/playerClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export enum PlayerClientEvent {
TOGGLE_PLAY_PAUSE = 'play/pause',
UPDATE_CURRENT_TIME = 'time',
SESSION_END = 'end',
PLAYBACK_ERROR = 'playback error',
}

export class PlayerClient extends Client {
Expand All @@ -42,13 +43,16 @@ export class PlayerClient extends Client {
// Overrides Client implementation.
processMessage(buffer: ArrayBuffer) {
const json = JSON.parse(this.textDecoder.decode(buffer));

if (json.message === 'end') {
this.emit(PlayerClientEvent.SESSION_END);
return;
} else if (json.message === 'error') {
this.emit(PlayerClientEvent.PLAYBACK_ERROR, new Error(json.errorText));
} else {
const ms = json.ms;
this.emit(PlayerClientEvent.UPDATE_CURRENT_TIME, ms);
super.processMessage(base64ToArrayBuffer(json.message));
}
const ms = json.ms;
super.processMessage(base64ToArrayBuffer(json.message));
this.emit(PlayerClientEvent.UPDATE_CURRENT_TIME, ms);
}

// Overrides Client implementation.
Expand Down