Jssezhng/bug fixes

README.md

@@ -2,6 +2,8 @@
 
 Here is a demo of how the watch party works: https://www.loom.com/share/c6bb194d55b749919a308e7c5fd89521
 
+Here is the customer's bug report: https://www.loom.com/share/b5dc95868b8840e1bbfe89523c53ff59
+
 ## System Overview
 
 This is a simple implementation of a collaborative Youtube Watch Party, backed by a socket.io backend that enables users to send messages to each other via our server in real time.
@@ -12,18 +14,21 @@ Every Youtube player has a custom slider with a play / pause button. Whenever th
 
 ## Instructions
 
-1. There is a significant bug in this implementation of a collaborative watch party. Find the bug.
+1. There is a significant bug in this implementation of a collaborative watch party. This bug can be replicated with just two people in the session. Find the bug.
 2. Figure out the best strategy to fix the bug and implement the fix!
 
 ## Recommended Reading Order
 
 `server/src/app.ts` - has all the server logic that implements the watch party collaboration functionality
 `src/VideoPlayer.ts` - makes API calls to the server and listens to socket events to control the users progress through the video
 
+If you'd prefer to work with a backend written entirely in Python (same frontend code) - check out the `nikhil/python-version` branch instead.
+
 ## How to Run Locally
 
 - Make sure nodeJS (I am using v19.7.0) and npm is installed on your local machine
 - Open a terminal and run install dependencies
   `$ npm run deps`
 - In your terminal at project root, start the server and the client simultaneously
   `$ npm run start`
+- Unfortunately the node backend will not restart automatically when you make changes to the code, so you will need to restart the server manually if you want to see your changes take effect.

server/src/app.ts

