Skip to content

Name FISHE_WEB_PORT when a web port is busy or misspelled - #177

Merged
dmccoystephenson merged 2 commits into
mainfrom
fix/web-port-errors
Aug 12, 2026
Merged

Name FISHE_WEB_PORT when a web port is busy or misspelled#177
dmccoystephenson merged 2 commits into
mainfrom
fix/web-port-errors

Conversation

@dmccoystephenson

Copy link
Copy Markdown
Member

Summary

  • web/serve.py — the entry point the Dockerfile runs, and therefore the one most likely to be handed a value from outside — converted FISHE_WEB_PORT with a bare int(). A typo was reported as ValueError: invalid literal for int() with base 10: '80801x', which names neither the variable nor what it should hold. The integer check UserInterfaceFactory's WEB branch already had has been given to it as well.
  • Neither web entry point handled a port that is simply taken. Both bound inside a constructor with nothing around it, so a second copy of the game — or anything else already on 8000/8080 — was surfaced as OSError: [Errno 98] Address already in use traced through http.server. allow_reuse_address does not cover this; only a socket left in TIME_WAIT is reopened by it, while a live listener still refuses the bind. A refused bind is now turned into a sentence naming the address, the reason (already listening / not permitted), and FISHE_WEB_PORT.
  • Both failures happen before the front-end exists, so no showDialogue is available to say them through — they are raised, the way the factory already does.
  • web/serve.py now binds before the URL is announced, so a failure is no longer preceded by an address that was never served.
  • The two defaults are kept apart (8080 for the static server, 8000 for the server-backed front-end behind examples/web_app.py), since those are separate programs that can be run at the same time; a comment now records why. Only the check on the value is shared in spirit.
  • README.md now mentions FISHE_WEB_PORT/FISHE_WEB_HOST, which the new messages instruct the player to set.

Front-ends touched

Only the two web entry points bind a socket, so this failure path has no console, pygame, or Pyodide equivalent: the console and pygame front-ends open no server, and the Pyodide front-end runs inside the browser tab. No BaseUserInterface primitive was changed, so front-end parity is unaffected.

