Skip to content

refactor(server): single rescue boundary in dispatch#157

Merged
QMalcolm merged 1 commit into
mainfrom
qmalcolm--refactor-single-rescue-boundary
Jul 21, 2026
Merged

refactor(server): single rescue boundary in dispatch#157
QMalcolm merged 1 commit into
mainfrom
qmalcolm--refactor-single-rescue-boundary

Conversation

@QMalcolm

@QMalcolm QMalcolm commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Closes #129.

What

22 of the 26 handle/2 clauses in server.ex wrapped their body in the identical try do ... rescue e -> {error(Exception.message(e)), state} end block. All per-handler blocks are deleted; the rescue now lives in exactly one place on the dispatch path.

How

  • handle_command/2 (already the public test hook) is now the single rescue boundary, using a function-level rescue. dispatch/2 routes through it.
  • One deviation from the issue sketch: the boundary sits in handle_command/2 rather than inline in dispatch/2, because several tests assert error responses by calling handle_command directly with maps — dispatch/2 takes a raw JSON line the tests never construct. Same single-boundary result, and tests pass unchanged.
  • State-threading safety verified across all six state-mutating handlers (gen, build_start, build_select, build_resolve_sub, build_finish, save): each builds its new state and returns it only on success, so a mid-handler raise reaches the boundary before any mutation escapes and the original state is returned — identical to the old per-handler behavior.

Net −80 lines; the large diff line count is the unindenting of every handler body by one level.

All 389 tests pass unchanged; credo and dialyzer clean.

Summary by Bito

  • Centralized exception handling for command processing by introducing a rescue block in handle_command/2.
  • Removed redundant try/rescue blocks from individual command handler functions, simplifying the codebase.
  • Ensured that any exception raised during command execution results in a protocol error response while preserving the original state.

Closes #129.

22 of the 26 handle/2 clauses wrapped their entire body in the
identical "try do ... rescue e -> {error(Exception.message(e)), state}
end" block. Beyond the sheer boilerplate (~110 lines and an extra
indentation level in nearly every handler), the pattern made it easy
for a future handler to forget the wrapper and crash the server loop
on a bad request.

The rescue now lives in one place: handle_command/2, which dispatch/2
routes through. handle_command was already the public test hook, so
placing the boundary there (rather than inline in dispatch/2, as the
issue sketched) keeps the tests' entry point semantics identical —
several tests assert that raising commands return protocol errors via
handle_command directly, and dispatch/2 takes a raw JSON line, which
tests never construct.

State-threading safety, verified across all six state-mutating
handlers (gen, build_start, build_select, build_resolve_sub,
build_finish, save): each builds its new state and returns it only on
success, so a mid-handler raise reaches the boundary before any
mutation escapes, and the caller's original state is returned
unchanged. This is the same behavior the per-handler blocks provided.

The four clauses that had no try/rescue (systems.list,
characters.list variants' siblings, delete, unknown-command
fallbacks) are unchanged; they either cannot raise or already return
tagged errors.
@QMalcolm
QMalcolm merged commit 3b11eb3 into main Jul 21, 2026
6 checks passed
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.

refactor(server): single rescue boundary in dispatch

1 participant