feat(server): counters, readiness, drain mode and a version stamp

expvar counters for connections, rooms, games, submissions by rejection
reason, eliminations, chat, joins and bot moves, served on a separate
debug address so they never sit on the public mux, plus one structured
word_rejected log line per refused word carrying the normalized word and
its link. GET /readyz flips to 503 while draining; SIGTERM stops new
rooms, waits up to NOITU_DRAIN_TIMEOUT for live games, then shuts down.
GET /version and the startup log carry the build's git describe.

Fuzz targets for the frame decoder, the text sanitizer and Vietnamese
normalization; the last one found that composing before lowercasing
could leave a non-NFC result, now recomposed after lowering.

CI runs on dev as well as main, gates gofmt and golangci-lint, tracks the
buf major instead of an exact pin, and dependabot watches every
ecosystem. The lint findings that had been hidden by the default
per-issue cap are fixed.
This commit is contained in:
tiennm99 committed 2026-09-21 01:17:52 +07:00
1 parent 90cd679639
commit 8223f40b16
29 files changed
+1204 -66

No files matched your search

+37
View File
@@ -0,0 +1,37 @@
# One dependency update ecosystem per manifest the project actually has, so
# an upgrade is proposed rather than left to be noticed by chance. See
# docs/deployment.md and README.md for the "moving tag over exact pin"
# preference this exists to serve: a bot is the mechanism that rule assumes.
version: 2
updates:
- package-ecosystem: gomod
directory: /server
schedule:
interval: weekly
groups:
go-minor-patch:
update-types: [minor, patch]
- package-ecosystem: npm
directory: /web
schedule:
interval: weekly
groups:
web-minor-patch:
update-types: [minor, patch]
- package-ecosystem: github-actions
directory: /
schedule:
interval: weekly
groups:
actions-minor-patch:
update-types: [minor, patch]
- package-ecosystem: docker
directory: /
schedule:
interval: weekly
groups:
docker-minor-patch:
update-types: [minor, patch]
+27 -1
View File
@@ -10,7 +10,7 @@ name: ci
on:
push:
branches: [main]
branches: [main, dev]
pull_request:
release:
types: [published]
@@ -33,6 +33,28 @@ jobs:
run: go vet ./...
working-directory: server
# gofmt -l lists files that are not gofmt-clean; -d would only show the
# diff. Non-empty output is the failure signal, so it decides the exit
# code itself rather than leaning on the emptiness of a report nobody
# reads.
- name: Format check
run: |
unformatted="$(gofmt -l .)"
if [ -n "$unformatted" ]; then
echo "not gofmt-clean:"
echo "$unformatted"
exit 1
fi
working-directory: server
# Default linters only: this is a signal every PR has to pass, not a
# style debate, so nothing beyond golangci-lint's own defaults is
# enabled here.
- name: Lint
uses: golangci/golangci-lint-action@v9
with:
working-directory: server
# -race because the whole transport layer is goroutines and timers, and a
# data race there is exactly the kind of defect that passes without it.
- name: Test
@@ -57,6 +79,10 @@ jobs:
run: npm run check
working-directory: web
- name: Lint
run: npm run lint
working-directory: web
# npm test builds first, so this also proves the bundle compiles and
# carries no wordlist.
- name: Test
+5 -2
View File
@@ -7,7 +7,7 @@ name: proto
on:
push:
branches: [main]
branches: [main, dev]
pull_request:
permissions:
@@ -22,9 +22,12 @@ jobs:
# buf breaking compares against main, which needs real history.
fetch-depth: 0
# A moving version rather than an exact pin, per house rule: updates
# arrive on the next run, and a breaking major would surface here before
# it could surprise a contributor's own machine.
- uses: bufbuild/buf-setup-action@v1
with:
version: 1.69.0
version: latest
- uses: actions/setup-go@v5
with:
+8 -1
View File
@@ -18,13 +18,20 @@ RUN npm run build
# --- the binary -------------------------------------------------------------
FROM golang:1.25-alpine AS build
# What GET /version answers and the startup log line carries. .dockerignore
# deliberately keeps .git out of the build context — a stale copy should
# never ship — so git describe cannot run in here; a caller that wants a real
# version passes it in, the way `make image` does. Unset, this defaults to
# "dev", which is honest about an unstamped build.
ARG VERSION=dev
WORKDIR /src/server
COPY server/go.mod server/go.sum ./
RUN go mod download
COPY server/ ./
# CGO_ENABLED=0 is what makes a distroless static image possible, and it works
# because the SQLite driver is pure Go.
RUN CGO_ENABLED=0 go build -trimpath -ldflags='-s -w' -o /out/noitu-server ./cmd/noitu-server
RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w -X main.version=${VERSION}" -o /out/noitu-server ./cmd/noitu-server
RUN CGO_ENABLED=0 go build -trimpath -o /out/build-dictionary ./cmd/build-dictionary
# --- the dictionary ---------------------------------------------------------
+19 -3
View File
@@ -14,7 +14,12 @@ FIXTURE_WORDS := testdata/fixture-words.txt
FIXTURE_DB := data/fixture.db
SERVER_BIN := noitu-server
.PHONY: help fetch-dict dict fixture-dict proto proto-check server web web-dev test test-go test-web test-e2e run clean
# What GET /version answers and what the startup log line carries. "dev" when
# there is no tag and no git history at all (a shallow clone, an extracted
# tarball) — the same fallback -ldflags leaves unstamped Go code with anyway.
VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo dev)
.PHONY: help fetch-dict dict fixture-dict proto proto-check server image web web-dev web-lint test test-go test-web test-e2e run clean
help:
@echo "fetch-dict download the current Wiktionary tiếng Việt dump (~61 MB) into data/"
@@ -22,9 +27,11 @@ help:
@echo "fixture-dict build the small test dictionary — no download needed"
@echo "proto regenerate the Go and JS wire types from proto/"
@echo "proto-check lint the schema and verify the committed output is in sync"
@echo "server build the Go server binary"
@echo "server build the Go server binary, stamped with VERSION"
@echo "image build the container image, stamped with VERSION"
@echo "web build the SvelteKit frontend"
@echo "web-dev run the frontend dev server, proxying /ws to a local server"
@echo "web-lint lint the frontend"
@echo "test run all tests"
@echo "test-e2e run the Playwright suite against the fixture dictionary"
@echo "run build and run the server locally"
@@ -78,7 +85,13 @@ web/node_modules: web/package.json web/package-lock.json
@touch web/node_modules
server:
cd server && CGO_ENABLED=0 go build -o ../$(SERVER_BIN) ./cmd/noitu-server
cd server && CGO_ENABLED=0 go build -ldflags "-X main.version=$(VERSION)" -o ../$(SERVER_BIN) ./cmd/noitu-server
# The Dockerfile cannot run git describe itself — .dockerignore deliberately
# keeps .git out of the build context, so a stale copy never ships — so this
# is the one place VERSION reaches it, as a build-arg.
image:
docker build --build-arg VERSION=$(VERSION) -t noitu:$(VERSION) -t noitu:latest .
web: web/node_modules
cd web && npm run build
@@ -89,6 +102,9 @@ web: web/node_modules
web-dev: web/node_modules
cd web && npm run dev
web-lint: web/node_modules
cd web && npm run lint
test: test-go test-web
test-go:
+23 -6
View File
@@ -151,6 +151,11 @@ Every Vietnamese string lives in `web/src/lib/i18n/vi.js`, including the map fro
`room_not_found` and never prose. A test walks the generated enums and fails when a value has
no message, so a schema change cannot quietly ship an untranslated screen.
The rules — the chain, the clock, a dead end, elimination, scoring, room codes and reconnect
grace — are written out once on `/rules`, a single anchored page linked from the landing
screen and from a small "Luật chơi" link beside the connection badge in the game header and
the lobby. It is not a button: reading the rules is not a way to start playing.
The countdown is drawn against the server's clock, estimated from the `Ping`/`Pong` round
trip, and settles 300ms early so the ring never claims more time than the server allows.
@@ -192,13 +197,17 @@ Configuration is environment-only; every variable has a working default.
| `NOITU_TRUSTED_PROXIES` | *(unset)* | Comma-separated proxy addresses or CIDRs whose `X-Forwarded-For` is believed. Unset keys limiters on the socket peer |
| `NOITU_MAX_ROOMS` | `1000` | Ceiling on live rooms across the process; a creator past it is told `server_full` |
| `NOITU_MAX_CONNECTIONS` | `2000` | Ceiling on open WebSockets; the next upgrade gets HTTP 503 |
| `NOITU_DEBUG_ADDR` | *(unset)* | A separate listen address for `GET /debug/vars` (expvar). Unset means the operational counters are not exposed anywhere |
| `NOITU_DRAIN_TIMEOUT` | `0s` | How long a shutdown waits for live *games* (not lobbies) to finish before ending them anyway. `0s` is today's behaviour: end them immediately |
An invalid duration or count is logged and ignored rather than silently
changing the rules of the game.
Endpoints: `GET /ws` (Protobuf over binary WebSocket frames), `GET /healthz`,
and — when `NOITU_WEB_DIR` is set — the frontend on everything else, with
unknown paths falling back to `index.html` because deep links are client routes.
Endpoints: `GET /ws` (Protobuf over binary WebSocket frames), `GET /healthz`
(liveness), `GET /readyz` (readiness — 503 while draining), `GET /version`
(plain text), and — when `NOITU_WEB_DIR` is set — the frontend on everything
else, with unknown paths falling back to `index.html` because deep links are
client routes.
### Smoke-testing without a frontend
@@ -227,9 +236,11 @@ dev-only URL to get wrong.
|---|---|
| `fetch-dict` | Download the current upstream Wiktionary export (~62 MB) into `data/` |
| `dict` | Derive `data/noitu.db` from the upstream export |
| `server` | Build the Go server binary |
| `server` | Build the Go server binary, stamped with the version `git describe` reports |
| `image` | Build the container image, stamped the same way |
| `web` | Build the SvelteKit frontend to static assets |
| `web-dev` | Run the frontend dev server, proxying `/ws` to a local server |
| `web-lint` | Lint the frontend |
| `proto` | Regenerate the Go and JS wire types from `proto/` (needs `buf`) |
| `proto-check` | Lint the schema and verify the committed generated code is in sync |
| `fixture-dict` | Build the small test dictionary, no download needed |
@@ -254,11 +265,17 @@ cd server && go run ./cmd/build-dictionary --dump ../data/viwiktionary-latest-pa
cd server && go vet ./... && go test ./... -race
# server
cd server && CGO_ENABLED=0 go build -o ../noitu-server ./cmd/noitu-server
cd server && CGO_ENABLED=0 go build -ldflags "-X main.version=$(git describe --tags --always --dirty)" -o ../noitu-server ./cmd/noitu-server
# image
docker build --build-arg VERSION="$(git describe --tags --always --dirty)" -t noitu:latest .
# web
cd web && npm ci && npm run build
# web-lint
cd web && npm run lint
# test-web (npm test builds first, then checks the bundle carries no wordlist)
cd web && npm run check && npm test
@@ -292,7 +309,7 @@ dictionary. See [`docs/deployment.md`](./docs/deployment.md) for configuration,
reverse-proxy requirements, and what a restart costs.
```sh
docker build -t noitu:latest .
make image # stamps the image with `git describe`; see docs/deployment.md
docker run -p 8080:8080 noitu:latest
```
+99 -14
View File
@@ -1,9 +1,9 @@
# Deployment
The whole game is one binary. It serves the WebSocket API, the built frontend,
and a health check, and it reads a single database file at startup. The
supported shape is the container image behind a reverse proxy that terminates
TLS.
and its own health, readiness and version endpoints, and it reads a single
database file at startup. The supported shape is the container image behind a
reverse proxy that terminates TLS.
## Configuration
@@ -21,6 +21,8 @@ so the image runs with nothing set.
| `NOITU_TRUSTED_PROXIES` | *(unset)* | Comma-separated proxy addresses or CIDRs whose `X-Forwarded-For` is believed. Unset keys limiters on the socket peer |
| `NOITU_MAX_ROOMS` | `1000` | Ceiling on live rooms across the process; a creator past it is told `server_full` |
| `NOITU_MAX_CONNECTIONS` | `2000` | Ceiling on open WebSockets; the next upgrade gets HTTP 503 |
| `NOITU_DEBUG_ADDR` | *(unset)* | Separate listen address for `GET /debug/vars` (expvar counters). Unset means the counters exist in the process but nothing serves them |
| `NOITU_DRAIN_TIMEOUT` | `0s` | How long a shutdown waits for live games to finish before ending them anyway; see "Draining on deploy" below |
An invalid duration or count is logged and ignored rather than silently
changing the rules of the game.
@@ -50,10 +52,13 @@ permissive direction lets any page open a socket as one of your players.
## The image
```sh
docker build -t noitu:latest .
docker build --build-arg VERSION="$(git describe --tags --always --dirty)" -t noitu:latest .
docker run -p 8080:8080 noitu:latest
```
`make image` runs the same build with `VERSION` filled in for you; see
"Version" below.
The build downloads the ~62 MB upstream Wiktionary export in a builder stage and
derives the ~2 MB database the game uses. Only the derived file is copied into
the final image, so the upstream export never ships. The result is a
@@ -63,6 +68,20 @@ Passing `--build-arg FIXTURE_DICT=1` builds the same image against the
checked-in word sample instead. It produces a playable but tiny dictionary and
exists so the image can be tested without the download; do not ship it.
### Version
`GET /version` answers with the build's version, and the same string opens
the startup log line — the fastest way to confirm a deploy actually replaced
the running process rather than restarted the old one. It comes from
`-X main.version=...` at link time, populated from `git describe --tags
--always --dirty`. `make server` runs that command directly; the Dockerfile
cannot — `.dockerignore` deliberately keeps `.git` out of the build context,
so a stale copy never ships in the image — so it takes the version as the
`VERSION` build-arg instead, which `make image` supplies. Building the image
directly with `docker build .` and no `--build-arg VERSION=...` reports
`dev`, which is an honest answer for an unstamped build rather than a wrong
one.
### What travels with the data
The derived wordlist is CC BY-SA 4.0 while the code is Apache-2.0, so the image
@@ -144,25 +163,91 @@ client past it is disconnected rather than throttled. The defaults are
generous for one binary on a small host; lower them if memory is tight,
because a room is a goroutine and an engine held for up to its idle window.
## Health check
## Observability
`GET /healthz` returns 200 once the dictionary has loaded. It does not report
on live games, so it is a liveness check rather than a readiness one.
Set `NOITU_DEBUG_ADDR` to a second listen address — one that is not the one
players reach — to expose `GET /debug/vars` there: standard-library
[`expvar`](https://pkg.go.dev/expvar), zero extra dependencies, a JSON object
of process counters refreshed on every write. It is never mounted on the
public address, unset or not, so leaving `NOITU_DEBUG_ADDR` unset is the same
as not having it. The counters, all prefixed `noitu_`: connections open and
total; rooms live and total, each split `bot`/`pvp`; games started and
finished the same way; words submitted, accepted, and rejected by reason;
eliminations by reason; chat lines; join attempts refused, by whether it was
the rate limit, an unknown code, or a full room; bot moves by difficulty; and
resumes attempted versus succeeded. None of it is read by the game itself —
it is a second write next to a decision already made, not an input to one.
Every rejected word also gets one structured log line at `Info`,
`word_rejected`, carrying `reason`, `word`, `link` (the syllable it had to
start with), `mode` (`bot`/`pvp`) and `room`. `word` is never the raw text a
player typed — it is normalized the same way the engine matches it (NFC,
lowercase, single-spaced) and capped at 64 runes — so the line is safe to
collect and is exactly the corpus-review question this project has open:
which words players type that the dictionary does not have. Nothing else a
player types is logged: not chat, not a nickname, not an accepted word.
## Health and readiness
`GET /healthz` returns 200 once the dictionary has loaded, for the rest of the
process's life. It does not report on live games, so it is a liveness check
rather than a readiness one, and it never moves — a proxy or orchestrator
using it to decide whether to kill the process must not see it fail during a
drain, because the process is still correctly finishing the games it has.
```sh
curl -fsS https://noitu.example/healthz
```
A post-deploy check worth having is a real socket open, because the health
check passes whether or not the proxy forwards upgrades. Opening the site and
starting a game against the bot is the shortest version of that.
`GET /readyz` is the readiness check: 200 while the server is accepting new
rooms, 503 once it has started draining (see below). Point a load balancer's
"stop sending me new traffic" check here and its "restart me" check at
`/healthz`; pointing both at the same endpoint defeats the reason there are
two.
A post-deploy check worth having beyond either is a real socket open, because
both health checks pass whether or not the proxy forwards upgrades. Opening
the site and starting a game against the bot is the shortest version of that.
## Draining on deploy
Rooms are in memory, so a restart has always ended every live game — but a
plain `kill` used to do that the instant the signal arrived, which is why
"deploy when the game is quiet" was the only advice this document had.
`SIGTERM` now runs a short sequence first:
1. The server stops accepting new rooms. A creator past this point is told
`server_restarting` — the same UI key a live shutdown sends everyone else —
rather than `server_full`, because unlike a full room this one is never
coming back.
2. `GET /readyz` flips to 503, so a load balancer that checks it stops routing
new players here.
3. The server waits up to `NOITU_DRAIN_TIMEOUT` for every room with a game
*actually running* to finish on its own turn clock. A room sitting in its
lobby does not count — it has no game a restart would cost, and waiting for
one would make every deploy sit out somebody's abandoned tab.
4. Once every game has finished, or the timeout passes, every player still
connected is told the server is restarting and the process shuts down as
it always did.
Each step logs the room and live-game count, so "did the deploy actually
wait, and for what" is answered from the log rather than guessed at.
The default, `NOITU_DRAIN_TIMEOUT=0s`, is today's behaviour: nothing waits,
every live game ends immediately. Setting it to something like `60s` turns
"deploy when the game is quiet" into "deploy whenever, and the games in
flight get up to a minute to finish before they are cut off anyway" — the
turn clock already bounds how long any one game can take, so a timeout a
little over `NOITU_TURN_LIMIT` covers the common case of a handful of games
mid-turn.
## What a restart costs
Rooms are in memory. A restart ends every live game, and players are told the
server is restarting rather than being left waiting. Deploy when the game is
quiet, or accept that the games in flight are lost — there is no session
persistence, by design, in this version.
A restart with `NOITU_DRAIN_TIMEOUT` unset, or a signal harder than `SIGTERM`,
ends every live game immediately and tells players the server is restarting
rather than leaving them waiting. There is no session persistence, by design,
in this version — a game that does not finish inside the drain window is
simply lost.
## Updating the dictionary
@@ -0,0 +1,168 @@
# Server ops/observability batch
Scope: server/, .github/, Dockerfile, Makefile, README.md, docs/deployment.md. Did not
touch proto/, web/, or generated code.
## What changed
### 1. Observability (D2)
- `server/internal/wsapi/metrics.go` (new): `metricSet` struct of `expvar.Int`/`expvar.Map`,
one package var `metrics`. Counters: connections open/total; rooms live/total (bot/pvp);
games started/finished (bot/pvp); words submitted/accepted/rejected (by
`game.RejectReason.String()`); eliminations (by wire `GameEndReason` string); chat lines;
joins refused (`too_many_attempts`/`room_not_found`/`room_full` — reused wire error codes
as keys, one vocabulary instead of two); bot moves (by `bot.Difficulty.String()`); resumes
attempted/succeeded.
- Wired at call sites: `server.go` (handleWS: connections), `hub.go` (newRegisteredRoom:
rooms live/total, draining check), `room.go` (beginGame: games started; broadcastGameOver:
games finished; handleSubmit: words + new `recordRejection` helper; handleBotMove: bot
moves; handleChat: chat lines; handleJoin: room_full; broadcastElimination: eliminations;
handleResume: resumes succeeded), `session.go` (resumeFrom: resumes attempted; JoinRoom
dispatch: rate-limited/not-found refusals).
- `GET /debug/vars` only ever mounted on a **separate** `http.Server` in
`cmd/noitu-server/main.go` (`newDebugServer`), started only when `NOITU_DEBUG_ADDR` is set.
Never touches the public mux — verified by `TestDebugVarsNotOnPublicMux`.
- `word_rejected` Info log line in `room.recordRejection` (room.go): `reason`, `word`
(normalized via `vietnamese.Normalize` after `sanitizeText`, capped at `maxWordRunes`=64 —
never the raw typed text), `link` (`engine.Current()`), `mode`, `room`. No chat text or
nicknames logged anywhere.
- Documented in `docs/deployment.md` under new "## Observability" section: what
`NOITU_DEBUG_ADDR` does, every counter name, and exactly what the log line contains.
### 2. Ops hygiene (D4 + D1a)
- `GET /readyz` (server.go): 200 while accepting, 503 while draining (`hub.isDraining()`).
`GET /healthz` unchanged.
- Drain mode: `hub.draining` (atomic.Bool) refuses new rooms with `errDraining` →
`server_restarting` (session.go `roomCreateError`, reusing the existing i18n key).
`hub.liveGames` (atomic.Int64) counts rooms with a game **running**, not lobbies;
`room.liveCounted` (atomic.Bool) + a `CompareAndSwap` in `room.run()`'s teardown guarantees
exactly one decrement per increment even if a room is cancelled mid-game rather than
finishing normally. `main.go`'s `run()`: on ctx.Done, logs room/live-game count, calls
`api.StartDraining()`, calls `waitForGamesToFinish(api, cfg.drainTimeout)` (polls
`LiveGameCount()` every 200ms up to `NOITU_DRAIN_TIMEOUT`), logs again, then does the
existing `api.Shutdown()` / `srv.Shutdown()`.
- Version stamp: `main.version` (default `"dev"`), set via `-X main.version=...` from
`git describe --tags --always --dirty`. `Makefile`'s `server` target computes it directly;
a new `image` target passes it to `docker build --build-arg VERSION=...` (`.dockerignore`
deliberately excludes `.git`, so the Dockerfile can't run `git describe` itself — this is
the standard, documented reason). `wsapi.Config.Version` → `Server.version` → printed in
the `listening` log line and served as plain text by `GET /version`.
- `README.md` and `docs/deployment.md`: added `NOITU_DEBUG_ADDR`, `NOITU_DRAIN_TIMEOUT` to
the env tables; new "Draining on deploy" section; "Version" subsection under "The image";
`image` make target in both the targets table and "Without make" raw equivalents;
`/readyz` and `/version` added to the endpoints list.
### 3. Fuzz targets (F3)
- `server/internal/wsapi/codec_test.go`: `FuzzDecode`, seeded from `wire_test.go`'s
`clientVariants()` plus malformed/short/wrong-type frames. Asserts non-binary always
`ErrNotBinary`, otherwise either a real error or a real message — never both nil.
- `server/internal/wsapi/nickname_test.go` (new): `FuzzSanitizeText`, seeded from the
existing `TestSanitizeNickname` table plus the stacking-mark/bidi/zero-width regression
cases. Asserts: no control/format char in the output, no non-printable rune, rune count
≤ `maxNicknameRunes`, no run of combining marks over `maxNicknameMarks`.
- `server/internal/vietnamese/normalize_test.go`: `FuzzNormalize`, seeded from the existing
table tests. Asserts NFC, lowercase, single-spaced, and word/syllables agree with each
other (or `ErrEmpty` with empty output).
**Found and fixed a real bug**: `Normalize` composed (NFC) then lowercased, but lowering a
base rune can enable a composition that only exists for the lowercase form (e.g. `Y` + ring
above has no precomposed codepoint; `y` + ring above does, U+1E99) — so the result could be
non-NFC despite the doc's promise. Fixed by re-composing after lowering
(`normalize.go`). Vietnamese text never hits this (no case-asymmetric diacritics in the
language), so no dictionary data is invalidated by the fix. Regression seed corpus saved at
`server/internal/vietnamese/testdata/fuzz/FuzzNormalize/d3ebd1ce3935d20e`.
- All three run their seed corpus under plain `go test` (fast, no `-fuzz` needed); verified
each with `-fuzz -fuzztime 15-20s` locally, clean after the Normalize fix.
### 4. CI hygiene (F4 + reviewer findings)
- `.github/workflows/ci.yml`: `push` trigger now `[main, dev]`. Go job gained a `gofmt -l`
format-check step and a `golangci-lint` step (`golangci/golangci-lint-action@v9`, moving
major tag, default config — no `.golangci.yml` added, no linters disabled).
- `.github/workflows/proto.yml`: `push` trigger now `[main, dev]`; `bufbuild/buf-setup-action`
`version:` changed from the pinned `1.69.0` to `latest`; `buf breaking` still compares
against `.git#ref=origin/main`, untouched.
- `.github/dependabot.yml` (new): `gomod` (`/server`), `npm` (`/web`), `github-actions` (`/`),
`docker` (`/`); weekly; each grouped for minor+patch.
- Fixed **23** pre-existing `errcheck` findings and **1** `staticcheck` finding (Unicode
format chars in a string literal) across `server/` so the tree passes `golangci-lint`
default linters clean — these were latent (hidden by golangci-lint's default
`max-same-issues: 3`, confirmed with `--max-same-issues=0`), not something I introduced;
fixed rather than suppressed per the instruction not to add lint noise. Files touched:
`cmd/build-dictionary/{dump.go,main.go,main_test.go}`, `internal/dictionary/{store.go,
store_test.go}`, `internal/wsapi/{multiplayer_test.go,limits_test.go}`. All are
`defer x.Close()`/`Rollback()`/`os.Remove()` wrapped as `defer func() { _ = x.Close() }()`,
or a raw Unicode literal replaced with its `\uXXXX` escape.
- `make proto-check` unaffected (didn't touch `proto/`, `buf.yaml`, `buf.gen.yaml`, or the
builder's `DICT_URL` constant).
### 5. Makefile / README
- `Makefile`: `VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo
dev)`; `server` target now passes `-ldflags "-X main.version=$(VERSION)"`; new `image`
target; help text and `.PHONY` updated for both. (`web-lint` target/step already present
from a concurrent change — left untouched, not mine.)
- `README.md`: env table, endpoints list, Make targets table, "Without make" raw equivalents
(server ldflags line, new `image` raw docker command), Deployment section's docker example
now points at `make image`.
## Verification
```
cd server && go vet ./... && gofmt -l . && golangci-lint run ./... && go test ./... -race -count=1
```
Tail of the actual run:
```
0 issues.
ok github.com/tiennm99dev/noitu/server/cmd/build-dictionary 2.803s
? github.com/tiennm99dev/noitu/server/cmd/noitu-server [no test files]
? github.com/tiennm99dev/noitu/server/gen/noitu/v1 [no test files]
ok github.com/tiennm99dev/noitu/server/internal/bot 1.609s
ok github.com/tiennm99dev/noitu/server/internal/dictionary 4.074s
ok github.com/tiennm99dev/noitu/server/internal/game 1.055s
ok github.com/tiennm99dev/noitu/server/internal/vietnamese 1.030s
ok github.com/tiennm99dev/noitu/server/internal/wsapi 28.489s
```
Also: built the real binary via `make server` (embeds `442ced3-dirty`, confirmed via
`/version`); ran it against the fixture dictionary with `NOITU_DEBUG_ADDR` set and curled
`/healthz` (200), `/readyz` (200, then 503 conceptually verified via the unit test),
`/version` (`442ced3-dirty`), `/debug/vars` on the public port (404) and on the debug port
(200, all `noitu_*` counters present at zero); sent `SIGTERM` and confirmed the `draining` /
`shutting down` log lines with room/live-game counts. Cleaned up the binary, fixture db, and
verified no leftover process. Also built the Dockerfile's `build` stage in isolation
(`docker build --target build --build-arg VERSION=...`, avoids the `web` stage's `npm`
entirely per the constraint) to confirm the new `-ldflags` line is valid; removed the test
image afterward.
New/changed tests, all in the behaviour list asked for:
- `TestReadyzFlipsOnDrain`, `TestDrainRefusesNewRooms`, `TestLiveGameCountTracksGamesNotLobbies`
(drain_test.go)
- `TestVersionEndpoint`, `TestVersionEndpointDefaultsWhenUnset`, `TestDebugVarsNotOnPublicMux`
(drain_test.go)
- `TestMetricsCountWordSubmissions` (metrics_test.go)
- `FuzzDecode`, `FuzzSanitizeText`, `FuzzNormalize`
Two existing tests (`TestTheFirstTurnIsDrawn`, `TestABotGameOpensWithTheHuman`) hand-build a
`room{}` and call `beginGame()` directly without going through the hub; `beginGame` now
touches `r.hub.gameStarted()`, so both were given `hub: &hub{}` — a zero-value hub is fine
since only its atomic fields are touched.
## Deferred / not done
- Did not add per-room `game_in_progress` join refusal to the `joinsRefused` metric — spec
named exactly three buckets (rate limit/not found/full); adding a fourth wasn't asked for.
- Did not touch `ci.yml`'s image-build steps to pass `VERSION` — the deliverable named the
Makefile and Dockerfile specifically; the Dockerfile's `ARG VERSION=dev` default keeps
those builds working unchanged.
- Observed `TestStandingsReachEverySeat` fail once under full-suite load (`context deadline
exceeded` reading a socket) but pass 5/5 in isolation and clean on every subsequent full
run; looks like a pre-existing environment flake under CPU contention, not something my
change touches (it doesn't call any of the changed code paths). Did not chase further since
the suite has been green on every full re-run since.
## Unresolved questions
- None blocking. One judgment call worth flagging: `NOITU_DRAIN_TIMEOUT` accepts `0` as an
explicit valid value (meaning "don't wait"), unlike every other duration env var where `0`/
invalid falls back to the default — needed a second parser (`envNonNegDuration`) rather than
reusing `envDuration`. Documented inline; flagging in case the two-parser asymmetry should
instead be a single parser with a flag, but the current split reads clearer to me.
+1 -1
View File
@@ -76,7 +76,7 @@ func readDump(path string) (map[string]entry, map[string][]sense, map[rejectReas
if err != nil {
return nil, nil, nil, stats, prov, fmt.Errorf("read dump: %w", err)
}
defer f.Close()
defer func() { _ = f.Close() }()
info, err := f.Stat()
if err != nil {
// A provenance row must be right or absent, never a plausible zero.
+9 -9
View File
@@ -230,7 +230,7 @@ func verify(path string, minWords int, requireCoverage bool) error {
if err != nil {
return err
}
defer db.Close()
defer func() { _ = db.Close() }()
checks := []struct {
desc string
@@ -369,7 +369,7 @@ func write(path string, words map[string]entry, meanings map[string][]sense, ali
committed := false
defer func() {
if !committed {
os.Remove(tmp)
_ = os.Remove(tmp)
}
}()
@@ -399,7 +399,7 @@ func writeTo(path string, words map[string]entry, meanings map[string][]sense, a
if err != nil {
return fmt.Errorf("create output: %w", err)
}
defer db.Close()
defer func() { _ = db.Close() }()
schema := `
CREATE TABLE words (
@@ -441,13 +441,13 @@ CREATE TABLE meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
if err != nil {
return err
}
defer tx.Rollback()
defer func() { _ = tx.Rollback() }()
insertWord, err := tx.Prepare(`INSERT INTO words (word, first, last, syllables) VALUES (?, ?, ?, ?)`)
if err != nil {
return err
}
defer insertWord.Close()
defer func() { _ = insertWord.Close() }()
outDegree := make(map[string]int)
for _, e := range words {
@@ -466,7 +466,7 @@ CREATE TABLE meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
if err != nil {
return err
}
defer insertSyllable.Close()
defer func() { _ = insertSyllable.Close() }()
for syllable, degree := range outDegree {
if _, err := insertSyllable.Exec(syllable, degree); err != nil {
return fmt.Errorf("insert syllable %q: %w", syllable, err)
@@ -477,7 +477,7 @@ CREATE TABLE meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
if err != nil {
return err
}
defer insertAlias.Close()
defer func() { _ = insertAlias.Close() }()
for variant, canonical := range aliases {
if _, err := insertAlias.Exec(variant, canonical); err != nil {
return fmt.Errorf("insert alias %q: %w", variant, err)
@@ -488,7 +488,7 @@ CREATE TABLE meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
if err != nil {
return err
}
defer insertMeaning.Close()
defer func() { _ = insertMeaning.Close() }()
meaningCount := 0
for word, senses := range meanings {
if _, isWord := words[word]; !isWord {
@@ -509,7 +509,7 @@ CREATE TABLE meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
if err != nil {
return err
}
defer insertMeta.Close()
defer func() { _ = insertMeta.Close() }()
meta := [][2]string{
{"source_url", src.url},
{"source_license", src.license},
+4 -4
View File
@@ -38,7 +38,7 @@ func openOut(t *testing.T, path string) *sql.DB {
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { db.Close() })
t.Cleanup(func() { _ = db.Close() })
return db
}
@@ -123,7 +123,7 @@ func TestBuildWritesMeanings(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
var got []sense
for rows.Next() {
var ord int
@@ -269,7 +269,7 @@ func TestWordListCarriesTabSeparatedMeanings(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
type row struct {
word string
ord int
@@ -326,7 +326,7 @@ func brokenDB(t *testing.T, extraSQL string) string {
if err != nil {
t.Fatal(err)
}
defer db.Close()
defer func() { _ = db.Close() }()
if _, err := db.Exec(extraSQL); err != nil {
t.Fatal(err)
}
+107 -3
View File
@@ -5,6 +5,7 @@ package main
import (
"context"
"errors"
"expvar"
"log/slog"
"net/http"
"os"
@@ -28,8 +29,20 @@ const (
// arrives. Rooms are told separately and immediately, so this only covers
// the HTTP side.
shutdownGrace = 10 * time.Second
// drainPollInterval is how often waitForGamesToFinish checks whether the
// last live game has ended. Short enough that a drain does not overrun its
// timeout by more than a blink, cheap enough to poll at all — the
// alternative is a channel the hub would need to fan out to every room.
drainPollInterval = 200 * time.Millisecond
)
// version is the build the process is running. The default here is what
// `go run` or a build with no -ldflags reports; a real build sets it with
// -X main.version=$(git describe --tags --always --dirty), from the Makefile
// server target or the Dockerfile's VERSION build-arg.
var version = "dev"
type config struct {
addr string
dbPath string
@@ -40,6 +53,8 @@ type config struct {
trustedProxies []string
maxRooms int
maxConnections int
debugAddr string
drainTimeout time.Duration
}
func main() {
@@ -81,6 +96,7 @@ func run() error {
TrustedProxies: cfg.trustedProxies,
MaxRooms: cfg.maxRooms,
MaxConnections: cfg.maxConnections,
Version: version,
})
srv := &http.Server{
@@ -89,13 +105,23 @@ func run() error {
ReadHeaderTimeout: 10 * time.Second,
}
debugSrv := newDebugServer(cfg.debugAddr)
errc := make(chan error, 1)
go func() {
slog.Info("listening", "addr", cfg.addr, "turn_limit", cfg.turnLimit, "web_dir", cfg.webDir)
slog.Info("listening", "addr", cfg.addr, "turn_limit", cfg.turnLimit, "web_dir", cfg.webDir, "version", version)
if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
errc <- err
}
}()
if debugSrv != nil {
go func() {
slog.Info("debug endpoint listening", "addr", cfg.debugAddr)
if err := debugSrv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
slog.Error("debug endpoint exited", "err", err)
}
}()
}
select {
case err := <-errc:
@@ -103,16 +129,74 @@ func run() error {
case <-ctx.Done():
}
// Draining is the first half of shutdown: stop seating new rooms and flip
// /readyz unhealthy, so a load balancer stops sending this instance new
// traffic while the games it already has finish on their own turn clock.
// A creator refused during this window gets server_restarting rather than
// server_full — the room is not coming back, unlike a full one.
slog.Info("draining", "rooms", api.RoomCount(), "live_games", api.LiveGameCount())
api.StartDraining()
waitForGamesToFinish(api, cfg.drainTimeout)
// Tell players why before the sockets go, rather than dropping them and
// leaving the UI to guess.
slog.Info("shutting down")
slog.Info("shutting down", "rooms", api.RoomCount(), "live_games", api.LiveGameCount())
api.Shutdown()
if debugSrv != nil {
shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownGrace)
_ = debugSrv.Shutdown(shutdownCtx)
cancel()
}
shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownGrace)
defer cancel()
return srv.Shutdown(shutdownCtx)
}
// newDebugServer builds the expvar listener, or nil when NOITU_DEBUG_ADDR is
// unset. It is a separate *http.Server on its own address rather than a route
// on the public mux: /debug/vars is an operator surface, and the two must not
// be reachable from the same port a player's browser talks to.
func newDebugServer(addr string) *http.Server {
if addr == "" {
return nil
}
mux := http.NewServeMux()
mux.Handle("/debug/vars", expvar.Handler())
return &http.Server{Addr: addr, Handler: mux, ReadHeaderTimeout: 10 * time.Second}
}
// waitForGamesToFinish blocks until every room's game has ended or timeout
// passes, whichever is first. timeout <= 0 returns immediately, which is
// today's behaviour: rooms are told the server is restarting and torn down
// with whatever they were doing.
//
// Only games count, not lobbies: a room nobody has started a game in has
// nothing a restart costs, and waiting for it would make every deploy sit out
// somebody's abandoned tab for the full timeout.
func waitForGamesToFinish(api *wsapi.Server, timeout time.Duration) {
if timeout <= 0 {
return
}
deadline := time.NewTimer(timeout)
defer deadline.Stop()
ticker := time.NewTicker(drainPollInterval)
defer ticker.Stop()
for {
if api.LiveGameCount() == 0 {
return
}
select {
case <-deadline.C:
slog.Info("drain timed out with games still running", "live_games", api.LiveGameCount())
return
case <-ticker.C:
}
}
}
func loadConfig() config {
return config{
addr: env("NOITU_ADDR", defaultAddr),
@@ -124,6 +208,8 @@ func loadConfig() config {
trustedProxies: envList("NOITU_TRUSTED_PROXIES"),
maxRooms: envInt("NOITU_MAX_ROOMS", 0),
maxConnections: envInt("NOITU_MAX_CONNECTIONS", 0),
debugAddr: env("NOITU_DEBUG_ADDR", ""),
drainTimeout: envNonNegDuration("NOITU_DRAIN_TIMEOUT", 0),
}
}
@@ -150,7 +236,9 @@ func env(key, fallback string) string {
}
// envDuration falls back loudly. A typo in a timing variable would otherwise
// silently change the rules of the game.
// silently change the rules of the game. Zero is rejected along with anything
// unparseable, which is right for every duration except the drain timeout —
// see envNonNegDuration.
func envDuration(key string, fallback time.Duration) time.Duration {
raw := strings.TrimSpace(os.Getenv(key))
if raw == "" {
@@ -164,6 +252,22 @@ func envDuration(key string, fallback time.Duration) time.Duration {
return d
}
// envNonNegDuration is envDuration with zero accepted as a real value rather
// than a trigger for the fallback: NOITU_DRAIN_TIMEOUT=0 means "do not wait",
// which is a deliberate choice an operator can make explicitly, not a typo.
func envNonNegDuration(key string, fallback time.Duration) time.Duration {
raw := strings.TrimSpace(os.Getenv(key))
if raw == "" {
return fallback
}
d, err := time.ParseDuration(raw)
if err != nil || d < 0 {
slog.Warn("ignoring invalid duration", "key", key, "value", raw, "using", fallback)
return fallback
}
return d
}
// envList returns nil for an unset variable, which coder/websocket reads as
// same-origin only.
func envList(key string) []string {
+5 -5
View File
@@ -94,7 +94,7 @@ func Open(path string) (*Store, error) {
if err != nil {
return nil, fmt.Errorf("open dictionary: %w", err)
}
defer db.Close()
defer func() { _ = db.Close() }()
if err := db.Ping(); err != nil {
return nil, fmt.Errorf("open dictionary: %w", err)
@@ -193,7 +193,7 @@ func (s *Store) loadSyllables(db *sql.DB) error {
if err != nil {
return fmt.Errorf("load syllables: %w", err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
for rows.Next() {
var syllable string
@@ -214,7 +214,7 @@ func (s *Store) loadWords(db *sql.DB) error {
if err != nil {
return fmt.Errorf("load words: %w", err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
for rows.Next() {
var word, first, last string
@@ -236,7 +236,7 @@ func (s *Store) loadAliases(db *sql.DB) error {
if err != nil {
return fmt.Errorf("load aliases: %w", err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
for rows.Next() {
var variant, canonical string
@@ -256,7 +256,7 @@ func (s *Store) loadMeanings(db *sql.DB) error {
if err != nil {
return fmt.Errorf("load meanings: %w", err)
}
defer rows.Close()
defer func() { _ = rows.Close() }()
for rows.Next() {
var word string
+2 -2
View File
@@ -38,7 +38,7 @@ func fixtureAt(tb testing.TB, dir string) string {
if err != nil {
tb.Fatal(err)
}
defer db.Close()
defer func() { _ = db.Close() }()
data := fixtureSchema + `
INSERT INTO meta VALUES ('source_license','CC BY-SA 4.0'),('word_count','7'),('meaning_count','3');
@@ -87,7 +87,7 @@ func writeDB(tb testing.TB, sqlText string) string {
if err != nil {
tb.Fatal(err)
}
defer db.Close()
defer func() { _ = db.Close() }()
if _, err := db.Exec(sqlText); err != nil {
tb.Fatal(err)
}
+9 -1
View File
@@ -36,7 +36,15 @@ var ErrEmpty = errors.New("vietnamese: empty input")
// non-breaking spaces that IMEs and copy-paste routinely introduce.
func Normalize(raw string) (string, []string, error) {
composed := norm.NFC.String(raw)
lowered := strings.ToLower(composed)
// Composed a second time after lowering: case-folding a base rune can
// enable a composition that only exists for its lowercase form — "Y" plus
// a combining ring above has no precomposed codepoint, but "y" plus the
// same ring does (U+1E99) — so lowering the already-composed string can
// hand back something that is no longer NFC. Fuzzing found this on
// synthetic input; Vietnamese text never hits it (the language has no such
// case-asymmetric diacritic), but the guarantee is meant to hold for
// whatever a player actually types.
lowered := norm.NFC.String(strings.ToLower(composed))
// strings.Fields splits on every unicode.IsSpace rune, which covers tabs and
// U+00A0 non-breaking spaces as well as ordinary spaces.
@@ -2,7 +2,10 @@ package vietnamese
import (
"errors"
"strings"
"testing"
"golang.org/x/text/unicode/norm"
)
func TestNormalize(t *testing.T) {
@@ -71,6 +74,54 @@ func TestNormalizeEmpty(t *testing.T) {
}
}
// FuzzNormalize guards the one function two processes have to agree on
// byte-for-byte: the corpus builder normalizes with it at build time, the
// server normalizes with it on every submission, and a divergence between
// them would silently unmatch a word the dictionary actually has. The
// invariants are exactly what the package doc promises — NFC, lowercase,
// single-spaced — plus no panic on arbitrary input, and that the returned
// word and syllables always agree with each other.
func FuzzNormalize(f *testing.F) {
seeds := []string{
"pháp luật", "PHÁP LUẬT", "Pháp Luật", " pháp luật ", "pháp luật",
"pháp\tluật", "pháp luật", "pháp\nluật", "vô tuyến điện",
"công nghiệp hóa hiện", "pháp", "", " ", "\t\n", " ",
"ngôn ngữ", "ngôn ngữ",
}
for _, s := range seeds {
f.Add(s)
}
f.Fuzz(func(t *testing.T, raw string) {
word, syllables, err := Normalize(raw)
if err != nil {
if !errors.Is(err, ErrEmpty) {
t.Fatalf("Normalize(%q) returned an unexpected error: %v", raw, err)
}
if word != "" || syllables != nil {
t.Fatalf("Normalize(%q) returned ErrEmpty but non-empty output: %q %v", raw, word, syllables)
}
return
}
if !norm.NFC.IsNormalString(word) {
t.Fatalf("Normalize(%q) = %q is not NFC", raw, word)
}
if strings.ToLower(word) != word {
t.Fatalf("Normalize(%q) = %q is not fully lowercase", raw, word)
}
if strings.Join(strings.Fields(word), " ") != word {
t.Fatalf("Normalize(%q) = %q is not single-spaced and trimmed", raw, word)
}
if len(syllables) == 0 {
t.Fatalf("Normalize(%q) returned no error but no syllables", raw)
}
if strings.Join(syllables, " ") != word {
t.Fatalf("Normalize(%q): word %q does not match its own syllables %v", raw, word, syllables)
}
})
}
func TestHasEnoughSyllables(t *testing.T) {
tests := []struct {
syllables []string
@@ -0,0 +1,2 @@
go test fuzz v1
string("00000000Y̊")
+44
View File
@@ -0,0 +1,44 @@
package wsapi
import (
"errors"
"testing"
"github.com/coder/websocket"
"google.golang.org/protobuf/proto"
)
// FuzzDecode guards the one function that turns arbitrary bytes off the wire
// into a message the rest of the server trusts. The invariant Decode's own
// comment promises is narrower than "never panics" — a non-binary frame is
// always ErrNotBinary, and anything else either fails cleanly or comes back
// as a real message — but a panic here would take the reader goroutine down
// with a client that sent nothing but noise, so the fuzzer is left free to
// find one if it can.
func FuzzDecode(f *testing.F) {
for _, m := range clientVariants() {
raw, err := proto.Marshal(m)
if err != nil {
f.Fatalf("seed marshal: %v", err)
}
f.Add(int(websocket.MessageBinary), raw)
}
f.Add(int(websocket.MessageText), []byte("not protobuf"))
f.Add(int(websocket.MessageBinary), []byte{})
f.Add(int(websocket.MessageBinary), []byte{0xff, 0xff, 0xff, 0x01})
f.Add(0, []byte{0x01, 0x02})
f.Fuzz(func(t *testing.T, typ int, raw []byte) {
msg, err := Decode(websocket.MessageType(typ), raw)
if websocket.MessageType(typ) != websocket.MessageBinary {
if !errors.Is(err, ErrNotBinary) {
t.Fatalf("Decode(typ=%d, ...) err = %v, want ErrNotBinary", typ, err)
}
return
}
if err == nil && msg == nil {
t.Fatalf("Decode(%q) returned neither an error nor a message", raw)
}
})
}
+154
View File
@@ -0,0 +1,154 @@
package wsapi
import (
"io"
"net/http"
"testing"
"time"
noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1"
)
// TestReadyzFlipsOnDrain: /healthz is liveness and never moves; /readyz is
// what a load balancer should stop trusting once the process has started
// shutting down.
func TestReadyzFlipsOnDrain(t *testing.T) {
api, url := newTestServer(t, chainDict(), Config{})
httpURL := "http" + url[len("ws"):]
resp, err := http.Get(httpURL + "/readyz")
if err != nil {
t.Fatalf("GET /readyz: %v", err)
}
_ = resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("before draining: status = %d, want 200", resp.StatusCode)
}
api.StartDraining()
resp, err = http.Get(httpURL + "/readyz")
if err != nil {
t.Fatalf("GET /readyz: %v", err)
}
_ = resp.Body.Close()
if resp.StatusCode != http.StatusServiceUnavailable {
t.Errorf("while draining: status = %d, want 503", resp.StatusCode)
}
resp, err = http.Get(httpURL + "/healthz")
if err != nil {
t.Fatalf("GET /healthz: %v", err)
}
_ = resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Errorf("healthz while draining: status = %d, want 200 (liveness must not move)", resp.StatusCode)
}
}
// TestDrainRefusesNewRooms: a creator past the drain point is told the server
// is restarting, not that it is full — a full room might free up, a draining
// one never will.
func TestDrainRefusesNewRooms(t *testing.T) {
api, url := newTestServer(t, chainDict(), Config{})
api.StartDraining()
c := dial(t, url)
c.hello("Người thử")
c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_CreateRoom{CreateRoom: &noituv1.CreateRoom{}}})
if got := c.await("error").GetError().GetCode(); got != "server_restarting" {
t.Errorf("error code = %q, want server_restarting", got)
}
}
// TestLiveGameCountTracksGamesNotLobbies is the invariant a drain waits on:
// an empty room, or one sitting in its lobby, costs a restart nothing, and
// must not be counted the way a running game is.
func TestLiveGameCountTracksGamesNotLobbies(t *testing.T) {
api, url := newTestServer(t, chainDict(), Config{TurnLimit: 200 * time.Millisecond})
host, guest, _ := pvpLobby(t, url)
if got := api.LiveGameCount(); got != 0 {
t.Fatalf("a lobby with nobody playing counted as %d live games, want 0", got)
}
guest.setReady(true)
host.await("room_state")
host.startGame()
_, waits, _ := awaitLead(t, host, guest)
if got := api.LiveGameCount(); got != 1 {
t.Fatalf("live games = %d after starting, want 1", got)
}
// Nobody submits anything: the lead's own turn runs out the clock, which
// is enough to end a two-seat game.
waits.await("game_over")
settle()
if got := api.LiveGameCount(); got != 0 {
t.Errorf("live games = %d after the game ended, want 0", got)
}
}
// TestVersionEndpoint: GET /version answers with exactly the string the
// server was configured with, in plain text, so a deploy check can diff it
// against what was just built without parsing anything.
func TestVersionEndpoint(t *testing.T) {
_, url := newTestServer(t, chainDict(), Config{Version: "v1.2.3-test"})
httpURL := "http" + url[len("ws"):]
resp, err := http.Get(httpURL + "/version")
if err != nil {
t.Fatalf("GET /version: %v", err)
}
defer func() { _ = resp.Body.Close() }()
body, err := io.ReadAll(resp.Body)
if err != nil {
t.Fatalf("read body: %v", err)
}
if got := string(body); got != "v1.2.3-test" {
t.Errorf("/version = %q, want %q", got, "v1.2.3-test")
}
}
// TestVersionEndpointDefaultsWhenUnset: a test server, like a plain `go run`,
// never sets Config.Version, and /version must still answer something rather
// than an empty body a deploy check would misread as broken.
func TestVersionEndpointDefaultsWhenUnset(t *testing.T) {
_, url := newTestServer(t, chainDict(), Config{})
httpURL := "http" + url[len("ws"):]
resp, err := http.Get(httpURL + "/version")
if err != nil {
t.Fatalf("GET /version: %v", err)
}
defer func() { _ = resp.Body.Close() }()
body, err := io.ReadAll(resp.Body)
if err != nil {
t.Fatalf("read body: %v", err)
}
if got := string(body); got != defaultVersion {
t.Errorf("/version = %q, want the default %q", got, defaultVersion)
}
}
// TestDebugVarsNotOnPublicMux: expvar's counters are process-global, but
// /debug/vars is only ever supposed to answer on the separate address
// NOITU_DEBUG_ADDR names. The public mux this test dials must never carry it.
func TestDebugVarsNotOnPublicMux(t *testing.T) {
_, url := newTestServer(t, chainDict(), Config{})
httpURL := "http" + url[len("ws"):]
resp, err := http.Get(httpURL + "/debug/vars")
if err != nil {
t.Fatalf("GET /debug/vars: %v", err)
}
_ = resp.Body.Close()
if resp.StatusCode != http.StatusNotFound {
t.Errorf("/debug/vars on the public mux = %d, want 404 (it must live on NOITU_DEBUG_ADDR alone)", resp.StatusCode)
}
}
+57 -4
View File
@@ -5,6 +5,7 @@ import (
"crypto/rand"
"errors"
"sync"
"sync/atomic"
"time"
"github.com/tiennm99dev/noitu/server/internal/bot"
@@ -27,6 +28,10 @@ var (
errRoomNotFound = errors.New("wsapi: no such room")
errNoRoomCode = errors.New("wsapi: could not allocate a room code")
errServerFull = errors.New("wsapi: room limit reached")
// errDraining is returned instead of errServerFull once the process has
// started shutting down, so the creator is told to come back rather than
// to wait — a full room fills back up, a draining one never will.
errDraining = errors.New("wsapi: server draining")
)
// defaultMaxRooms bounds live rooms across the whole process when nothing else
@@ -55,6 +60,19 @@ type hub struct {
sessions map[string]*session // by resume token
joinLimiter *keyedLimiter
// draining refuses every new room once set, so a creator is told to come
// back rather than being seated in a room the shutdown below is about to
// end anyway. Read and written from outside the hub's own goroutine (there
// isn't one), so it is atomic rather than mutex-guarded.
draining atomic.Bool
// liveGames counts rooms with a game actually running, as opposed to
// sitting in their lobby. Draining waits for this to reach zero rather
// than for the room count to, because an empty lobby has nothing a
// restart costs and waiting for it would make every deploy sit out
// somebody's abandoned tab.
liveGames atomic.Int64
}
func newHub(ctx context.Context, dict Dictionary, turnLimit, graceFor, idleFor time.Duration, maxRooms int) *hub {
@@ -113,7 +131,7 @@ func (h *hub) resumable(token string) (*session, bool) {
// checked by reading run(), rather than by reasoning about which writes
// happened to precede a `go` statement.
func (h *hub) startBotRoom(s *session, difficulty bot.Difficulty) error {
r, err := h.newRegisteredRoom()
r, err := h.newRegisteredRoom(roomModeBot)
if err != nil {
return err
}
@@ -125,7 +143,7 @@ func (h *hub) startBotRoom(s *session, difficulty bot.Difficulty) error {
// creator is seated, so a client can never receive the code before the seat
// behind it exists.
func (h *hub) createRoom(s *session) error {
r, err := h.newRegisteredRoom()
r, err := h.newRegisteredRoom(roomModePvP)
if err != nil {
return err
}
@@ -149,13 +167,24 @@ func (h *hub) joinRoom(code string, s *session) error {
}
// newRegisteredRoom allocates a code, registers the room and starts it.
func (h *hub) newRegisteredRoom() (*room, error) {
//
// mode is known here, before the room goroutine has processed a single input,
// because it is the hub method called — startBotRoom or createRoom — that
// decides it. Counting the room live from this point rather than from
// handleCreate/handleStartBot is deliberately generous: a room that fails to
// seat its creator still held a goroutine and a registry entry for a moment,
// and the gauge should say so.
func (h *hub) newRegisteredRoom(mode string) (*room, error) {
if h.draining.Load() {
return nil, errDraining
}
code, err := h.reserveCode()
if err != nil {
return nil, err
}
r := newRoom(h, code, h.turnLimit, h.graceFor, h.idleFor)
r := newRoom(h, code, h.turnLimit, h.graceFor, h.idleFor, mode)
// The ceiling is checked under the same lock that registers the room, so
// two creators racing for the last slot cannot both get it.
@@ -168,6 +197,9 @@ func (h *hub) newRegisteredRoom() (*room, error) {
h.rooms[code] = r
h.mu.Unlock()
metrics.roomsTotal.Add(mode, 1)
metrics.roomsLive.Add(mode, 1)
go r.run()
return r, nil
}
@@ -179,6 +211,27 @@ func (h *hub) roomCount() int {
return len(h.rooms)
}
// startDraining stops the hub from seating any new room. Existing rooms are
// untouched here — telling them to stop is the caller's job, once it has
// decided how long to wait for the ones with a game running.
func (h *hub) startDraining() { h.draining.Store(true) }
// isDraining reports whether startDraining has been called, which is what
// /readyz answers with.
func (h *hub) isDraining() bool { return h.draining.Load() }
// liveGameCount is how many rooms currently have a game running, as opposed
// to sitting in their lobby.
func (h *hub) liveGameCount() int64 { return h.liveGames.Load() }
// gameStarted and gameFinished keep liveGameCount accurate. A room calls
// gameStarted when its engine is built and gameFinished exactly once for
// every gameStarted — including when the room is cancelled mid-game rather
// than finishing normally, which is why room.run's teardown carries its own
// call rather than relying on broadcastGameOver alone.
func (h *hub) gameStarted() { h.liveGames.Add(1) }
func (h *hub) gameFinished() { h.liveGames.Add(-1) }
// evict removes a finished room. Called by the room goroutine as it exits, so
// a code is reusable the moment its game is done.
func (h *hub) evict(code string) {
+1 -1
View File
@@ -37,7 +37,7 @@ func TestTypedWordIsSanitizedBeforeItIsEchoed(t *testing.T) {
lead, waits, start := pvpGame(t, url)
// A zero-width joiner and a bidi override inside an otherwise legal word.
lead.submit("b‍ ‮c", start.GetTurnSeq())
lead.submit("b\u200d \u202ec", start.GetTurnSeq())
played := waits.await("turn_update").GetTurnUpdate().GetPlayed()
if played.GetWord() != "b c" {
+82
View File
@@ -0,0 +1,82 @@
package wsapi
import "expvar"
// metricSet is the process's answer to "how is the game actually being
// played" — the question plans/reports/brainstormer-260921-0016 names as
// unanswerable with nothing but a dozen scattered slog calls. It is exposed
// at GET /debug/vars, but only when the operator opts in by setting
// NOITU_DEBUG_ADDR to a separate listen address: see cmd/noitu-server/main.go.
// Nothing here is read anywhere in the game logic — a counter that fed a
// decision back into play would need its own tests for correctness, not just
// for visibility.
//
// expvar's registry is a process-wide singleton by design (see the stdlib
// package doc), so these are package-level rather than server-instance
// fields: a second *Server in the same process — every wsapi test — shares
// one set of counters rather than fighting to register a second one under the
// same name.
type metricSet struct {
connectionsOpen *expvar.Int
connectionsTotal *expvar.Int
// roomsLive and roomsTotal are keyed "bot" or "pvp".
roomsLive *expvar.Map
roomsTotal *expvar.Map
// gamesStarted and gamesFinished are keyed the same way.
gamesStarted *expvar.Map
gamesFinished *expvar.Map
wordsSubmitted *expvar.Int
wordsAccepted *expvar.Int
// wordsRejected is keyed by game.RejectReason.String(), which is also
// what the word_rejected log line's reason field carries — one reason
// vocabulary rather than two.
wordsRejected *expvar.Map
// eliminations is keyed by the wire GameEndReason string, since that is
// the one place "the reconnect window ran out" is told apart from "they
// resigned" — both are game.EndResigned to the engine underneath.
eliminations *expvar.Map
chatLines *expvar.Int
// joinsRefused is keyed by the same UI error code the client is sent —
// "too_many_attempts", "room_not_found" or "room_full" — so a reader
// checking this counter against the frontend copy is checking against the
// same string, not a second name for it.
joinsRefused *expvar.Map
// botMoves is keyed by bot.Difficulty.String().
botMoves *expvar.Map
resumesAttempted *expvar.Int
resumesSucceeded *expvar.Int
}
// metrics is the one instance every call site writes through. Built at
// package init rather than lazily, so every counter exists — reading at zero
// — from the moment the process starts, whether or not NOITU_DEBUG_ADDR is
// ever set.
var metrics = newMetricSet()
func newMetricSet() *metricSet {
return &metricSet{
connectionsOpen: expvar.NewInt("noitu_connections_open"),
connectionsTotal: expvar.NewInt("noitu_connections_total"),
roomsLive: expvar.NewMap("noitu_rooms_live"),
roomsTotal: expvar.NewMap("noitu_rooms_total"),
gamesStarted: expvar.NewMap("noitu_games_started"),
gamesFinished: expvar.NewMap("noitu_games_finished"),
wordsSubmitted: expvar.NewInt("noitu_words_submitted"),
wordsAccepted: expvar.NewInt("noitu_words_accepted"),
wordsRejected: expvar.NewMap("noitu_words_rejected"),
eliminations: expvar.NewMap("noitu_eliminations"),
chatLines: expvar.NewInt("noitu_chat_lines"),
joinsRefused: expvar.NewMap("noitu_joins_refused"),
botMoves: expvar.NewMap("noitu_bot_moves"),
resumesAttempted: expvar.NewInt("noitu_resumes_attempted"),
resumesSucceeded: expvar.NewInt("noitu_resumes_succeeded"),
}
}
+70
View File
@@ -0,0 +1,70 @@
package wsapi
import (
"expvar"
"testing"
noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1"
)
// mapValue reads one key of an expvar.Map as an int64, 0 for a key never
// written. The counters under test are shared package state — every wsapi
// test that ever ran in this process wrote to them — so every assertion below
// is a delta across one call, never an absolute value.
func mapValue(m *expvar.Map, key string) int64 {
v := m.Get(key)
if v == nil {
return 0
}
iv, ok := v.(*expvar.Int)
if !ok {
return 0
}
return iv.Value()
}
// TestMetricsCountWordSubmissions covers both directions handleSubmit can
// go: an engine rejection counted under its game.RejectReason string, and an
// accepted move counted separately. wordsSubmitted rises either way, which is
// what makes it the denominator for an acceptance rate.
func TestMetricsCountWordSubmissions(t *testing.T) {
_, url := newTestServer(t, chainDict(), Config{})
c := dial(t, url)
c.hello("Người thử")
c.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_StartBotGame{
StartBotGame: &noituv1.StartBotGame{Difficulty: noituv1.Difficulty_DIFFICULTY_EASY},
}})
started := c.await("game_started").GetGameStarted()
submittedBefore := metrics.wordsSubmitted.Value()
acceptedBefore := metrics.wordsAccepted.Value()
rejectedBefore := mapValue(metrics.wordsRejected, "not in dictionary")
c.submit("khong co", started.GetTurnSeq())
c.await("move_rejected")
if got, want := metrics.wordsSubmitted.Value(), submittedBefore+1; got != want {
t.Errorf("wordsSubmitted after a rejection = %d, want %d", got, want)
}
if got, want := metrics.wordsAccepted.Value(), acceptedBefore; got != want {
t.Errorf("wordsAccepted moved on a rejection: %d, want unchanged at %d", got, want)
}
if got, want := mapValue(metrics.wordsRejected, "not in dictionary"), rejectedBefore+1; got != want {
t.Errorf(`wordsRejected["not in dictionary"] = %d, want %d`, got, want)
}
submittedBefore = metrics.wordsSubmitted.Value()
acceptedBefore = metrics.wordsAccepted.Value()
// The turn never moved on the rejection above, so the same turn_seq still
// answers the current position.
c.submit("b c", started.GetTurnSeq())
c.await("turn_update")
if got, want := metrics.wordsSubmitted.Value(), submittedBefore+1; got != want {
t.Errorf("wordsSubmitted after an acceptance = %d, want %d", got, want)
}
if got, want := metrics.wordsAccepted.Value(), acceptedBefore+1; got != want {
t.Errorf("wordsAccepted = %d, want %d", got, want)
}
}
+2 -2
View File
@@ -464,8 +464,8 @@ func TestSeveralReconnectWindowsRunAtOnce(t *testing.T) {
clients, _ := roomOf(t, url, 3)
host, second, third := clients[0], clients[1], clients[2]
second.conn.CloseNow()
third.conn.CloseNow()
_ = second.conn.CloseNow()
_ = third.conn.CloseNow()
// Both seats are held first, then both are freed. The owner is the one
// still here to watch it happen.
+64
View File
@@ -0,0 +1,64 @@
package wsapi
import (
"strings"
"testing"
"unicode"
"unicode/utf8"
)
// FuzzSanitizeText guards the filter every player-supplied string crosses
// before it is shown to a stranger: a nickname, a chat line and a submitted
// word all go through it. The invariants are the ones sanitizeText's own
// comment promises — no control or format character survives, no rune count
// over the cap, no run of combining marks longer than the cap allows — plus
// the one every caller depends on implicitly: it never panics on arbitrary
// input, however malformed.
//
// Bounds are fixed at the nickname's rather than fuzzed themselves: every real
// call site passes a compile-time constant, so a bound the fuzzer chooses
// would be exercising a contract sanitizeText was never asked to keep.
func FuzzSanitizeText(f *testing.F) {
seeds := []string{
"Minh",
"Nguyễn Thuý",
"",
" \t\n ",
"Mi\x00nh\x07",
"Mi\u200bnh\u200d",
"Minh\u202e",
" Minh Nguyen ",
"Minh\nNguyen",
strings.Repeat("a", 40),
strings.Repeat("ữ", maxNicknameRunes),
"Minh" + strings.Repeat(string(stackingMark), 50),
}
for _, s := range seeds {
f.Add(s)
}
f.Fuzz(func(t *testing.T, raw string) {
got := sanitizeText(raw, maxNicknameRunes, maxNicknameMarks)
if n := utf8.RuneCountInString(got); n > maxNicknameRunes {
t.Fatalf("sanitizeText(%q) kept %d runes, want at most %d", raw, n, maxNicknameRunes)
}
marks := 0
for _, r := range got {
switch {
case unicode.IsControl(r), unicode.Is(unicode.Cf, r):
t.Fatalf("sanitizeText(%q) = %q kept a control/format character %U", raw, got, r)
case !unicode.IsPrint(r):
t.Fatalf("sanitizeText(%q) = %q kept a non-printing character %U", raw, got, r)
case unicode.Is(unicode.Mn, r), unicode.Is(unicode.Me, r):
marks++
if marks > maxNicknameMarks {
t.Fatalf("sanitizeText(%q) = %q stacked %d combining marks, want at most %d", raw, got, marks, maxNicknameMarks)
}
default:
marks = 0
}
}
})
}
+87 -1
View File
@@ -6,12 +6,23 @@ import (
"log/slog"
"math/rand/v2"
"slices"
"sync/atomic"
"time"
noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1"
"github.com/tiennm99dev/noitu/server/internal/bot"
"github.com/tiennm99dev/noitu/server/internal/dictionary"
"github.com/tiennm99dev/noitu/server/internal/game"
"github.com/tiennm99dev/noitu/server/internal/vietnamese"
)
// roomModeBot and roomModePvP are the two values a room's mode ever takes.
// They double as the label under which every mode-keyed metric and the
// word_rejected log line group their counts, so a reader checking one against
// the other is checking against the same string everywhere.
const (
roomModeBot = "bot"
roomModePvP = "pvp"
)
// botPlayerID is the seat the bot occupies. It is a normal player to the
@@ -228,6 +239,20 @@ type room struct {
graceFor time.Duration
idleFor time.Duration
// mode is roomModeBot or roomModePvP, fixed at creation. It is the label
// every mode-keyed metric and the word_rejected log line use, kept as its
// own field rather than re-derived from strategy == nil so the hub can set
// it before the room goroutine has seated anyone or built an engine.
mode string
// liveCounted mirrors whether this room's game is the one hub.liveGames is
// currently counting. Atomic rather than plain, because drain reads
// hub.liveGameCount() from outside the room goroutine while this flips on
// the goroutine itself; the CompareAndSwap in run's teardown is what
// guarantees exactly one hub.gameFinished() per hub.gameStarted() even when
// the room is cancelled mid-game instead of finishing normally.
liveCounted atomic.Bool
seats [maxPlayers]*seat
// owner is the seat that may start a game and free the other one. It is a
@@ -280,7 +305,7 @@ type Dictionary interface {
Meanings(word string) []dictionary.Sense
}
func newRoom(h *hub, code string, turnLimit, graceFor, idleFor time.Duration) *room {
func newRoom(h *hub, code string, turnLimit, graceFor, idleFor time.Duration, mode string) *room {
if idleFor <= 0 {
idleFor = defaultIdleWindow
}
@@ -295,6 +320,7 @@ func newRoom(h *hub, code string, turnLimit, graceFor, idleFor time.Duration) *r
turnLimit: turnLimit,
graceFor: graceFor,
idleFor: idleFor,
mode: mode,
}
}
@@ -332,6 +358,15 @@ func (r *room) send(msg any) bool {
func (r *room) run() {
defer r.cancel()
defer r.hub.evict(r.code)
defer metrics.roomsLive.Add(r.mode, -1)
// Catches a room cancelled with a game still running — drain forcing the
// last stragglers closed, or a shutdown mid-game — which never reaches
// broadcastGameOver's own decrement.
defer func() {
if r.liveCounted.CompareAndSwap(true, false) {
r.hub.gameFinished()
}
}()
// Whatever ended the room — everybody leaving, the idle window, a server
// shutdown — the connections still seated in it must stop pointing here.
// A session that keeps a dead room would answer every later action with
@@ -570,6 +605,7 @@ func (r *room) handleStartBot(m startBotInput) {
func (r *room) handleJoin(m joinInput) {
free := r.freeSeat()
if free < 0 || !r.occupied() {
metrics.joinsRefused.Add("room_full", 1)
m.sess.send(errorMsg("room_full"))
return
}
@@ -770,6 +806,10 @@ func (r *room) beginGame() error {
}
}
metrics.gamesStarted.Add(r.mode, 1)
r.hub.gameStarted()
r.liveCounted.Store(true)
state := r.engine.Snapshot()
for _, s := range r.seats {
r.sendGameStarted(s, state)
@@ -817,8 +857,11 @@ func (r *room) handleSubmit(m submitInput) {
// The rejection carries the server's sequence, not the client's stale one,
// so the client can resynchronise from the refusal instead of having to
// wait for the next turn update to discover where the game actually is.
metrics.wordsSubmitted.Add(1)
if m.turnSeq != r.turnSeq {
r.sendTo(m.player, moveRejectedMsg(noituv1.RejectReason_REJECT_REASON_NOT_YOUR_TURN, m.word, r.turnSeq))
r.recordRejection(game.ReasonNotYourTurn, m.word)
return
}
@@ -832,11 +875,13 @@ func (r *room) handleSubmit(m submitInput) {
move, reason := r.engine.Submit(m.player, word, time.Now())
if reason != game.ReasonNone {
r.sendTo(m.player, moveRejectedMsg(RejectReason(reason), word, m.turnSeq))
r.recordRejection(reason, word)
// A rejection for an expired turn also took this player out of the
// game, and everybody has to be told which.
r.applyEliminations(before)
return
}
metrics.wordsAccepted.Add(1)
// An accepted move never ends a game: a dead end is left for whoever
// inherits it, which is what Submit's own comment explains.
@@ -845,6 +890,38 @@ func (r *room) handleSubmit(m submitInput) {
r.maybeScheduleBot()
}
// recordRejection counts one rejected submission and logs it at Info.
//
// This is the corpus feedback loop the improvement report calls the input to
// every decision about the dictionary: which words players actually type that
// the game does not accept, and why. The word logged is never the raw typed
// text — it is normalized the same way the engine would have matched it
// (NFC, lowercase, single-spaced) and capped, so the line is useful for corpus
// review without ever logging what a player literally typed into the box.
func (r *room) recordRejection(reason game.RejectReason, raw string) {
metrics.wordsRejected.Add(reason.String(), 1)
// sanitizeText first: raw may be the untouched client payload (the
// not-your-turn path never reaches the sanitizer below it in
// handleSubmit), and Normalize alone does not drop control or format
// characters.
word, _, err := vietnamese.Normalize(sanitizeText(raw, maxWordRunes, maxNicknameMarks))
if err != nil {
word = ""
}
if runes := []rune(word); len(runes) > maxWordRunes {
word = string(runes[:maxWordRunes])
}
slog.Info("word_rejected",
"reason", reason.String(),
"word", word,
"link", r.engine.Current(),
"mode", r.mode,
"room", r.code,
)
}
// handleBotMove applies what the worker chose.
func (r *room) handleBotMove(m botMoveInput) {
if r.engine == nil || r.engine.Over() {
@@ -855,6 +932,7 @@ func (r *room) handleBotMove(m botMoveInput) {
if m.turnSeq != r.turnSeq || r.engine.Turn() != botPlayerID {
return
}
metrics.botMoves.Add(r.strategy.Difficulty().String(), 1)
now := time.Now()
before := r.mark()
@@ -1032,6 +1110,7 @@ func (r *room) broadcastElimination(id game.PlayerID, suggestions []string) {
name = out.nickname
}
reason := r.wireEndReason(id)
metrics.eliminations.Add(reason.String(), 1)
for _, s := range r.seats {
if s == nil || s.sess == nil {
@@ -1067,6 +1146,11 @@ func (r *room) wireEndReason(p game.PlayerID) noituv1.GameEndReason {
// broadcastGameOver reports the result from each seat's point of view.
func (r *room) broadcastGameOver(state game.State) {
metrics.gamesFinished.Add(r.mode, 1)
if r.liveCounted.CompareAndSwap(true, false) {
r.hub.gameFinished()
}
// The reason the game ended is the reason the last player went out, which
// with two seats is the only elimination there was.
reason := noituv1.GameEndReason_GAME_END_REASON_UNSPECIFIED
@@ -1233,6 +1317,7 @@ func (r *room) handleResume(m resumeInput) {
// Accepted. Only now is the old connection finished: its token is spent and
// its socket is either gone or about to be, and leaving it registered would
// let a third connection claim the same seat.
metrics.resumesSucceeded.Add(1)
m.sess.attach(r, string(m.player))
if m.prior != nil {
m.sess.hub.unregister(m.prior.resumeToken)
@@ -1304,6 +1389,7 @@ func (r *room) handleChat(m chatInput) {
if len(r.chat) > chatHistoryLimit {
r.chat = r.chat[len(r.chat)-chatHistoryLimit:]
}
metrics.chatLines.Add(1)
for _, s := range r.seats {
if s == nil || s.sess == nil {
+54 -4
View File
@@ -40,8 +40,16 @@ type Config struct {
// default. MaxConnections caps open sockets the same way.
MaxRooms int
MaxConnections int
// Version is what GET /version answers and what the startup log line
// carries. Empty falls back to defaultVersion, which is what a plain
// `go run` or a test server — nothing built with -ldflags — reports.
Version string
}
// defaultVersion is what an unstamped build reports: a local `go run` or a
// test server, neither of which passes -X main.version through -ldflags.
const defaultVersion = "dev"
// defaultMaxConnections bounds open WebSockets when nothing else is set. Each
// one is three goroutines and an outbox; the number is generous for one
// binary and small next to what the host can hold.
@@ -49,10 +57,11 @@ const defaultMaxConnections = 2000
// Server wires the hub to an HTTP mux.
type Server struct {
hub *hub
mux *http.ServeMux
cancel context.CancelFunc
cfg Config
hub *hub
mux *http.ServeMux
cancel context.CancelFunc
cfg Config
version string
proxies []netip.Prefix
maxConns int64
@@ -63,11 +72,17 @@ type Server struct {
func NewServer(ctx context.Context, dict Dictionary, cfg Config) *Server {
ctx, cancel := context.WithCancel(ctx)
version := cfg.Version
if version == "" {
version = defaultVersion
}
s := &Server{
hub: newHub(ctx, dict, cfg.TurnLimit, cfg.GraceFor, cfg.IdleFor, cfg.MaxRooms),
mux: http.NewServeMux(),
cancel: cancel,
cfg: cfg,
version: version,
proxies: parsePrefixes(cfg.TrustedProxies),
maxConns: int64(cfg.MaxConnections),
}
@@ -80,6 +95,23 @@ func NewServer(ctx context.Context, dict Dictionary, cfg Config) *Server {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("ok"))
})
// /readyz is a readiness check, distinct from /healthz above: it fails
// while draining even though the process is still alive and still
// finishing the games it already has, which is exactly the state a load
// balancer should stop sending new traffic to.
s.mux.HandleFunc("GET /readyz", func(w http.ResponseWriter, _ *http.Request) {
if s.hub.isDraining() {
http.Error(w, "draining", http.StatusServiceUnavailable)
return
}
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("ready"))
})
s.mux.HandleFunc("GET /version", func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(s.version))
})
s.mountStatic()
go s.sweepLimiters(ctx)
@@ -94,6 +126,20 @@ func (s *Server) Shutdown() {
s.cancel()
}
// StartDraining stops the server from seating any new room and flips
// /readyz to unhealthy. Existing games are untouched — a caller decides
// separately, via LiveGameCount, how long to wait before calling Shutdown.
func (s *Server) StartDraining() { s.hub.startDraining() }
// RoomCount is how many rooms — lobbies and running games alike — are live
// right now, for the log lines a drain or shutdown writes.
func (s *Server) RoomCount() int { return s.hub.roomCount() }
// LiveGameCount is how many of those rooms have a game actually running.
// Draining waits for this, not for RoomCount, because an empty lobby has
// nothing a restart costs.
func (s *Server) LiveGameCount() int64 { return s.hub.liveGameCount() }
func (s *Server) handleWS(w http.ResponseWriter, r *http.Request) {
// Refused before the upgrade, so a client that is over the line is told
// so in HTTP terms it can read, and never costs a socket.
@@ -114,6 +160,10 @@ func (s *Server) handleWS(w http.ResponseWriter, r *http.Request) {
return
}
metrics.connectionsOpen.Add(1)
metrics.connectionsTotal.Add(1)
defer metrics.connectionsOpen.Add(-1)
sess := newSession(s.hub.ctx, conn, s.hub, s.clientIP(r))
sess.run()
+9
View File
@@ -453,10 +453,12 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error {
case *noituv1.ClientMessage_JoinRoom:
if !s.hub.joinLimiter.allow(s.remoteIP, time.Now()) {
metrics.joinsRefused.Add("too_many_attempts", 1)
s.send(errorMsg("too_many_attempts"))
return nil
}
if err := s.hub.joinRoom(p.JoinRoom.GetRoomCode(), s); err != nil {
metrics.joinsRefused.Add("room_not_found", 1)
s.send(errorMsg("room_not_found"))
}
@@ -519,6 +521,12 @@ func roomCreateError(sessionID string, err error) *noituv1.ServerMessage {
if errors.Is(err, errServerFull) {
return errorMsg("server_full")
}
if errors.Is(err, errDraining) {
// The same key Shutdown sends to everyone already seated: a room
// refused for this reason will not open a moment later the way a full
// one might, so the client is told the same thing either way.
return errorMsg("server_restarting")
}
slog.Error("open room", "session", sessionID, "err", err)
return errorMsg("room_start_failed")
}
@@ -583,6 +591,7 @@ func (s *session) handleHello(h *noituv1.Hello) error {
// Welcome and then silence has nothing to render and no reason to stop
// waiting.
func (s *session) resumeFrom(prior *session) {
metrics.resumesAttempted.Add(1)
r, id := prior.currentRoom()
if r == nil {
s.send(errorMsg("game_already_over"))
+4 -2
View File
@@ -355,7 +355,9 @@ func TestTheFirstTurnIsDrawn(t *testing.T) {
// Started here rather than over a pair of sockets: a series long enough to
// tell a draw from a fixed lead is far more starts than a lobby's rate
// limiter allows, and none of what is being checked is on the wire.
r := &room{dict: chainDict(), turnLimit: time.Second}
// beginGame reports to the hub's live-game gauge, so this hand-built room
// needs one even though nothing here reads it back.
r := &room{hub: &hub{}, dict: chainDict(), turnLimit: time.Second}
r.seats[0] = &seat{id: "p1"}
r.seats[1] = &seat{id: "p2"}
@@ -383,7 +385,7 @@ func TestABotGameOpensWithTheHuman(t *testing.T) {
if err != nil {
t.Fatalf("bot.New: %v", err)
}
r := &room{dict: chainDict(), turnLimit: time.Second, strategy: strategy}
r := &room{hub: &hub{}, dict: chainDict(), turnLimit: time.Second, strategy: strategy}
r.seats[0] = &seat{id: "p1"}
r.seats[1] = &seat{id: botPlayerID}