fix(viewport): report once the socket is open, not at page load - #10
Open
myrddian wants to merge 1 commit into
Open
fix(viewport): report once the socket is open, not at page load#10myrddian wants to merge 1 commit into
myrddian wants to merge 1 commit into
Conversation
Found by running it in a browser, which is also what this commit adds
the scaffolding for.
The initial report never reached the server. Two bugs stacked:
1. viewport.start() runs before ws.start(), so the first report was
attempted at readyState=NULL and dropped on the floor.
2. reportNow() recorded the size *before* knowing whether it was sent.
With the size marked as reported, de-duplication then suppressed every
later report of that same size — so a client that opened the page and
never resized left the server on its 80x24 default forever.
Console evidence before the fix:
[ws] send viewport.resize readyState=NULL
ws not open, dropping outbound viewport.resize readyState= NULL
[ws] open <- nothing after this
Fixes:
- WsClient.send returns whether the frame actually left, and the reporter
only records a size that did. A dropped send is no longer mistaken for
a delivered one.
- New WsCallbacks.onOpen; the report fires there. Covers reconnects too,
which the previous auth.ok/resume.ok hooks only partly did — those are
now redundant and removed.
- The first measurement is retried briefly: a region with no layout yet
measures zero, which we correctly refuse to report.
Dev pipeline, so this is reproducible:
- scripts/dev-db.sh — Postgres matching sql/init, port published
- scripts/dev-run.sh — bootRun against it, overlay instance root wired
- .claude/launch.json — preview on 8090 (8080 is often taken by Docker)
Verified in Chrome against a live server: on open, cols=105 rows=24 for
an 846px region; narrowing to 586px reports cols=73; three resizes
produce two sends (de-duplication working); no NOT_IMPLEMENTED warning
server-side and no error frame client-side, so the server consumes it.
Co-Authored-By: Claude Opus 5 <[email protected]>
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.
Stacked on #9. Found by actually running it in a browser — which this PR also adds the scaffolding for.
The bug
The initial viewport report never reached the server. Two bugs stacked:
viewport.start()runs beforews.start(), so the first report was attempted atreadyState=NULLand dropped.reportNow()recorded the size before knowing whether it was sent. With the size marked as reported, de-duplication suppressed every later report of that same size — so a client that opened the page and never resized left the server on its 80x24 default forever.Console evidence before the fix:
Note #8's tests all passed against this. They asserted the reporter's own behaviour, and the reporter behaved: it measured, it called send. Nothing modelled "send didn't actually go anywhere", so nothing caught it.
Fixes
WsClient.sendreturns whether the frame left, and the reporter only records a size that did. A dropped send is no longer mistaken for a delivered one — that's the general fix, not just this instance.WsCallbacks.onOpen, where the report now fires. Covers reconnects too, which the previousauth.ok/resume.okhooks only partly did — those are now redundant and removed.Dev pipeline
So this is reproducible rather than a one-off:
scripts/dev-db.sh— Postgres matchingsql/init, with 5432 published (the Compose postgres deliberately stays on the private network)scripts/dev-run.sh—bootRunagainst it, overlay instance root wired toapp/dev-instance.claude/launch.json— preview on 8090; 8080 is commonly taken on a dev box (Docker Desktop binds it)One trap worth knowing: Spring serves static assets from
build/resources, so a frontend change needs a server restart, not justnpm run build.Verified in Chrome, against a live server
cols=105 rows=24for an 846px-wide region (8×23px cell)cols=73NOT_IMPLEMENTEDwarning server-side and no error frame client-side — the server consumes itStill unverified: anything behind login. I don't create accounts or type passwords, so the editor's focus behaviour and the wrapped-body width change need a human session.