add solution by GabrielSpelchuk · Pull Request #198 · mate-academy/node_chat

GabrielSpelchuk · 2026-04-15T10:51:01Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are a couple of critical issues that currently block the application from meeting the task requirements.

The server cannot start due to an incorrect require path: in src/index.js (near the top, around the line where you import the WebSocket server) you use require('./src/websocket'), but since index.js is already inside src, this resolves to src/src/websocket.js and will throw MODULE_NOT_FOUND; this should be changed to something like require('./websocket') (and the same pattern appears in src/utils.js and src/store.js) so Node can actually load the module.
The project is missing the entire client-side implementation required by the task: there is no UI that lets the user type a username, no use of localStorage to persist it, and no client code that sends JOIN_ROOM, CREATE_ROOM, NEW_MESSAGE, etc. with username to the server, so checklist items about having a full client–server chat app and persisting the username [CHECKLIST ITEM #1, #2, #3, #4, #15] are not satisfied.

There are also a couple of non-blocking but important improvements to consider once the above are fixed: when deleting a room in src/handlers.js, moved users are not sent the ROOM_HISTORY for the general room (so they don't see previous messages immediately) and handleMessage in src/websocket.js is not wrapped in a try/catch, so malformed JSON from a client could crash the server. On the positive side, your server-side room and message handling is well structured (message author/time/text, room CRUD, and ROOM_HISTORY on JOIN_ROOM are all implemented correctly), so you mainly need to fix the module paths and add the required browser client to complete the assignment. Once you address these issues, your solution should be in a strong position to fully satisfy the task requirements.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-04-15T13:32:49Z

@@ -0,0 +1,28 @@
+const WebSocket = require('ws');
+const { rooms } = require('./store');
+


This require path is incorrect given file location. src/index.js is requiring ./src/websocket, which resolves to src/src/websocket.js and will fail. Change to require the local module: ./websocket (or the correct relative path) so the server can start.

mateacademy-ai-mentor · 2026-04-15T13:32:49Z

+  }));
+  const message = JSON.stringify({ type: 'ROOMS_LIST', rooms: roomList });
+


