fleetd #571: make FleetApp.writeReply's inner Outcome switch exhaustive, no default
Ticket comments (17126, 17127) corrected the original acceptance criterion after this unit was already in flight: a hand-listed grep for the enum's constant names goes stale silently the moment a new constant lands, so the compiler must be the enumeration instead. sendOutcomeLabel (MessageService.java) and formatReply (FleetMcp.java) were already default-free switch expressions. The one gap was writeReply's inner "status" switch, which had `default -> "done"` — the exact value that would have lied about TIMED_OUT_UNCONFIRMED. Remove the default and list every Outcome constant explicitly; REPLIED, COMPLETED_UNREPLIED, QUESTION and STALE_TURN get an arm too even though the outer switch always dispatches them first, so the inner switch stays exhaustive on its own. The outer switch (a statement, not an expression) keeps its own default — Java does not require exhaustiveness there regardless, and "everything not terminal is a 202" is an intentional catch-all. Verified with the proof the ticket asked for: added a scratch 11th Outcome constant after deleting all default arms and confirmed all three switch-expression sites (and no test file) fail to compile without an arm for it, one at a time, then removed the scratch constant.
This commit is contained in:
@@ -640,19 +640,25 @@ public final class FleetApp {
|
||||
}
|
||||
default -> ctx.status(202).json(Map.of(
|
||||
"sessionId", id,
|
||||
// fleetd #571 (ticket comment 17126): no `default` here on purpose. This switch
|
||||
// is an expression, so the compiler already demands every Outcome constant have
|
||||
// an arm — adding an 11th constant to Outcome is a compile error here, not a
|
||||
// silent fall-through. That is exactly the bug this ticket exists to fix:
|
||||
// `default -> "done"` used to sit here and would have told a REST caller the
|
||||
// delegation completed for TIMED_OUT_UNCONFIRMED, the one outcome where delivery
|
||||
// is unknown. REPLIED, COMPLETED_UNREPLIED, QUESTION and STALE_TURN can never
|
||||
// actually reach this inner switch — the outer switch above always dispatches
|
||||
// them first — but they still need an arm to keep this switch exhaustive.
|
||||
"status", switch (reply.outcome()) {
|
||||
case TIMED_OUT_WORKING -> "working";
|
||||
case TIMED_OUT_QUEUED -> "queued";
|
||||
// fleetd #571: delivery here is unknown, not merely still queued — see
|
||||
// Outcome#TIMED_OUT_UNCONFIRMED. Give it its own status rather than let it
|
||||
// fall to the default below, which would report "done" (delegation
|
||||
// completed) for the one case where we do not know whether it even arrived.
|
||||
// Delivery here is unknown, not merely still queued — see
|
||||
// Outcome#TIMED_OUT_UNCONFIRMED's own javadoc.
|
||||
case TIMED_OUT_UNCONFIRMED -> "unconfirmed";
|
||||
case BUSY -> "busy";
|
||||
case WORKER_FAILED -> "failed";
|
||||
case BACKEND_EXHAUSTED -> "backend_exhausted";
|
||||
default -> "done"; // unreachable — every non-terminal outcome reaching this
|
||||
// switch (all six above) has its own explicit case
|
||||
case REPLIED, COMPLETED_UNREPLIED, QUESTION, STALE_TURN -> "done"; // unreachable
|
||||
},
|
||||
"detail", reply.outcome() == MessageService.Outcome.TIMED_OUT_UNCONFIRMED
|
||||
? "no reply within " + timeout + "ms; delivery is unconfirmed — the "
|
||||
|
||||
Reference in New Issue
Block a user