Improve LAN IP Detection and User Selection for WebSocket Server#35
Improve LAN IP Detection and User Selection for WebSocket Server#35DeveloperAmrit wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughServer IP detection now returns multiple LAN IPs and selects a preferred one; WebSocket onboarding and get-ip messages include the ips array and chosen serverIp. Client settings stores detected IPs and displays clickable LAN IP buttons beneath the IP input for quick selection. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Server
participant WS as WebSocket
participant Client as Client UI
participant User as User
Server->>Server: getLocalIps() -> [ips]
Server->>Server: select BEST_IP (192.168 > 10 > first)
Server->>WS: onboarding msg { ips: [...], serverIp: BEST_IP }
WS->>Client: deliver onboarding payload
Client->>Client: set availableIps state
Client->>Client: show "Detected LAN IPs" buttons
User->>Client: click IP button
Client->>Client: populate IP input with chosen IP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip We've launched Issue Planner and it is currently in beta. Please try it out and share your feedback on Discord! 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/routes/settings.tsx`:
- Around line 111-115: The button element that calls setIp(addr) should include
an explicit type to avoid accidental form submissions; update the JSX button
(the one with key={addr} and onClick={() => setIp(addr)}) to add type="button"
so it doesn't default to type="submit".
- Around line 64-68: The guard prevents applying the auto-detected IP because it
only sets IP when localStorage.getItem('rein_ip') is falsy; instead, read the
stored value into a variable and only skip overwriting when the user has
explicitly chosen a non-default LAN IP. Update the condition around
setIp(data.ip) to something like: const stored =
localStorage.getItem('rein_ip'); if (!stored || stored === '' || stored ===
'localhost' || stored === '127.0.0.1') { setIp(data.ip); } — reference the use
of localStorage.getItem('rein_ip') and the setIp(data.ip) call when making the
change.
In `@src/server/websocket.ts`:
- Around line 26-29: Update the IP selection heuristic used to compute BEST_IP
so it also prefers RFC1918 172.16.0.0/12 addresses before falling back to
10.x.x.x; specifically, change the LAN_IPS checks (the expression that sets
BEST_IP) to test for 192.168.* first, then detect 172.16.0.0/12 addresses (e.g.,
by checking the first octet is "172" and the second octet is between 16 and 31),
then 10.*, and finally LAN_IPS[0]; keep the order of preference and existing
symbol names (BEST_IP, LAN_IPS).
🧹 Nitpick comments (2)
src/server/websocket.ts (1)
48-48: Inconsistent property naming between message types.The
connectedmessage usesserverIpwhile theserver-ipresponse usesipfor the same BEST_IP value. This forces the client to handle two different property names for the same data.Consider aligning on a single name (e.g.,
ipin both, orserverIpin both) to reduce client-side confusion.Also applies to: 56-56
src/routes/settings.tsx (1)
80-82: WebSocket cleanup doesn't handleCONNECTINGstate.If the component unmounts while the socket is still connecting, it won't be closed (readyState is
CONNECTING, notOPEN). This can cause theonmessagehandler to fire after unmount, triggering state updates on an unmounted component.Proposed fix
return () => { - if (socket.readyState === WebSocket.OPEN) socket.close(); + if (socket.readyState === WebSocket.OPEN || socket.readyState === WebSocket.CONNECTING) { + socket.close(); + } }
| <button | ||
| key={addr} | ||
| className="btn btn-xs btn-outline" | ||
| onClick={() => setIp(addr)} | ||
| > |
There was a problem hiding this comment.
Add explicit type="button" to prevent unintended form submission.
Per the Biome lint finding (useButtonType): buttons default to type="submit". While there's no enclosing <form> currently, adding the explicit type is a defensive best practice.
Proposed fix
<button
key={addr}
+ type="button"
className="btn btn-xs btn-outline"
onClick={() => setIp(addr)}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| key={addr} | |
| className="btn btn-xs btn-outline" | |
| onClick={() => setIp(addr)} | |
| > | |
| <button | |
| key={addr} | |
| type="button" | |
| className="btn btn-xs btn-outline" | |
| onClick={() => setIp(addr)} | |
| > |
🧰 Tools
🪛 Biome (2.3.13)
[error] 111-115: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In `@src/routes/settings.tsx` around lines 111 - 115, The button element that
calls setIp(addr) should include an explicit type to avoid accidental form
submissions; update the JSX button (the one with key={addr} and onClick={() =>
setIp(addr)}) to add type="button" so it doesn't default to type="submit".
| // Heuristic: Prefer 192.168.x.x, then 10.x.x.x, then first available | ||
| const BEST_IP = LAN_IPS.find(ip => ip.startsWith('192.168.')) | ||
| || LAN_IPS.find(ip => ip.startsWith('10.')) | ||
| || LAN_IPS[0]; |
There was a problem hiding this comment.
Missing 172.16.0.0/12 private subnet in heuristic.
The RFC 1918 private range 172.16.0.0 – 172.31.255.255 is not considered. This is common in corporate/VPN environments and Docker default bridges.
Proposed fix
const BEST_IP = LAN_IPS.find(ip => ip.startsWith('192.168.'))
|| LAN_IPS.find(ip => ip.startsWith('10.'))
+ || LAN_IPS.find(ip => {
+ const second = parseInt(ip.split('.')[1], 10);
+ return ip.startsWith('172.') && second >= 16 && second <= 31;
+ })
|| LAN_IPS[0];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Heuristic: Prefer 192.168.x.x, then 10.x.x.x, then first available | |
| const BEST_IP = LAN_IPS.find(ip => ip.startsWith('192.168.')) | |
| || LAN_IPS.find(ip => ip.startsWith('10.')) | |
| || LAN_IPS[0]; | |
| // Heuristic: Prefer 192.168.x.x, then 10.x.x.x, then first available | |
| const BEST_IP = LAN_IPS.find(ip => ip.startsWith('192.168.')) | |
| || LAN_IPS.find(ip => ip.startsWith('10.')) | |
| || LAN_IPS.find(ip => { | |
| const second = parseInt(ip.split('.')[1], 10); | |
| return ip.startsWith('172.') && second >= 16 && second <= 31; | |
| }) | |
| || LAN_IPS[0]; |
🤖 Prompt for AI Agents
In `@src/server/websocket.ts` around lines 26 - 29, Update the IP selection
heuristic used to compute BEST_IP so it also prefers RFC1918 172.16.0.0/12
addresses before falling back to 10.x.x.x; specifically, change the LAN_IPS
checks (the expression that sets BEST_IP) to test for 192.168.* first, then
detect 172.16.0.0/12 addresses (e.g., by checking the first octet is "172" and
the second octet is between 16 and 31), then 10.*, and finally LAN_IPS[0]; keep
the order of preference and existing symbol names (BEST_IP, LAN_IPS).
|
@DeveloperAmrit |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/settings.tsx (1)
82-84:⚠️ Potential issue | 🟡 MinorWebSocket cleanup doesn't handle
CONNECTINGstate.If the component unmounts while the socket is still connecting (state
0),socket.close()is never called and the connection may complete after unmount, potentially firingonmessageon a stale component.Proposed fix
return () => { - if (socket.readyState === WebSocket.OPEN) socket.close(); + if (socket.readyState === WebSocket.OPEN || socket.readyState === WebSocket.CONNECTING) { + socket.close(); + } }
2 participants
Fixes #27
Summary
This PR addresses a bug in the getLocalIp() function that caused the application to select incorrect IP addresses (e.g., Docker or VPN interfaces) on developer machines. The changes improve the LAN IP detection logic and enhance the frontend UI to allow users to select the correct IP address if the auto-detection is inaccurate.
Changes Made
Testing
Video
Screen.Recording.2026-02-08.at.9.05.36.AM.mov
Summary by CodeRabbit