Test plan

  • python3 -m compileall -q src tests web
  • python3 -m pytest — 820 passed, including four new tests in tests/web/test_serve.py (default port, port from the environment, a misspelled port naming the variable, a taken port explained rather than traced) and one in tests/ui/test_webUserInterface.py (a taken port explained rather than traced)
  • black run over the changed files
  • No Player/Stats/TimeService field changed, so schemas/*.json and the *JsonReaderWriter classes are untouched

Closes #176

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

dmccoystephenson and others added 2 commits August 12, 2026 03:50
Both web entry points reported the two ordinary startup failures as a raw
traceback. web/serve.py - the one the Dockerfile runs, and so the one most
likely to be handed a value from outside - converted FISHE_WEB_PORT with a
bare int(), so a typo died on "invalid literal for int() with base 10",
naming neither the variable nor what it should hold. Neither entry point
handled a port that was simply taken: both bound inside a constructor with
nothing around it, so a second copy of the game surfaced as an errno raised
from inside http.server. allow_reuse_address does not cover that; it only
reopens a socket left in TIME_WAIT.

serve.py now gets the integer check UserInterfaceFactory's WEB branch already
had, and both entry points turn a refused bind into a sentence naming the
address, the reason, and FISHE_WEB_PORT. The failure happens before the
front-end exists, so there is no showDialogue to say it through - it is
raised, the way the factory already does. serve.py also binds before
announcing the URL, so a failure is no longer preceded by an address that was
never served.

The two defaults stay apart (8080 here, 8000 for the server-backed front-end)
since they are separate programs that can run at once; a comment now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The helper was only exercised through its failure branch, so a version that
always raised would have passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dmccoystephenson

dmccoystephenson commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

Self-review

The full diff was read and checked for logic errors, missed edge cases, front-end-parity gaps, schema/reader-writer mismatches, missing tests on public methods, and doc drift. No blocking issue was found. Three notes are recorded below, along with one gap that has already been closed on this branch.

Front-end parity — verified as not applicable rather than skipped. Only WebUserInterface and web/serve.py bind a socket: UserInterface/ConsoleUserInterface and PygameUserInterface open no server, and PyodideUserInterface runs inside the browser tab with no server at all. No BaseUserInterface primitive was added, removed, or changed, so nothing here has to be mirrored into the other three front-ends.

Save-file contract — no field on Player, Stats, or TimeService was touched, so schemas/player.json, schemas/stats.json, schemas/timeService.json and their *JsonReaderWriter classes are correctly left alone.

Socket cleanup on a failed bind — checked, because a helper that swallows and re-raises is where a half-open socket would be leaked. socketserver.TCPServer.__init__ already wraps server_bind()/server_activate() in a bare except that calls server_close() before re-raising, so the failed socket is closed before either _bindServer ever sees the OSError. Nothing further is needed.

Notes (non-blocking)

  • src/ui/webUserInterface.py:165 — the raised OSError is constructed from a message alone, so errno and strerror are None on it. Nothing in the codebase catches this by errno, and raise ... from e keeps the original reachable via __cause__, so no behaviour is affected. The two-argument form (OSError(e.errno, message)) was considered and rejected: it would prefix the player-facing text with [Errno 98], which is the noise this change exists to remove. The same applies at web/serve.py:118.
  • src/ui/webUserInterface.py:167 — the message tells the reader to set FISHE_WEB_PORT, while WebUserInterface also accepts port as a constructor argument that owes nothing to the environment (the tests pass it directly). In production the only construction path is UserInterfaceFactory's WEB branch, which does read that variable, so the advice is correct for every player who can see the message; it is only approximate for a caller constructing the class directly.
  • web/serve.py:100 — the ValueError is re-raised from inside except ValueError without from None, so the underlying int() error is still shown as context. This is deliberate: UserInterfaceFactory's existing check at src/ui/userInterfaceFactory.py:46-50 does exactly the same, and the two are meant to behave identically.

Addressed during review

  • tests/web/test_serve.py_bindServer was covered only through its failure branch, so an implementation that always raised would have passed. A success-path test asserting that a free port is bound and the server handed back was added in the second commit on this branch.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

@dmccoystephenson
dmccoystephenson merged commit 252c741 into main Aug 12, 2026
1 check passed
@dmccoystephenson
dmccoystephenson deleted the fix/web-port-errors branch August 12, 2026 03:53
dmccoystephenson added a commit that referenced this pull request Aug 14, 2026
examples/web_app.py printed the address before anything was listening on it,
and built that address from its own copy of FISHE_WEB_HOST/FISHE_WEB_PORT
rather than from the server. Both ordinary startup failures - a misspelled
port and a busy one - were therefore preceded by a confident sentence naming
an address that would never answer, since the value is not converted there at
all: UserInterfaceFactory's WEB branch does that, and WebUserInterface binds,
both after the print. web/serve.py was given the opposite ordering in #177, so
the two web entry points disagreed about it.

The ordering cannot simply be swapped there: FishE.__init__ builds the
front-end and then blocks in the save-file manager, so control never comes
back to main() to say what was bound. The announcement moves to
WebUserInterface instead, immediately after _bindServer returns, and names the
socket's own address - so a caller that asked for an ephemeral port is told
the one it actually got. The entry point now announces nothing of its own.

The "8000" default and the two variables that move it move to
webUserInterface as DEFAULT_HOST/DEFAULT_PORT and
resolveAddressFromEnvironment(), which the factory branch and the constructor
now share; the example no longer reads the environment at all. web/serve.py
keeps its own 8080 default, deliberately, as its comment already explains.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A busy or misspelled port is reported as a raw traceback by both web entry points

1 participant