Fix(gateway): gracefully handle uv_interface_addresses system error i…#2451
Fix(gateway): gracefully handle uv_interface_addresses system error i…#2451Iamkewl wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
…n sandboxed environments ## Description This PR resolves the fatal Gateway crash reported in NVIDIA#2427. When running `openclaw gateway` inside strict containerized environments with default `seccomp` profiles (like OpenShell / Docker sandboxes), the sandbox restricts low-level network enumeration. The `@homebridge/ciao` dependency calls `os.networkInterfaces()`, which throws `uv_interface_addresses returned Unknown system error 1`. Because this was unhandled, it resulted in a promise rejection that instantly killed the Gateway process. ## Changes Made - Added a wrapper to handle restricted network enumeration gracefully. - Implemented a custom `[Sandbox Compatibility]` warning logger to clearly indicate *why* mDNS/discovery failed, rather than throwing cryptic `uv` system errors. - Provided a loopback fallback (`127.0.0.1`) so the Gateway process remains stable, allowing the TUI to connect manually via `--url ws://127.0.0.1:<port>`. Fixes NVIDIA#2427 Signed off: Suryaansh Suryaansh2112@gmail.com
📝 WalkthroughWalkthroughThe module overrides Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoclaw/src/index.ts (1)
29-38: Log the sandbox-compatibility warning only once.If
os.networkInterfaces()is retried repeatedly in a restricted sandbox, this branch will print the same multiline warning every time. That can spam stderr without adding new signal. Gate the warning with a module-level boolean and keep returning the same fallback.Proposed change
import os from 'os'; // Cache the original function const originalNetworkInterfaces = os.networkInterfaces; +let hasWarnedAboutSandboxCompatibility = false; // Use (os as any) to bypass TypeScript's read-only protection (os as any).networkInterfaces = function () { try { // Just call the function directly! No args needed. return originalNetworkInterfaces(); } catch (error: any) { if (error?.message?.includes('uv_interface_addresses') || error?.code === 'EACCES') { - console.warn( - '\n[Sandbox Compatibility] Restricted network access detected. ' + - 'Falling back to loopback (127.0.0.1) to prevent crash.\n' - ); + if (!hasWarnedAboutSandboxCompatibility) { + hasWarnedAboutSandboxCompatibility = true; + console.warn( + '\n[Sandbox Compatibility] Restricted network access detected. ' + + 'Falling back to loopback (127.0.0.1) to prevent crash.\n' + ); + } return { lo: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nemoclaw/src/index.ts` around lines 29 - 38, The sandbox-compatibility warning inside the os.networkInterfaces override is currently printed on every retry; add a module-level boolean (e.g., warnedAboutSandboxCompatibility) initialized false and, inside the catch branch that detects 'uv_interface_addresses' or 'EACCES', check this flag before calling console.warn; if false, call console.warn once and set warnedAboutSandboxCompatibility = true, then continue returning the same loopback fallback from originalNetworkInterfaces's catch path so subsequent calls do not spam stderr. Reference: the overridden os.networkInterfaces function and originalNetworkInterfaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nemoclaw/src/index.ts`:
- Around line 29-38: The sandbox-compatibility warning inside the
os.networkInterfaces override is currently printed on every retry; add a
module-level boolean (e.g., warnedAboutSandboxCompatibility) initialized false
and, inside the catch branch that detects 'uv_interface_addresses' or 'EACCES',
check this flag before calling console.warn; if false, call console.warn once
and set warnedAboutSandboxCompatibility = true, then continue returning the same
loopback fallback from originalNetworkInterfaces's catch path so subsequent
calls do not spam stderr. Reference: the overridden os.networkInterfaces
function and originalNetworkInterfaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e9341e7-0f65-40e6-8ecc-3312de922458
📒 Files selected for processing (1)
nemoclaw/src/index.ts
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
Review: PR #2451
Thanks for investigating #2427 and identifying the root cause (seccomp blocking uv_interface_addresses). The diagnosis is solid. However, the proposed fix has several issues that need to be addressed before this can be merged.
Wrong layer for the fix
The crash originates in @homebridge/ciao, a dependency of OpenClaw (the upstream host process), not NemoClaw. Monkey-patching os.networkInterfaces() from the NemoClaw plugin (nemoclaw/src/index.ts) is fragile because:
- Load order is not guaranteed. If
@homebridge/ciaoinitializes before the NemoClaw plugin registers, the patch arrives too late and the crash still happens. - A plugin should not monkey-patch Node.js builtins. This creates invisible coupling -- any future code or dependency that calls
os.networkInterfaces()silently gets the patched version, which can mask real errors and make debugging harder. - The proper fix belongs upstream (in OpenClaw itself, or in the sandbox entrypoint/Dockerfile so it runs before the gateway starts). If an upstream fix is not feasible short-term, a wrapper in
scripts/nemoclaw-start.shor a--requirepreload script would be a more appropriate location.
ESLint violations (will fail CI)
The plugin enforces "@typescript-eslint/no-explicit-any": "error". This PR uses:
(os as any).networkInterfaces-- triggersno-explicit-anycatch (error: any)-- triggersno-explicit-any
These will cause npx prek run --all-files and npm test to fail. The PR checklist confirms these were not run.
Style and convention issues
- Import specifier: Existing code uses
"node:os"(e.g.,import { homedir } from "node:os"), but this PR uses bare'os'. Should be"node:os". - Quote style: The file uses double quotes throughout. This PR uses single quotes, which will be caught by Prettier.
- Noisy comment:
// Just call the function directly! No args needed.adds no value. - Spam risk:
console.warn('[Sandbox Compatibility] ...')fires on every call that throws, not just once.os.networkInterfaces()can be called many times -- this will flood logs.
Missing tests
No tests were added. At minimum, a unit test should verify:
- The fallback returns a valid loopback interface when
os.networkInterfaces()throws the targeted error. - Non-matching errors are re-thrown.
Summary
The diagnosis of #2427 is correct and valuable. But monkey-patching a Node.js builtin from a plugin to fix a crash in the host application is the wrong approach. The fix should either:
- Go upstream to OpenClaw (where
@homebridge/ciaois a direct dependency), or - Be applied as a Node.js
--requirepreload script in the sandbox entrypoint (before the gateway loads), not in the plugin.
If you would like to pursue option 2 within NemoClaw, happy to help identify the right location (likely scripts/nemoclaw-start.sh or a new preload module).
|
Thank you so much for the detailed review. I see how monkey-patching a Node builtin from a downstream plugin is fragile and a bad architectural practice. Really appreciate the detailed feedback. I also apologize for the ESLint/Prettier violations and the lack of tests. This was a huge learning moment for me overall. I was just about to take you up on Option 2, but I noticed PR #2469 was just opened which looks like it implements the exact preload script approach! Since #2469 is up, would you prefer I close this PR and help with that one in any way, or is there still a piece of Option 2 you'd like me to tackle here? Happy to do whatever is most helpful for the repo. |
2 participants
Summary
This PR resolves a fatal Gateway crash occurring in sandboxed environments (like OpenShell or Docker with default
seccompprofiles). It intercepts a blockeduv_interface_addressessyscall triggered byos.networkInterfaces()and implements a graceful loopback fallback.Related Issue
Fixes #2427
Changes
os.networkInterfaces()at the gateway entry point to catch restricted system calls.[Sandbox Compatibility]warning log to provide clear context for discovery failures.127.0.0.1) to ensure process stability, allowing manual TUI connections via--url.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesAI Disclosure
Summary by CodeRabbit