Feature/tcp serversocket #4 - #10
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe pull request introduces a socket server component that listens on port 3000, accepting client connections and handling each with a virtual thread. The App class is modified to delegate its startup logic to the new SocketServer instead of executing a direct print statement. Changes
Sequence DiagramsequenceDiagram
participant App
participant SocketServer
participant ServerSocket
participant Client
participant VirtualThread
App->>SocketServer: main()
SocketServer->>ServerSocket: new ServerSocket(3000, 64)
SocketServer->>SocketServer: print startup message
loop Accept Connections
SocketServer->>ServerSocket: accept()
ServerSocket->>Client: establish connection
Client-->>SocketServer: Socket
SocketServer->>VirtualThread: spawn thread
VirtualThread->>SocketServer: handleClient(socket)
SocketServer->>VirtualThread: close socket
VirtualThread-->>SocketServer: complete
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/org/example/SocketServer.java`:
- Around line 9-12: The main method currently hardcodes the listen port (int
port = 3000) which must be made configurable; change the port initialization in
main to read a system property or environment variable with a default fallback
(e.g., use System.getProperty("port") or System.getenv("PORT") and parse to int,
falling back to 3000 if missing/invalid) before creating the ServerSocket, and
validate/handle NumberFormatException so ServerSocket(serverSocket = new
ServerSocket(port, 64)) uses the resolved configurable port.
🧹 Nitpick comments (1)
src/main/java/org/example/SocketServer.java (1)
26-31: Use try-with-resources to guarantee socket closure.This makes the closure resilient if handler logic grows beyond a simple close.
♻️ Suggested change
static void handleClient(Socket socket) { - try { - socket.close(); + try (Socket ignored = socket) { } catch (IOException e) { e.printStackTrace(); } }
There was a problem hiding this comment.
Good start, just 2 thoughts here from my end:
1: Instead of a main method should we maybe make it more descriptive like "createSocket" or something? As we also already have main function inside App.java?
Or what do you think?
2: Currently we are not doing anything with the client socket in handleClient, for example creating a buffer and reading the input stream of the client request. Is that planned for another issue and is this just supposed to be a base?
|
I realized that for my 2nd point its not relevant as you mentioned this will be done in a separate issue! |
|
Absolutely, it's better to give the method a more descriptive name that reflects its purpose. Since this is the actual entry point for the socket server, we would call it startServer or createSocketServer? That makes it clearer for the next person reading the code. (I called it main to be extra clear to whoever's reading the code that this is where we start xD) |
Implemented a blocking TCP server in SocketServer class.
main() method starts the server and listens on a configurable port.
Server blocks on ServerSocket.accept() while waiting for incoming connections.
handleClient(Socket socket) method is a stub (currently just closes the socket).
This prepares the codebase for next issue "Parser for HTTP-request line",
where the next step is to implement logic inside handleClient to parse the
HTTP method, path, and version from client requests.
Closes #4