desktop playback error handling (#638)

gravitational · Mar 3, 2022 · 9cfa8f7 · 9cfa8f7
1 parent c49a241
commit 9cfa8f7
Show file tree

Hide file tree

Showing 4 changed files with 177 additions and 84 deletions.
diff --git a/packages/teleport/src/Player/DesktopPlayer.tsx b/packages/teleport/src/Player/DesktopPlayer.tsx
@@ -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';
@@ -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';
+
   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>
   );
 };
 
@@ -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,
@@ -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();
+      };
+    }
+  }, [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,
   };
 };
 
@@ -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;
+`;
diff --git a/packages/teleport/src/Player/ProgressBar/ProgressBarDesktop.tsx b/packages/teleport/src/Player/ProgressBar/ProgressBarDesktop.tsx
@@ -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';
 
@@ -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 (

diff --git a/packages/teleport/src/lib/tdp/client.ts b/packages/teleport/src/lib/tdp/client.ts
@@ -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();

diff --git a/packages/teleport/src/lib/tdp/playerClient.ts b/packages/teleport/src/lib/tdp/playerClient.ts
@@ -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 {
@@ -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.