Features: #304 — member routes name a herdr failure
+25
@@ -3666,3 +3666,28 @@ its guard checked `content == null`, not blank, so whitespace went through. Movi
|
||||
handler as an uncaught exception. The tool's own guard was widened to `isBlank` for that reason, and
|
||||
`FleetMcpTest.replyWithBlankContentIsACleanToolErrorNotAnUncaughtException` pins it. If you ever move
|
||||
the check again, check the handler between the door and the service, not only the two ends.
|
||||
|
||||
## The member REST routes name a herdr failure instead of returning a bare 500
|
||||
|
||||
**What.** `POST /members` and `DELETE /members/{paneId}` were the last two routes in `FleetApp`
|
||||
with no `catch (HerdrException)`. `FleetApp` registers no Javalin exception mapper, so the exception
|
||||
escaped as the default 500 with the body `Server Error` — no herdr code, no herdr message. Both now
|
||||
go through the same `herdrError` mapper every other herdr-calling route uses: 404 when herdr says
|
||||
the target is gone, 502 otherwise.
|
||||
|
||||
**On.** Always on.
|
||||
|
||||
**Why it exists.** `fleet_spawn` and `fleet_stop` already caught the same exception and reported a
|
||||
named error, so the two doors disagreed about the same failure — the shape #297 exists to remove.
|
||||
The stop route is the one that mattered: `SessionManager.release` deregisters the session, notifies
|
||||
the release listener and preserves a dirty worktree *before* it calls `launcher.stop`, so a throw
|
||||
from that stop arrives after the teardown the caller asked for has already happened. A 500 told the
|
||||
caller to retry and gave it nothing to reason about.
|
||||
|
||||
**One thing to know for maintenance.** An already-gone pane is **not** one of these failures.
|
||||
`HerdrPeerLauncher.stop` treats a `*_not_found` from `pane.close` as success, so a double stop still
|
||||
returns 204, and `stopToleratesAnAlreadyGonePane` pins that. What propagates is any other herdr
|
||||
failure — a transport error, or a code like `herdr_busy`. No new tests were added: two tests already
|
||||
covered these paths and asserted 500, and neither was about the status code (one guards that a
|
||||
failed spawn still closes its tab, the other that a failed teardown is not reported as 204). Their
|
||||
expectations moved to 502 and gained a body check.
|
||||
|
||||
Reference in New Issue
Block a user