fix: bind the report server where DDEV's router can reach it - #142
Draft
deviantintegral wants to merge 3 commits into
Draft
fix: bind the report server where DDEV's router can reach it#142deviantintegral wants to merge 3 commits into
deviantintegral wants to merge 3 commits into
Conversation
`ddev playwright show-report --host=127.0.0.1`, which the README has recommended since #93, serves the HTML report on the web container's own loopback interface. DDEV's router connects to that container over the Docker network instead, so it never reaches the report server and answers the routed URL with a 502 Bad Gateway -- the symptom reported in #103. show-report now defaults to --host=0.0.0.0, so it needs no flag at all. That does not expose the report to the host network either: container port 9323 is not published, leaving the router as the only way in. An explicit --host or --port is still honoured, with a note saying why it will not be reachable, rather than silently overridden. Playwright's own "Serving HTML report at http://0.0.0.0:9323" line is accurate but describes the in-container address, and #103 found that confusing next to the 9324 in `ddev describe`. Print the routed URL immediately above it and label the line below for what it is. The new test asserts against the routed URL rather than the container port, since a request straight to 9323 succeeds under either binding and would catch nothing. The testdata configs gain the html reporter, with open: 'never' so a failing run cannot start a report server the suite would then wait on. Refs #103 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZyXTWUBXi2LAkbf1kaC2D
Appending --host=0.0.0.0 to the user's arguments meant the wrapper had to know whether they had already passed --host, in either spelling, and had to special-case a trailing --host with no value -- which would otherwise swallow the injected default as its own argument. Playwright's last --host wins (verified against 1.62.1, both --host=VALUE and --host VALUE), so prepending the default instead lets its parser settle precedence. The bookkeeping that existed only to decide whether to inject goes away, and the argument scan that remains serves the advice message alone rather than control flow. Also from the review passes: - report_url() reads DDEV_PRIMARY_URL_WITHOUT_PORT rather than re-deriving the primary hostname by splitting DDEV_HOSTNAME on commas, which meant owning DDEV's rules for hostname ordering. Older releases that set only DDEV_PRIMARY_URL still work. - The router-topology explanation was written out three times. It stays where the binding is chosen; the test points at it. - The README now states the rule rather than the one instance: any server started in the container must bind 0.0.0.0 and use a routed port. `test --ui` and `show-trace` default to localhost and hit the identical 502, so both are documented with flag names checked against the CLI. - curl --max-time in the readiness poll, so the stated budget is a real ceiling rather than one that a hung router could overrun by minutes. - pkill -f '[p]laywright...' so the pattern cannot match the command line of the ddev exec wrapper running it. Recorded in a comment that this cannot be done in configuration: show-report reads neither playwright.config.ts nor PLAYWRIGHT_HTML_HOST, so the CLI flag is the only lever. Refs #103 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZyXTWUBXi2LAkbf1kaC2D
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YZyXTWUBXi2LAkbf1kaC2D
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
1 participant
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.
ddev playwright show-report --host=127.0.0.1, which the README has recommended since #93, serves the HTML report on the web container's own loopback interface. DDEV's router connects to that container over the Docker network instead, so it never reaches the report server and answers the routed URL with a 502 Bad Gateway -- the symptom reported in #103.show-report now defaults to --host=0.0.0.0, so it needs no flag at all. That does not expose the report to the host network either: container port 9323 is not published, leaving the router as the only way in. An explicit --host or --port is still honoured, with a note saying why it will not be reachable, rather than silently overridden.
Playwright's own "Serving HTML report at http://0.0.0.0:9323" line is accurate but describes the in-container address, and #103 found that confusing next to the 9324 in
ddev describe. Print the routed URL immediately above it and label the line below for what it is.The new test asserts against the routed URL rather than the container port, since a request straight to 9323 succeeds under either binding and would catch nothing. The testdata configs gain the html reporter, with open: 'never' so a failing run cannot start a report server the suite would then wait on.
Refs #103
Claude-Session: https://claude.ai/code/session_01YZyXTWUBXi2LAkbf1kaC2D