Fix: give an EACCES bind failure an actionable message in the dev API server - #692
Open
AmaadMartin wants to merge 1 commit into
Open
Fix: give an EACCES bind failure an actionable message in the dev API server#692AmaadMartin wants to merge 1 commit into
AmaadMartin wants to merge 1 commit into
Conversation
The dev API server rejected an EACCES bind with Node's raw text. The CLI prints only error.message, so the operator saw no cause and no next step. Map the errno in one module-level function. The EACCES message names the address, both causes, the netsh command that lists Windows reserved ports, and the port 0 escape hatch. EADDRINUSE keeps its wording.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Link to an existing issue (if applicable):
Or, if no issue exists, describe the change:
Problem: The dev API server rejects an
EACCESbind with Node's raw text,listen EACCES: permission denied ::1:8000. The CLI prints onlyerror.message, so that line is the whole diagnostic. Two ordinary situations produce it: a port below 1024 without elevated privileges, and a port inside a block that Hyper-V, WSL2 or the Windows NAT service reserved at boot. In the Windows case nothing holds the socket, sonetstatreports the port as free and the operator has no lead.Solution: One module-level function maps the errno to the reported error. The
EACCESmessage names the address, both causes, thenetshcommand that lists the Windows reserved ranges, andport 0as the escape hatch.EADDRINUSEmoved into the same function with its wording unchanged, which leaves the listener with no branch of its own and makes the mapping testable without a mock. Every other errno returns unchanged, with the same object identity, socausechains and programmatic callers keep working.Two notes for the reviewer:
NodeJS.ErrnoExceptionon the listener parameter. That type lives in an ambient namespace, andnpx eslintrejects it with'NodeJS' is not defined no-undef(2 errors). I declared a localinterface SystemError extends Error {code?: string}instead. No suppression, noany, and the shared ESLint config is untouched.dev/src/server/adk_api_server.ts— Fix: publish the bound port in A2A agent card URLs #300 (urlgetter and thetoA2acall), Fix: force established sockets shut in AdkApiServer.stop() so teardown always settles #452 (stop()) and Fix: reject a non-object Reasoning Engine raw body instead of crashing the dev server #621 (Reasoning Engine raw body). None touches theerrorlistener, so this branches frommainrather than stacking. No open or closed PR mentionsEACCES.Testing Plan
Unit Tests:
npx vitest run --project unit:dev dev/test/server/adk_api_server_test.ts— 55 passed, 4 of them new. Four new cases cover every arm of theswitch:EADDRINUSEwording, theEACCESmessage and itscause, an unrelated errno, and an error with nocode. No existing test changed.Every new test was run against mutated source and observed to fail:
case 'EACCES'armexpected 'listen EACCES: permission denied ::1:…' to contain 'localhost:80'default: return new Error(err.message)expected Error: read ECONNRESET to be Error: read ECONNRESET { code: 'ECON…' } // Object.is equalitydefault: return new Error(err.message)expected Error: something else went wrong to be Error: something else went wrong // Object.is equality`Port ${port} is busy`expected 'Port 8000 is busy' to be 'Port 8000 is already in use'reject(err)and skipstoListenErrorexpected [Function] to throw error including 'Port 45447 is already in use' but got 'listen EADDRINUSE: address already in…'The last mutation pins the wiring: the existing
EADDRINUSEstartup test drives a real bind failure throughstart().Coverage of the new function is 100% of lines and branches (
--coverage.include=dev/src/server/adk_api_server.ts; the uncovered lines the reporter lists all pre-date this change and start at line 117).Manual End-to-End (E2E) Tests:
Linux, as a normal user, from the repository root:
npm run buildnode dev/dist/esm/cli_entrypoint.js api_server --port 80 dev/samplesExit code 1. Repeating with
--port 0starts the server and printshttp://localhost:44133.Other checks on the pushed commit:
npx eslintandnpx prettier --checkare clean on both files.npx tsc --noEmitreports 281 errors with and without this change, all incore/test; the count is identical, so this branch adds none.Checklist