From 7a9c73999ec0e2cc882db26d581fa86c4e8d861d Mon Sep 17 00:00:00 2001
From: Filip Leonarski <filip.leonarski@psi.ch>
Date: Tue, 3 Nov 2026 10:42:01 +0100
Subject: [PATCH] broker: settings that parse but fail validation answer 400,
 not 500

handleOperationException mapped every JFJochException but WrongDAQState to 500, so a /start whose
module placement does not name exactly the detector's modules (one left out, named twice, or not the
detector's) came back as an internal error although the API documents 400 for invalid input. The input
categories (InputParameterInvalid/AboveMax/BelowMin, WrongNumber, WrongUnit) now answer 400 with the
message as text, as a parsing failure does; everything else stays 500. Test:
JFJochBrokerHttp_StartModulePlacementMismatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqXYdJKPUFLoeAchjzv3x4
---
 broker/JFJochBrokerHttp.cpp  | 14 +++++++++
 tests/BrokerHttpAuthTest.cpp | 60 ++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/broker/JFJochBrokerHttp.cpp b/broker/JFJochBrokerHttp.cpp
index b417085ab..31ce0d92b 100644
--- a/broker/JFJochBrokerHttp.cpp
+++ b/broker/JFJochBrokerHttp.cpp
@@ -136,6 +136,20 @@ JFJochBrokerHttp::HttpError JFJochBrokerHttp::handleOperationException(const std
         throw;
     } catch (const WrongDAQStateException &) {
         return {500, error_message_json(ex.what(), "WrongDAQState"), "application/json"};
+    } catch (const JFJochException &e) {
+        // Settings that parse but do not fit what they are checked against (a module placement naming
+        // modules the detector does not have, a value out of range) are the request's fault, as a parsing
+        // failure is: 400, as the API documents it.
+        switch (e.Category()) {
+            case JFJochExceptionCategory::InputParameterInvalid:
+            case JFJochExceptionCategory::InputParameterAboveMax:
+            case JFJochExceptionCategory::InputParameterBelowMin:
+            case JFJochExceptionCategory::WrongNumber:
+            case JFJochExceptionCategory::WrongUnit:
+                return {400, e.what(), "text/plain"};
+            default:
+                return {500, error_message_json(ex.what(), "Other"), "application/json"};
+        }
     } catch (const std::exception &) {
         return {500, error_message_json(ex.what(), "Other"), "application/json"};
     }
diff --git a/tests/BrokerHttpAuthTest.cpp b/tests/BrokerHttpAuthTest.cpp
index 433ae10e0..20868cbe7 100644
--- a/tests/BrokerHttpAuthTest.cpp
+++ b/tests/BrokerHttpAuthTest.cpp
@@ -8,6 +8,7 @@
 #include <thread>
 
 #include "../broker/JFJochBrokerHttp.h"
+#include "../common/DetectorModules.h"
 
 // The tokens given to /start guard the endpoints that expose the dataset, and nothing else, for as
 // long as that dataset is the current one. Runs the real HTTP layer against a broker with no
@@ -159,3 +160,62 @@ TEST_CASE("JFJochBrokerHttp_BearerTokens", "[broker]") {
     }
 
 }
+
+// A module placement that does not name exactly the detector's modules is the request's fault: /start
+// answers 400, whether a module is left out, placed twice or not the detector's at all.
+TEST_CASE("JFJochBrokerHttp_StartModulePlacementMismatch", "[broker]") {
+    DiffractionExperiment experiment;
+    SpotFindingSettings spot_finding;
+    JFJochBrokerHttp broker(experiment, spot_finding);
+    broker.AddDetectorSetup(DetJF4M());
+
+    httplib::Server server;
+    broker.attach(server);
+    const int port = server.bind_to_any_port("127.0.0.1");
+    REQUIRE(port > 0);
+    struct ServerThread {
+        httplib::Server &server;
+        std::thread thread;
+        explicit ServerThread(httplib::Server &s) : server(s), thread([&s] { s.listen_after_bind(); }) {}
+        ~ServerThread() { server.stop(); thread.join(); }
+    } server_thread(server);
+    server.wait_until_ready();
+
+    httplib::Client client("127.0.0.1", port);
+    REQUIRE(client.Post("/initialize")->status == 200);
+
+    const auto assembled = ModulesAsImaged(DetJF4M().GetGeometry()).Get();
+    REQUIRE(assembled.size() == 8);
+    const auto entry = [](const DetectorModule &m) {
+        return nlohmann::json{{"box_x", m.x0}, {"box_y", m.y0}, {"box_width", m.width}, {"box_height", m.height},
+                              {"plane_x", m.ox}, {"plane_y", m.oy}, {"x_step_x", 1.0}, {"x_step_y", 0.0},
+                              {"y_step_x", 0.0}, {"y_step_y", 1.0}};
+    };
+    const auto start = [&](const nlohmann::json &placement) {
+        nlohmann::json body = {{"beam_x_pxl", 1000}, {"beam_y_pxl", 1000},
+                               {"detector_distance_mm", 100}, {"incident_energy_keV", 12.4},
+                               {"images_per_trigger", 1}, {"ntrigger", 1},
+                               {"file_prefix", "placement_run"}, {"module_placement", placement}};
+        auto res = client.Post("/start", body.dump(), "application/json");
+        REQUIRE(res);
+        return res->status;
+    };
+
+    nlohmann::json missing = nlohmann::json::array();
+    for (size_t m = 1; m < assembled.size(); m++)
+        missing.push_back(entry(assembled[m]));
+    CHECK(start(missing) == 400);
+
+    nlohmann::json twice = missing;
+    twice.push_back(entry(assembled[1]));
+    CHECK(start(twice) == 400);
+
+    DetectorModule elsewhere = assembled[0];
+    elsewhere.y0 += 1;
+    nlohmann::json unknown = missing;
+    unknown.push_back(entry(elsewhere));
+    CHECK(start(unknown) == 400);
+
+    // Nothing was started.
+    CHECK(nlohmann::json::parse(client.Get("/status")->body).at("state") == "Idle");
+}