feat: one error object with a code, message and details map - #338
Merged
Conversation
asobi reported failures in four incompatible dialects: REST returned a
flat #{error => Binary}, WebSocket returned #{reason => Binary}, some
routes returned a bare status with no body at all, and the admin surface
added more. Nothing could branch on a failure programmatically.
Add asobi_error: one object, #{error => #{code, message, details}}, with
a closed set of namespaced codes that carry their HTTP status, and a Nova
return handler so a controller states the failure rather than the number.
details is always a map so no client needs a null branch.
Convert the WebSocket error path (additive - `reason` is unchanged and
still sent) and asobi_storage_controller as the worked example. The other
73 routes keep their existing shapes and are converted in a follow-up.
Taure
force-pushed
the
feat/error-object
branch
from
August 4, 2026 00:18
7e9efcd to
5f285ff
Compare
Taure
added a commit
that referenced
this pull request
Aug 4, 2026
* feat: finish the shared error object rollout across every route asobi_error shipped in #338 with one worked example: 7 of 64 REST routes. Every other controller still answered in its own dialect - a flat so nothing outside /saves and /storage could branch on a failure. Convert all of them, including the two controllers that live outside src/controllers/ (players, votes), the ops read plane, asobi_ops_auth's deny body, asobi_auth_tokens, and the three plugins that write their own 4xx/5xx body without reaching a controller. Add the codes those failures need: one namespace per domain, and a single code plus details.reason wherever the reason is not ours to publish - an identity provider's rejection, a store's receipt diagnostic, a game script's refusal to create a world. Additive except for `error` itself, which was already a string and is now the object. Every other top-level key a route sent survives untouched - `fields`, `errors`, `retry_after`, `field`, `order`, `reason` - and is repeated in `details` so new code reads one place. asobi_error:legacy/2 is the one funnel for that. Statuses are unchanged. Add asobi_error_contract_tests: it reads the abstract code of every module in the application and fails on a flat error body, a bodiless 4xx/5xx, a code outside the closed set, or a code that is not a literal - which is what stops a client- or script-supplied string from ever becoming one. The scan derives its own module set, so the next controller is covered whether or not anyone remembers this test exists. * fix(docs): teach the error-drift guard the shared error object The guard extracted {json, Status, #{}, #{error => ~"code"}} from a controller. Since this PR converts every controller to {asobi_error, Code}, it matched nothing on the source side and reported all sixteen documented guest codes as undocumented - correctly failing, for the right reason. A controller no longer names a status, so the guard resolves code -> status from asobi_error's ?CODES table, the same way asobi_error:status/1 does. A code with no entry reports 500 rather than being dropped, so an undefined code cannot hide from the guard by being absent from the table. Both regexes now accept the dot in a namespaced code. The guest table in guides/authentication.md is rewritten to the codes the controller actually returns, and its column header says error.code rather than error, which is where a client now reads it. Three codes it never documented are now listed: auth.registration_closed, auth.password_registration_disabled and auth.username_taken, all reachable on the upgrade path.
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.
What was broken
asobi reported failures in four incompatible dialects, so nothing could branch on a failure programmatically:
#{error => Binary}src/controllers/asobi_storage_controller.erl:43(pre-change)asobi_storage_controller.erl:117,119,121,125(pre-change) - a 403 and a 404 that say nothing#{error => Binary, current_version => N}asobi_storage_controller.erl:52(pre-change) - ad-hoc extra keys at the top level#{reason => Binary}src/ws/asobi_ws_handler.erl:647(pre-change), 38 call sitesNothing was namespaced (
not_foundfrom storage andnot_foundfrom a world were the same string), nothing was enumerated, and a client could not tell an empty-bodied 403 from an empty-bodied 500.What changed
src/asobi_error.erl(new) - one object:{"error": {"code": "storage.not_found", "message": "No object exists at this collection and key.", "details": {}}}codeis the contract: machine-readable, namespaced by domain (storage.,save.,match.,world.,chat.,matchmaker.) or bare when cross-cutting (rate_limited,internal). The set is closed -codes/0enumerates it. A string supplied by a client or a Lua game script can never become a code; it lands indetailsinstead.messageis prose for a human reading a log, explicitly not for parsing.detailsis always a map,#{}when empty, so no client needs a null branch.Nova return handler -
asobi_error:handle/3, registered inasobi_app:start/2. Controllers return:{asobi_error, ~"storage.not_found"} {asobi_error, ~"save.version_conflict", #{current_version => 4}} {asobi_error, Status, Code, Details} %% explicit status when the code does not imply itIt delegates the encode to
nova_basic_handler:handle_json/3, so content-type, status, and the configuredjson_lib(OTPjson) stay exactly as every other JSON response. An undefined code returns 500 and logsundefined_error_coderather than failing silently.WebSocket (
src/ws/asobi_ws_handler.erl) - all 38 error sites now funnel through oneencode_error/2,3. This is additive:reasonis unchanged byte-for-byte and still sent, anderroris added next to it.asobi_error:from_ws_reason/1maps the legacy reason onto a code, soreason: "invalid_token"now also carriescode: "unauthenticated". An unmapped reason becomesws.request_failedwith the raw string indetails.reason.REST worked example (
asobi_storage_controller.erl) - all 13 failure returns converted, including four that previously had no body at all. The other 73 routes are untouched and keep their shapes.Also updated:
priv/protocol/fixtures/error.json(SDK dispatch-test ground truth - it would otherwise pin a shape the server no longer sends),guides/rest-api.md(new Errors section, with an explicit rollout note that only/savesand/storagereturn this shape today),guides/websocket-protocol.md(newerrorframe section).Follow-up, deliberately not in this PR
Converting the remaining 73 routes across the other 15 controllers. Per the brief the shape matters more than the coverage here, since this is the shared prerequisite for both the ops API and the RPC wire. The old shapes keep working until then, and
guides/rest-api.mdsays so in the docs rather than leaving clients to discover it.Two things that shape review of that follow-up:
erroras a string, because the new object occupies the sameerrorkey. There is no additive option for REST the way there is for WebSocket. HTTP status codes are unchanged throughout, so a client that branches on status is unaffected.{status, N}→ error-object conversions in storage turn previously empty-bodied responses into JSON. That is the point (a 403 you can branch on), but it is a body where there was none.What the tests assert
test/asobi_error_tests.erl(24 tests, new):code,message,details- anddetailsserialises as{}, nevernull.ws.request_failedwith the raw string indetailsand cannot mint a code.priv/protocol/fixtures/error.jsonmatches whatfrom_ws_reason/1actually emits, so the SDK fixture cannot drift from the server.content-type: application/json.test/asobi_ws_handler_tests.erl(5 new tests) - drives real frames throughwebsocket_handle/2:reasonanderrorwith the right code and an emptydetailsmap.cidstill round-trips on an error reply.reason: "invalid_token"withcode: "unauthenticated"- the case that proves the legacy string survives while the code deliberately differs from it.test/asobi_storage_SUITE.erl(4 assertions strengthened) - end to end against Postgres:code: "save.version_conflict"anddetails.current_version == 2(the flat shape putcurrent_versionat the top level next toerror).code: "storage.value_too_large".Checks
rebar3 fmt --checkrebar3 xrefrebar3 eunitrebar3 dialyzerrebar3 ex_docrebar3 ct --suite=test/asobi_storage_SUITEWhat I could not verify
rebar3 ct --suite=test/asobi_ws_SUITEfails 8/8 on this machine. This is pre-existing and environmental, not this change: I reproduced it identically on a cleanorigin/maintree (git stash, re-run, same 8 failures). The CT log shows{listen_error, nova_listener, eaddrinuse}on the nova port plus Postgres53300 sorry, too many clients already- another node on this host is holding port 8082 and the connection pool.asobi_storage_SUITEpassed 14/14 earlier in the same session when the port was free, and re-running it after the contention started reproduced the sameeaddrinuse. CI should run both cleanly; worth a look at the CI result before merge rather than trusting my local run.I have also not verified the 7 client SDKs against the updated
priv/protocol/fixtures/error.json- they live in separate repos. The fixture change is additive (thereasonkey they dispatch on is untouched), but an SDK doing an exact-match assertion on the whole payload would need a bump.