@@ -23,55 +23,39 @@ const io = new Server(server, {
 app.use(cors());
 
 type VideoControlEvent = {
-  type: "PLAY" | "PAUSE" | "END";
+  type: "PLAY" | "PAUSE";
   progress: number;
-  createdAt: number;
 };
 
 type SessionProps = {
   videoUrl: string;
   users: Set<string>;
-  lastVideoControlEvent?: VideoControlEvent;
+  events: VideoControlEvent[];
 };
 
 const sessionData = new Map<string, SessionProps>();
-const sessions = new Map();
 
 // Creates a new session with a URL
 app.post("/session", async (req, res) => {
   const { sessionId, url } = req.body;
   sessionData.set(sessionId, {
     videoUrl: url,
     users: new Set(),
+    events: [],
   });
   res.sendStatus(201);
 });
 
-// Gets the last video control event from a session (for joining late)
-app.get("/session/:sessionId/lastVideoEvent", async (req, res) => {
+// Gets a session's data
+app.get("/session/:sessionId", (req, res) => {
   const { sessionId } = req.params;
+  const session = sessionData.get(sessionId);
 
-  const lastEvent = sessionData.get(sessionId)?.lastVideoControlEvent;
+  if (!session) {
+    return res.status(404).json({ error: "Session not found" });
+  }
 
-  // If there is no recent event, send undefined so the frontend knows to play from start
-  res.send(lastEvent).status(200);
-});
-
-// Ends a live session
-app.post("/session/:sessionId/end", async (req, res) => {
-  const { sessionId } = req.params;
-  const currentSession = sessionData.get(sessionId);
-
-  if (!currentSession) return;
-
-  // Write an "END" event so the video doesn't keep playing when last user leaves the page
-  currentSession.lastVideoControlEvent = {
-    type: "END",
-    progress: 0,
-    createdAt: Date.now(),
-  };
-
-  res.sendStatus(200);
+  res.json({ videoUrl: session.videoUrl });
 });
 
 io.on("connection", (socket) => {
@@ -86,18 +70,20 @@ io.on("connection", (socket) => {
     currentSession.users.add(socket.id);
     socket.join(sessionId);
 
-    // Broadcast to all users in the session that a new user has joined
-    io.to(sessionId).emit("userJoined", socket.id, [...currentSession.users]);
+    const lastEvent = currentSession.events[currentSession.events.length - 1];
 
     // Return the session's data to the user that just joined
-    callback({
+    const responseData = {
       videoUrl: currentSession.videoUrl,
-      users: [...currentSession.users],
-    });
+      progress: lastEvent?.progress ?? 0,
+      isPlaying: lastEvent?.type === "PLAY" ?? false,
+    };
+    console.log(`Sending session state to user ${socket.id}:`, responseData);
+    callback(responseData);
   });
 
   // Handle video control events from the client
-  socket.on("videoControl", (sessionId, videoControl) => {
+  socket.on("videoControl", (sessionId, videoControl: VideoControlEvent) => {
     console.log(
       `Received video control from client ${socket.id} in session ${sessionId}:`,
       videoControl
@@ -107,12 +93,8 @@ io.on("connection", (socket) => {
 
     if (!currentSession) return;
 
-    // Store last event (needed for late to the party)
-    currentSession.lastVideoControlEvent = {
-      type: videoControl.type,
-      progress: videoControl.progress,
-      createdAt: Date.now(),
-    };
+    // Store the event in the session
+    currentSession.events.push(videoControl);
 
     // Broadcast the video control event to all watchers in the session except the sender
     socket.to(sessionId).emit("videoControl", socket.id, videoControl);
@@ -126,7 +108,6 @@ io.on("connection", (socket) => {
       const { users } = sessionState;
       if (users.delete(socket.id)) {
         socket.leave(sessionId);
-        io.to(sessionId).emit("userLeft", socket.id, [...users]);
       }
     }
   });

src/Api.tsx

@@ -35,3 +35,14 @@ export const createSession = async (
     throw error;
   }
 };
+
+export const getSession = async (
+  sessionId: string
+): Promise<{ data: { videoUrl: string } }> => {
+  try {
+    return await apiClient.get(`/session/${sessionId}`);
+  } catch (error) {
+    console.error("Error creating video:", error);
+    throw error;
+  }
+};

src/components/VideoPlayer.tsx

@@ -1,61 +1,81 @@
 import { Box, Button } from "@mui/material";
-import React, { useRef, useState } from "react";
+import React, { useRef, useState, useEffect } from "react";
 import ReactPlayer from "react-player";
 import { Socket } from "socket.io-client";
 
-const MIN_VIDEO_PROGRESS = 0;
-const MAX_VIDEO_PROGRESS = 0.999999;
-
-interface IVideoPlayerProps {
-  sessionId: string;
+interface VideoPlayerProps {
   socket: Socket;
+  sessionId: string;
+  url: string;
 }
 
 interface IVideoControlProps {
-  type: "PLAY" | "PAUSE" | "END";
+  type: "PLAY" | "PAUSE";
   progress: number;
 }
 
 interface JoinSessionResponse {
   videoUrl: string;
-  users: String[];
+  progress: number;
+  isPlaying: boolean;
 }
 
-const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
-  const [url, setUrl] = useState<string | null>(null);
-  const [hasJoined, setHasJoined] = useState(false);
+const VideoPlayer: React.FC<VideoPlayerProps> = ({
+  socket,
+  sessionId,
+  url,
+}) => {
   const [isReady, setIsReady] = useState(false);
+  const [hasJoined, setHasJoined] = useState(false);
   const [playingVideo, setPlayingVideo] = useState(false);
-  const [seeking, setSeeking] = useState(false);
   const [played, setPlayed] = useState(0);
 
   const player = useRef<ReactPlayer>(null);
 
-  React.useEffect(() => {
-    // join session on init
+  console.log("VideoPlayer: ", url);
 
-    socket.emit("joinSession", sessionId, (response: JoinSessionResponse) => {
-      console.log("Response after joining session: ", response);
-      setUrl(response.videoUrl);
-    });
-  }, []);
+  useEffect(() => {
+    if (playingVideo && !!url && !!player.current?.getInternalPlayer() && !hasJoined) {
+      handleWatchStart();
+    }
+  }, [played, playingVideo, url, player.current?.getInternalPlayer()]); // Triggers when user is joining late and video is already playing. Waits for player to initialize, otherwise plays on a black screen.
 
-  const handleWatchStart = async () => {
-    // register to listen to video control events from socket
+  useEffect(() => {
     socket.on(
       "videoControl",
-      (senderId: string, control: IVideoControlProps) => {
-        if (control.type === "PLAY") {
-          playVideoAtProgress(control.progress);
-          setPlayed(control.progress);
-        } else if (control.type === "PAUSE") {
-          pauseVideoAtProgress(control.progress);
-          setPlayed(control.progress);
+      (userId: string, videoControl: IVideoControlProps) => {
+        console.log("Received video control event: ", userId, videoControl);
+        if (videoControl.type === "PLAY") {
+          playVideoAtProgress(videoControl.progress);
+        } else if (videoControl.type === "PAUSE") {
+          pauseVideoAtProgress(videoControl.progress);
         }
+        setPlayed(videoControl.progress);
       }
     );
 
-    setHasJoined(true);
+    return () => {
+      socket.off("videoControl");
+    };
+  }, [socket]);
+
+  useEffect(() => {
+    console.log("Played: ", played);
+  }, [played]);
+
+  const handleWatchStart = async () => {
+    socket.emit("joinSession", sessionId, (response: JoinSessionResponse) => {
+      setHasJoined(true);
+      console.log("Received join session response: ", response);
+      if (response.progress > 0) {
+        seekToVideo(response.progress);
+        if (response.isPlaying) {
+          playVideo();
+        }
+        setPlayed(response.progress);
+        setPlayingVideo(response.isPlaying);
+      }
+    });
   };
 
   function playVideo() {
@@ -67,7 +87,7 @@ const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
   }
 
   function seekToVideo(progress: number) {
-    player.current?.seekTo(progress);
+    player.current?.seekTo(progress, "seconds");
   }
 
   function playVideoAtProgress(progress: number) {
@@ -86,33 +106,36 @@ const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
     setIsReady(true);
   };
 
+  const handleEnded = () => {
+    socket!.emit("videoControl", sessionId, {
+      type: "PAUSE",
+      progress: 0,
+    });
+    pauseVideoAtProgress(0);
+  };
+
   const handlePlayPause = () => {
     if (playingVideo) {
       socket!.emit("videoControl", sessionId, {
         type: "PAUSE",
-        progress: played,
+        progress: player.current?.getCurrentTime(),
       });
       pauseVideo();
     } else {
       socket!.emit("videoControl", sessionId, {
         type: "PLAY",
-        progress: played,
+        progress: player.current?.getCurrentTime(),
       });
       playVideo();
     }
     setPlayingVideo(!playingVideo);
   };
 
-  const handleSeekMouseDown = () => {
-    setSeeking(true);
-  };
-
   const handleSeekChange = (e: any) => {
     setPlayed(parseFloat(e.target.value));
   };
 
   const handleSeekMouseUp = (e: any) => {
-    setSeeking(false);
     const progress = parseFloat(e.target.value);
     socket!.emit("videoControl", sessionId, {
       type: "PLAY",
@@ -121,17 +144,6 @@ const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
     playVideoAtProgress(progress);
   };
 
-  const handleProgress = (state: {
-    played: number;
-    playedSeconds: number;
-    loaded: number;
-    loadedSeconds: number;
-  }) => {
-    if (!seeking) {
-      setPlayed(state.played === 1 ? MAX_VIDEO_PROGRESS : state.played);
-    }
-  };
-
   return (
     <Box
       width="100%"
@@ -151,10 +163,10 @@ const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
           <ReactPlayer
             ref={player}
             url={url}
-            playing={false}
+            playing={playingVideo}
             controls={false}
             onReady={handleReady}
-            onProgress={handleProgress}
+            onEnded={handleEnded}
             width="100%"
             height="100%"
             style={{ pointerEvents: "none" }}
@@ -169,18 +181,18 @@ const VideoPlayer: React.FC<IVideoPlayerProps> = ({ socket, sessionId }) => {
             <input
               style={{ display: "block", width: "100%" }}
               type="range"
-              min={MIN_VIDEO_PROGRESS}
-              max={MAX_VIDEO_PROGRESS}
+              min={0}
+              max={player.current?.getDuration() ?? 1}
               step="any"
               value={played}
-              onMouseDown={() => handleSeekMouseDown()}
               onChange={(e) => handleSeekChange(e)}
               onMouseUp={(e) => handleSeekMouseUp(e)}
             />
           </div>
         </Box>
       )}
-      {!hasJoined && isReady && (
+
+      {!hasJoined && isReady && !playingVideo && (
         // Youtube doesn't allow autoplay unless you've interacted with the page already
         // So we make the user click "Join Session" button and then start playing the video immediately after
         <Button