refactor(server): split dispatch into handler modules and standardize error formatting#160
Merged
Merged
Conversation
Part of #132. fetch_choice_def! and valid_sub_choices lived in the CLI Serializer — validation logic in a module whose stated purpose is rendering, parked there only to keep Server within code-health thresholds. Like award application and progression resolution before them (#131), they are rules any frontend needs, so they move to Characters.Advancement with delegating wrappers on Characters. Their shared option-derivation helper moves too, as public Characters.sub_choice_options/2 (signature flipped to (system, choice_def); the choice type is read from the definition rather than passed alongside it). The Serializer's pending-sub-choice rendering now calls it instead of a private duplicate, so the option list shown to users and the option list validated against can no longer drift. The Serializer moduledoc now states its actual responsibility (rendering domain objects for the JSON protocol) instead of apologizing for its existence.
Part of #132. Three error idioms coexisted in the server: raise-and-rescue with Exception.message, per-command format_* functions, and raw `inspect(reason)` interpolation that leaked Elixir terms to the Rust frontend (spells, inventory.add, inventory.set). The same condition could also produce different messages depending on the path — e.g. :value_must_be_integer rendered differently for awards vs progressions, and a missing pending character had two different messages (characters.save vs fetch_pending!). Handlers now return {:ok, data, state} | {:error, message}; handle_command wraps those into protocol envelopes, so ok/error map construction happens in exactly one place. All domain {:error, reason} terms are rendered by the new Server.Errors module — one clause per documented library reason, covering awards, progression choices, preparation/activation, and inventory-item field validation. Its inspect fallback exists only as a defensive backstop for reasons not yet given a clause. Message unifications (no test asserted on the old texts): - :value_must_be_integer -> "value must be an integer" everywhere - :missing_effect_target -> "no effect_target configured" everywhere - characters.save on an unknown temp_id now goes through fetch_pending!, same message as the build handlers - characters.spells errors are formatted (previously raw inspected tuples like {:unknown_inventory_type, ...}) - inventory.add/set failures render the field-validation reason ("value must be of type boolean") instead of inspecting it Handler-detected conditions with request context (unknown command, unknown activate verb, character not found by slug) keep handler-authored messages; the formatter owns library reasons.
Part of #132. server.ex was a 660-line module of 26 handle/2 clauses; clause order was load-bearing (the two resolve_choice forms disambiguated by pattern), section comments had drifted (the scoped resolve_choice sat under "--- Inventory ---", 227 lines from its progression sibling), and every new command grew the pile. Handlers now live in per-group modules — Handlers.Dice, .Systems, .Characters, .Build, .Inventory — and server.ex is dispatch only: the loop, the rescue boundary, protocol envelope construction, and an explicit command -> module registry. The registry replaces clause-order dispatch, so "unknown command" is decided by map lookup rather than by falling off the end of 26 patterns, and each handler module carries its own catch-all for a known command with malformed arguments ("invalid arguments for command: ..." — previously such requests fell through to the misleading "unknown command" response). Shared request/response helpers (parse_display_mode, character_with_choices_response, fetch_pending!, validate_concept_selection!) move to Server.Common. Both resolve_choice forms now sit adjacent in Handlers.Characters. Handler bodies are moved verbatim aside from Common./Errors. qualification; no behavior change except the malformed-arguments message noted above (no test asserted on the old text).
Closes #132. The moduledoc protocol listing is the only protocol documentation the Rust frontend has, and it omitted six implemented commands: build_start, build_select, build_resolve_sub, build_finish, random_resolve, and delete. All 22 registry commands are now listed with request examples, grouped to mirror the handler modules, with a short prose walkthrough of the builder flow and a note on the protocol-wide display_mode field.
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.
Closes #132.
What
Four commits, mapping to the issue's four fixes:
Sub-choice validation into the library —
fetch_choice_def!andvalid_sub_choicesmove fromSerializertoCharacters.Advancement(withCharacterswrappers), following refactor: move award application and progression choice resolution into the library #131's pattern. Their shared option-derivation helper becomes publicCharacters.sub_choice_options/2and the Serializer's rendering path now uses it, so the options shown and the options validated can no longer drift. The Serializer moduledoc now states its actual responsibility instead of apologizing for its existence.One error contract, one formatter — handlers return
{:ok, data, state} | {:error, message};handle_commandbuilds protocol envelopes in exactly one place. All domain{:error, reason}terms render through the newServer.Errors.message/1(awards, progression choices, preparation/activation, inventory-field validation). Thecharacters.spells,inventory.add, andinventory.sethandlers no longer leakinspect(reason)terms to the Rust frontend. Same-condition-same-message unifications::value_must_be_integer,:missing_effect_target, and the two missing-pending-character messages (characters.savenow usesfetch_pending!).Per-group handler modules + registry —
server.exdrops from 660 lines / 26 clauses to 138 lines of dispatch: loop, rescue boundary, envelopes, and an explicit command → module registry (Handlers.Dice,.Systems,.Characters,.Build,.Inventory; shared helpers inServer.Common). Bothresolve_choiceforms now sit adjacent inHandlers.Characters(the scoped one previously lived under the Inventory section, 227 lines from its sibling). Known commands with malformed arguments now get "invalid arguments for command: ..." instead of falling through to a misleading "unknown command".Complete protocol docs — all 22 registry commands documented with examples (previously six were missing:
build_start/select/resolve_sub/finish,random_resolve,delete), grouped to mirror the handler modules, with a builder-flow walkthrough and the protocol-widedisplay_modefield noted. Verified programmatically that every registry command appears in the moduledoc.Notes
Errors.message/1keeps a defensiveinspectfallback clause for reasons not yet given a message — every documented library reason has an explicit clause.Summary by Bito