Skip to content

Keep the server up when a request fails and accept trailing-slash paths - #4

Merged
valzargaming merged 1 commit into
mainfrom
claude/request-robustness
Nov 6, 2026
Merged

valzargaming merged 1 commit into
mainfrom
claude/request-robustness

Conversation

@valzargaming

Copy link
Copy Markdown
Member

Requested by Val · project thread

Problem

  • Any exception thrown inside an endpoint (storage error, PDO failure, bad JSON) escaped handleReact. ReactPHP's HttpServer then emitted error, and the handler called logError($e, true), which closes the listening socket. One failing request took the whole verifier offline until restart. Reproduced with a state whose getVerifyList() throws.
  • Civilizationbot's default verify_url is http://valzargaming.com:8080/verified/. Endpoints are matched by exact path, so /verified/ returned 404 (reproduced through handleReact). Only the in-process bot path, which bypasses routing, was unaffected.
  • getSS14State() read $this->SS14state; the property is $ss14state, so it always returned null.
  • handleResource() had explode($sender, ':') with the arguments swapped.
  • Token comparison used !==; now hash_equals.

Change

  • handleReact catches Throwable, logs a warning and answers 500. The HTTP server error handler is no longer fatal. logError accepts Throwable, which is what the handler already passed.
  • Server::normalizePath() collapses trailing slashes in route(), handleEndpoint() and getEndpoint(); / still aliases /verified.
  • Unknown paths log at debug level instead of constructing an exception per request.

Compatibility with Civilizationbot

No change to the fields, statuses, arrayToRequestString format, handleEndpoint signature or getState()/getEndpoint() return types. The bot's tests/verifier-integration.php call pattern was replayed against this branch (combined with the other three open PRs) and passed. After merge the bot's composer.json pin (vzgcoders/verifier-server dev-main#838750d…) needs bumping.

Verification

php scripts/lint.php                                   # Parsed 29 PHP files with PHP 8.3.6
./vendor/bin/phpunit --bootstrap vendor/autoload.php tests/isolated   # OK (10 tests, 60 assertions)

New tests/isolated/RequestRobustnessTest.php covers the 500-not-close behaviour, trailing slashes on GET/POST/handleEndpoint, the unauthorized path and the SS14 accessor.


Generated by Claude Code

A thrown exception inside any endpoint used to escape handleReact; the
HttpServer error handler then called logError(fatal) and closed the
listener, so one failing request took the verifier offline. Requests now
get a 500 and a warning log instead, and the error handler is no longer
fatal. logError accepts Throwable like the handler already passed it.

Civilizationbot's default verify_url ends with a slash
(http://valzargaming.com:8080/verified/), which routed to 404 because
endpoints are matched by exact path. Paths are now normalised before
lookup in route(), handleEndpoint() and getEndpoint().

Also: compare tokens with hash_equals, fix the explode() argument order
in the stream-socket handler, and fix getSS14State() reading a property
with the wrong case (it always returned null).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016WTP2S4qKEFT8a6a6yFEL4
Copilot AI balanced review requested due to automatic review settings November 6, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@valzargaming valzargaming self-assigned this Nov 6, 2026

Copy link
Copy Markdown
Member Author

2026-11-06: head 3e103e3. Local: php scripts/lint.php (29 files) and ./vendor/bin/phpunit --bootstrap vendor/autoload.php tests/isolated OK (10 tests, 60 assertions). CI: https://github.com/Valgorithms/Verifier-server/actions/runs/42212407360 passed on PHP 8.2 to 8.5. Not verified: Civilizationbot's tests/verifier-integration.php against a bumped pin, live run.


Generated by Claude Code

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.

3 participants