The server expects the client to send username in the JOIN_ROOM message (ws.username = data.username). The task requires the user to be able to type a username and save it to localStorage (checklist items #3 and #15). There is no client implementation here — add a client UI that reads/saves the username to localStorage and sends it to the server on join.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

+/* eslint-disable no-console */
+const http = require('http');
+const { setupWebSocket } = require('./src/websocket');
+


The repository contains only server-side code (handlers, websocket, utils, store). The task requires a client as well (UI to type username, send it to server, and save username in localStorage). Without client-side code, checklist items #1, #2, #3 and #15 are not fulfilled — add a client that sends JOIN_ROOM/CREATE_ROOM/etc. and saves the username in localStorage.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

-'use strict';
+/* eslint-disable no-console */
+const http = require('http');
+const { setupWebSocket } = require('./src/websocket');


Incorrect require path: this file is already in src, so requiring './src/websocket' will try to load src/src/websocket.js and fail. Change to require('./websocket') or adjust the path to the actual websocket module location.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

@@ -0,0 +1,78 @@
+const { rooms } = require('./store');
+const { broadcastRooms, broadcastToRoom } = require('./utils');
+


In src/index.js you require './src/websocket' from a file that is already in src; this will resolve to src/src/websocket.js and fail. Change the path to './websocket' (or correct relative path) so Node can load the websocket module.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

+        wss.clients.forEach((client) => {
+          if (client.roomId === data.roomId) {
+            client.roomId = 'general';
+            client.send(JSON.stringify({ type: 'ROOM_DELETED' }));
+          }
+        });


When deleting a room you set affected clients' roomId to 'general' and send a ROOM_DELETED event, but you do not send the ROOM_HISTORY for the 'general' room to those clients. The requirement states new users (and users moved to a room) should see previous messages in the room (checklist item #12). Consider sending the 'ROOM_HISTORY' with rooms['general'].messages to moved clients.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

+      if (!rooms[data.roomId]) {
+        rooms[data.roomId] = {
+          id: data.roomId,
+          name: data.roomId,


When creating a missing room inside the JOIN_ROOM handler you set the room name to the roomId (line 15). For consistency with CREATE_ROOM (which uses data.roomName), consider using a human-friendly name if available or a consistent naming strategy.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

@@ -0,0 +1,5 @@
+const rooms = {
+  general: { id: 'general', name: 'General', messages: [] },
+};


This require is incorrect given this file is already in src. Requiring './src/websocket' will try to load src/src/websocket and fail with MODULE_NOT_FOUND. Change to require('./websocket') (or adjust path relative to this file).

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

+  wss.on('connection', (ws) => {
+    ws.isAlive = true;
+
+    ws.on('pong', () => {
+      ws.isAlive = true;
+    });
+
+    ws.send(
+      JSON.stringify({
+        type: 'ROOMS_LIST',
+        rooms: Object.values(rooms).map((r) => ({ id: r.id, name: r.name })),
+      }),
+    );


On new connections you send the rooms list but do not assign a default room or send its history. Consider setting a default ws.roomId = 'general' and sending a ROOM_HISTORY for that room so a newly connected user sees previous messages without requiring an explicit JOIN_ROOM.

mateacademy-ai-mentor · 2026-04-15T13:32:50Z

+
+    ws.on('message', (message) => handleMessage(ws, wss, message));


Calling handleMessage directly can allow exceptions from JSON.parse (in the handlers) to bubble up and crash the server. Wrap this call in a try/catch (or validate input) and handle parse errors gracefully so a malformed client message doesn't bring down the server.

mateacademy-ai-mentor

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
_{[CODE: NCM]}

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still critical blockers that prevent the project from meeting the task requirements and from running reliably.

The most severe issue is that the server will not start: in src/index.js (and similarly in src/store.js, src/handlers.js, and src/utils.js), the import require('./src/websocket') is incorrect given that index.js already lives in src/; this resolves to src/src/websocket.js and will throw MODULE_NOT_FOUND. You need to change these to a correct relative path such as require('./websocket') at the top of each affected file so the WebSocket server module can actually be loaded. The other major blocker is that there is no client-side implementation at all: the task explicitly requires an HTML/JS client where the user can type a username, save it to localStorage, and send it to the server, as well as a UI to create/rename/join/delete rooms and display room history for new users; you must add these client files and wire them to the existing WebSocket API.

On the server side, there are a few functional gaps that should be addressed while you are updating things: in src/handlers.js the NEW_MESSAGE handler uses ws.username without checking it, so you should validate that ws.username and ws.roomId are set (after a successful JOIN) before accepting and broadcasting a message, otherwise you can end up with messages that have no author. In JOIN_ROOM (inside your utilities/handlers), when a room is auto-created because it doesn’t exist yet, you should also call your broadcastRooms(wss) helper so other connected clients see this new room in their room list. Finally, in src/websocket.js, consider wrapping the handleMessage call in a try/catch and setting a default room like 'general' with an initial ROOM_HISTORY send on connection, so malformed JSON from a client does not crash the server and new users immediately see existing messages.

Once you fix the broken require paths, add the required client-side UI and localStorage behavior, and tighten up the username/room handling, your existing room and history logic will form a solid foundation for the assignment. Keep going—your server logic is close, and with these changes you can fully satisfy the specification.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-04-15T13:33:59Z

@@ -0,0 +1,5 @@
+const rooms = {
+  general: { id: 'general', name: 'General', messages: [] },
+};


This require is incorrect given this file is already in src. Requiring './src/websocket' will try to load src/src/websocket and fail with MODULE_NOT_FOUND. Change to require('./websocket') (or adjust path relative to this file).

mateacademy-ai-mentor · 2026-04-15T13:33:59Z

+  wss.on('connection', (ws) => {
+    ws.isAlive = true;
+
+    ws.on('pong', () => {
+      ws.isAlive = true;
+    });
+
+    ws.send(
+      JSON.stringify({
+        type: 'ROOMS_LIST',
+        rooms: Object.values(rooms).map((r) => ({ id: r.id, name: r.name })),
+      }),
+    );


On new connections you send the rooms list but do not assign a default room or send its history. Consider setting a default ws.roomId = 'general' and sending a ROOM_HISTORY for that room so a newly connected user sees previous messages without requiring an explicit JOIN_ROOM.

mateacademy-ai-mentor · 2026-04-15T13:33:59Z

+
+    ws.on('message', (message) => handleMessage(ws, wss, message));


Calling handleMessage directly can allow exceptions from JSON.parse (in the handlers) to bubble up and crash the server. Wrap this call in a try/catch (or validate input) and handle parse errors gracefully so a malformed client message doesn't bring down the server.

mateacademy-ai-mentor · 2026-04-15T13:33:59Z

-'use strict';
+/* eslint-disable no-console */
+const http = require('http');
+const { setupWebSocket } = require('./src/websocket');


From src/index.js this require uses './src/websocket' while the file itself is already in src/. That resolves to src/src/websocket which likely doesn't exist. Use a correct relative path such as require('./websocket') (or adjust the path to where the websocket module actually lives).

mateacademy-ai-mentor · 2026-04-15T13:33:59Z

+setupWebSocket(server);
+
+const PORT = 8080;
+


The server stores ws.username from the incoming JOIN_ROOM message here, which is fine server-side. The task requires the username to be saved in localStorage on the client — ensure you implement client-side code that saves the username to localStorage and sends it with the JOIN_ROOM message.

mateacademy-ai-mentor · 2026-04-15T13:34:00Z

+        wss.clients.forEach((client) => {
+          if (client.roomId === data.roomId) {
+            client.roomId = 'general';
+            client.send(JSON.stringify({ type: 'ROOM_DELETED' }));


When a room is deleted you set moving clients' roomId to 'general' and send a generic ROOM_DELETED message. Consider including the new room id and/or immediately sending the ROOM_HISTORY for 'general' so clients have enough info to update UI and show previous messages as required.

mateacademy-ai-mentor · 2026-04-15T13:34:00Z

+const { rooms } = require('./store');
+const { broadcastRooms, broadcastToRoom } = require('./utils');
+
+function handleMessage(ws, wss, messageAsString) {


The server-side implements room operations and history broadcasting, which is good, but the task requires that the username be saved in localStorage (client-side). Make sure the client saves the username in localStorage and re-sends it when reconnecting/joining a room so the server can set ws.username correctly.

mateacademy-ai-mentor · 2026-04-15T13:34:00Z

@@ -0,0 +1,28 @@
+const WebSocket = require('ws');


The task requires both client and server parts and that the username is saved in localStorage and sent to the server. I don't see any client-side files here implementing the UI, localStorage usage, or sending username/join/create room actions. You must add the client implementation (HTML/JS) so users can type a username, save it to localStorage, and perform room actions.

mateacademy-ai-mentor · 2026-04-15T13:34:00Z

@@ -0,0 +1,28 @@
+const WebSocket = require('ws');
+const { rooms } = require('./store');
+


This require path is incorrect for a file located at src/index.js: requiring './src/websocket' will try to load src/src/websocket.js and likely fail. Use the correct relative path (e.g. require('./websocket')) or adjust according to your project layout so Node can find the websocket module.

mateacademy-ai-mentor · 2026-04-15T13:34:00Z

+    if (client.readyState === WebSocket.OPEN) {
+      client.send(message);
+    }
+  });
+}
+
+function broadcastToRoom(wss, roomId, messageObj) {


When JOIN_ROOM creates a new room (if it doesn't exist) the code doesn't broadcast the updated rooms list to other clients. That means other connected users won't see rooms that are implicitly created by JOIN_ROOM. Consider calling broadcastRooms(wss) after creating the room (or ensure rooms are only created via CREATE_ROOM which already broadcasts).

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still two critical checklist-required behaviors missing in your WebSocket handlers, which can break core chat functionality.

Missing username validation on NEW_MESSAGE: In src/handlers.js, in the NEW_MESSAGE case, you currently accept and broadcast messages as long as ws.roomId is set and the room exists, but you do not check that ws.username is defined. This can result in messages with an author of undefined, which violates the requirement that every message must have a valid author. Please update the guard condition so that it requires both ws.roomId and ws.username to be truthy before constructing and broadcasting a message (for example, if (!ws.roomId || !ws.username || !rooms[ws.roomId]) return;).
Not broadcasting rooms on JOIN auto-create: In the JOIN_ROOM handler in src/handlers.js, inside the block where you auto-create a new room with something like rooms[data.roomId] = { ... } when it doesn’t exist, you don’t call broadcastRooms(wss). The requirements specify that when a room is auto-created on join, all other connected clients must receive an updated room list. To fix this, immediately after creating rooms[data.roomId], call broadcastRooms(wss) so that the new room appears in other clients’ room lists.

Your WebSocket setup, room utilities (broadcastRooms, broadcastToRoom), and general room/message flow look solid, so you are very close. Once you add the ws.username validation in NEW_MESSAGE and broadcast the room list on auto-create in JOIN_ROOM, your server will meet the functional requirements for this task.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-04-15T13:54:43Z

+function broadcastToRoom(wss, roomId, messageObj) {
+  const message = JSON.stringify(messageObj);
+
+  wss.clients.forEach((client) => {
+    if (client.readyState === WebSocket.OPEN && client.roomId === roomId) {
+      client.send(message);
+    }


When JOIN_ROOM auto-creates a room (inside this block) you also need to notify other connected clients about the new room. Call broadcastRooms(wss) after creating rooms[data.roomId] so the new room appears in other clients' room lists (checklist requires broadcasting rooms on auto-create).

mateacademy-ai-mentor

Review Summary

I am approving your solution as an educational override so you can continue progressing while you address a small remaining issue.
Your server-side WebSocket logic is in good shape: messages consistently include author, time, and text, NEW_MESSAGE correctly requires both ws.username and ws.roomId, and you’ve implemented the full room lifecycle (create/rename/join/delete) plus ROOM_HISTORY on join.
The main functional issue to fix is in src/handlers.js: when you auto-create a room in the JOIN_ROOM handler, you currently call broadcastRooms(wss) only after sending ROOM_HISTORY (around line 41), which delays informing other connected clients about the new room.
To align with the requirement that other clients see the updated room list immediately, move the broadcastRooms(wss) call into the creation block itself (right after you assign rooms[ws.roomId] = { ... }), and you can then drop the extra isNewRoom flag.
As a minor robustness improvement, consider adding simple validation for incoming payloads (e.g. ensuring data.roomName exists on CREATE_ROOM and data.newName exists on RENAME_ROOM) so you don’t end up with empty or undefined room names.
Overall, you’ve implemented the core server behavior well, and with this small adjustment to when you broadcast the rooms list your backend will be even more solid—nice work getting this far.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-04-15T14:05:32Z

+      let isNewRoom = false;
+
+      if (!rooms[ws.roomId]) {
+        rooms[ws.roomId] = { id: ws.roomId, name: ws.roomId, messages: [] };


When you auto-create a room here, call broadcastRooms(wss) immediately after creating the room so other connected clients receive the updated room list right away (instead of relying on an isNewRoom flag and broadcasting later).

mateacademy-ai-mentor · 2026-04-15T14:05:32Z

+      );
+
+      if (isNewRoom) {
+        broadcastRooms(wss);


Right now you call broadcastRooms after sending ROOM_HISTORY (line 41). That delays notifying other clients. Move the broadcastRooms call into the creation block (immediately after creating rooms[ws.roomId]).

add solution

c2e045e

mateacademy-ai-mentor suggested changes Apr 15, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Apr 15, 2026

View reviewed changes

mateacademy-ai-mentor suggested changes Apr 15, 2026

View reviewed changes

created client application

c88980f

GabrielSpelchuk requested a review from mateacademy-ai-mentor April 15, 2026 13:52

mateacademy-ai-mentor suggested changes Apr 15, 2026

View reviewed changes

refactored code

e6bfc7d

GabrielSpelchuk requested a review from mateacademy-ai-mentor April 15, 2026 14:03

mateacademy-ai-mentor approved these changes Apr 15, 2026

View reviewed changes

		@@ -0,0 +1,28 @@
		const WebSocket = require('ws');
		const { rooms } = require('./store');

		}));
		const message = JSON.stringify({ type: 'ROOMS_LIST', rooms: roomList });

		@@ -0,0 +1,78 @@
		const { rooms } = require('./store');
		const { broadcastRooms, broadcastToRoom } = require('./utils');


		ws.on('message', (message) => handleMessage(ws, wss, message));

Conversation

GabrielSpelchuk commented Apr 15, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants