fix: exit with the real error when the backend cannot bind its port (supersedes #14) - #31
Closed
abubear wants to merge 1 commit into
Closed
Conversation
The listen-error handler rethrew everything that was not EADDRINUSE. A throw inside an 'error' listener becomes an uncaught exception, and the handler at the top of server.js deliberately keeps the process alive through those -- so a bind failure such as EACCES left a backend running that had never started listening. The desktop shell then reported only "Backend did not start in time", with the actual Node.js error nowhere in sight. Every listen failure is now fatal and named: EADDRINUSE and EACCES get an explanation and a way out, anything else prints the error itself, and all three exit(1) so the shell, a container, or CI sees a failed start. Reported by @srbot2016 in #14, which this supersedes. Co-Authored-By: srbot2016 <38800635+srbot2016@users.noreply.github.com>
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.
Supersedes #14 by @srbot2016, which no longer merges:
mainhas since gained aserver.on('error')handler of its own (209154b), so the PR conflicts and itsconst server = app.listen(...)change is already there. The part of #14 thatstill matters is carried here, with credit to the original author on the commit.
The bug that is still on
mainA
throwinside an'error'listener becomes an uncaught exception, andprocess.on('uncaughtException')at the top ofserver.jsdeliberately keepsthis process alive. So any bind failure other than a port collision — @srbot2016
hit
listen EACCES: permission denied 0.0.0.0:3001— leaves a backend processrunning that never started listening. The Electron shell waits, times out, and
reports:
The real Node.js error is printed only as "Uncaught exception (server kept
alive)" noise, and the process lingers instead of failing.
Reproduced on
main(Windows, port inside a WinNAT-reserved range): the processwas still alive 60s later, having never bound.
The change
Every listen failure is fatal and named, then
process.exit(1):EADDRINUSE— unchanged message and advice.EACCES— new: says permission denied, points at privileged ports (<1024) andat Windows reserved ranges (
netsh int ipv4 show excludedportrange protocol=tcp).rather than behind a timeout.
Nothing is swallowed: the unknown branch logs the full error, and the exit code
now tells the shell, a container, or CI that the start failed.
Testing
npm testdoes not exist in this repo (no test script or runner), so this wasverified by running the backend:
PORT=3201 npm startEADDRINUSEEACCESPORT=49726(WinNAT-reserved range)EACCESonmain(before)npx eslint server.js— clean. Repo-widenpm run lintstill fails onpre-existing
react-hooksissues undersrc/, untouched by this PR.Note for the maintainer
#14 can be closed once this merges — the fix and the credit both live here.