broker: error responses carry the content type the OpenAPI spec declares
Build Packages / Create release (push) Successful in 21s
Build Packages / build:rugnux:aarch64 (cross) (push) Successful in 7m28s
Build Packages / build:rugnux-tgz (x86_64) (push) Successful in 8m1s
Build Packages / build:viewer-tgz:cpu (push) Successful in 8m41s
Build Packages / build:viewer-tgz:cuda (push) Successful in 9m52s
Build Packages / build:windows:nocuda (push) Successful in 17m33s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 14m7s
Build Packages / build:windows:cuda (push) Successful in 20m12s
Build Packages / HDF5 consumer tests (DIALS, XDS) (push) Successful in 24m10s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 16m57s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 16m26s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 17m59s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 16m8s
Build Packages / Generate python client (push) Successful in 35s
Build Packages / Build documentation (push) Successful in 1m15s
Build Packages / build:rugnux:windows (push) Successful in 10m52s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 16m2s
Build Packages / build:rpm (rocky8) (push) Successful in 15m24s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 15m31s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 15m9s
Build Packages / build:rpm (rocky9) (push) Successful in 16m21s
Build Packages / Unit tests (push) Successful in 2h17m0s
Build Packages / Create release (push) Successful in 21s
Build Packages / build:rugnux:aarch64 (cross) (push) Successful in 7m28s
Build Packages / build:rugnux-tgz (x86_64) (push) Successful in 8m1s
Build Packages / build:viewer-tgz:cpu (push) Successful in 8m41s
Build Packages / build:viewer-tgz:cuda (push) Successful in 9m52s
Build Packages / build:windows:nocuda (push) Successful in 17m33s
Build Packages / build:rpm (rocky8_nocuda) (push) Successful in 14m7s
Build Packages / build:windows:cuda (push) Successful in 20m12s
Build Packages / HDF5 consumer tests (DIALS, XDS) (push) Successful in 24m10s
Build Packages / build:rpm (ubuntu2204_nocuda) (push) Successful in 16m57s
Build Packages / build:rpm (ubuntu2404_nocuda) (push) Successful in 16m26s
Build Packages / build:rpm (rocky9_nocuda) (push) Successful in 17m59s
Build Packages / build:rpm (rocky8_sls9) (push) Successful in 16m8s
Build Packages / Generate python client (push) Successful in 35s
Build Packages / Build documentation (push) Successful in 1m15s
Build Packages / build:rugnux:windows (push) Successful in 10m52s
Build Packages / build:rpm (rocky9_sls9) (push) Successful in 16m2s
Build Packages / build:rpm (rocky8) (push) Successful in 15m24s
Build Packages / build:rpm (ubuntu2204) (push) Successful in 15m31s
Build Packages / build:rpm (ubuntu2404) (push) Successful in 15m9s
Build Packages / build:rpm (rocky9) (push) Successful in 16m21s
Build Packages / Unit tests (push) Successful in 2h17m0s
The 500 handlers built a correct error_message object but sent it through send_plain(), which labels the body text/plain. The spec declares 500 as application/json + error_message, and the generated clients key their deserialization off the content type: the Python client kept the body as a raw str, handed it to ErrorMessage.from_dict(), and pydantic raised - the exception swallowed by the finally: in response_deserialize, so the caller got a ServiceException with data=None and only the raw text in .body. send_plain becomes send_error, taking the content type alongside the code and the body; the two exception handlers now return it. 400 stays a plain-text exception string, exactly as the spec says; 500 is JSON. Two further 500s did not carry an error_message object at all and now do: the generic std::exception branch of handleParsingException, which returned a bare what(), and ProcessOutput's output-validation failure, which returned the validation dump. A catch-all set_exception_handler covers anything that escapes a route's own handler - httplib would otherwise answer with a bodyless 500, which fails to deserialize the same way. Verified against a running broker: POST /pedestal in the wrong state returns application/json and the generated Python client parses it into ErrorMessage(msg=..., reason='WrongDAQState'); a malformed body still returns 400 text/plain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+45
-47
@@ -74,15 +74,6 @@ namespace {
|
||||
return "application/octet-stream";
|
||||
}
|
||||
|
||||
std::string error_message_json(const std::string &msg, const std::string &reason) {
|
||||
Error_message m;
|
||||
m.setMsg(msg);
|
||||
m.setReason(reason);
|
||||
nlohmann::json j;
|
||||
to_json(j, m);
|
||||
return j.dump();
|
||||
}
|
||||
|
||||
inline bool read_file_to_string(const std::string &path, std::string &out) {
|
||||
std::ifstream f(path, std::ios::binary);
|
||||
if (!f)
|
||||
@@ -94,6 +85,15 @@ namespace {
|
||||
}
|
||||
}
|
||||
|
||||
std::string JFJochBrokerHttp::error_message_json(const std::string &msg, const std::string &reason) {
|
||||
Error_message m;
|
||||
m.setMsg(msg);
|
||||
m.setReason(reason);
|
||||
nlohmann::json j;
|
||||
to_json(j, m);
|
||||
return j.dump();
|
||||
}
|
||||
|
||||
JFJochBrokerHttp::JFJochBrokerHttp(const DiffractionExperiment &experiment,
|
||||
const SpotFindingSettings &spot_finding_settings)
|
||||
: state_machine(experiment, services, logger, spot_finding_settings) {
|
||||
@@ -113,29 +113,41 @@ JFJochBrokerHttp &JFJochBrokerHttp::FrontendDirectory(const std::string &directo
|
||||
return *this;
|
||||
}
|
||||
|
||||
std::pair<int, std::string> JFJochBrokerHttp::handleParsingException(const std::exception &ex) const noexcept {
|
||||
JFJochBrokerHttp::HttpError JFJochBrokerHttp::handleParsingException(const std::exception &ex) const noexcept {
|
||||
try {
|
||||
throw;
|
||||
} catch (const nlohmann::detail::exception &e) {
|
||||
return {400, e.what()};
|
||||
return {400, e.what(), "text/plain"};
|
||||
} catch (const org::openapitools::server::helpers::ValidationException &e) {
|
||||
return {400, e.what()};
|
||||
return {400, e.what(), "text/plain"};
|
||||
} catch (const std::exception &e) {
|
||||
return {500, e.what()};
|
||||
return {500, error_message_json(e.what(), "Other"), "application/json"};
|
||||
}
|
||||
}
|
||||
|
||||
std::pair<int, std::string> JFJochBrokerHttp::handleOperationException(const std::exception &ex) const noexcept {
|
||||
JFJochBrokerHttp::HttpError JFJochBrokerHttp::handleOperationException(const std::exception &ex) const noexcept {
|
||||
try {
|
||||
throw;
|
||||
} catch (const WrongDAQStateException &) {
|
||||
return {500, error_message_json(ex.what(), "WrongDAQState")};
|
||||
return {500, error_message_json(ex.what(), "WrongDAQState"), "application/json"};
|
||||
} catch (const std::exception &) {
|
||||
return {500, error_message_json(ex.what(), "Other")};
|
||||
return {500, error_message_json(ex.what(), "Other"), "application/json"};
|
||||
}
|
||||
}
|
||||
|
||||
void JFJochBrokerHttp::attach(httplib::Server &server) {
|
||||
// Last resort for an exception that got past a route's own handler - httplib would otherwise
|
||||
// answer with a bodyless 500, which no generated client can deserialize into an error_message.
|
||||
server.set_exception_handler([this](const httplib::Request &, httplib::Response &res,
|
||||
const std::exception_ptr &ep) {
|
||||
try {
|
||||
std::rethrow_exception(ep);
|
||||
} catch (const std::exception &e) {
|
||||
send_error(res, handleOperationException(e));
|
||||
} catch (...) {
|
||||
send_error(res, {500, error_message_json("Unknown exception", "Other"), "application/json"});
|
||||
}
|
||||
});
|
||||
register_routes(server);
|
||||
}
|
||||
|
||||
@@ -145,8 +157,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
(this->*method)(res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
};
|
||||
};
|
||||
@@ -160,8 +171,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
v.validate();
|
||||
} catch (const std::exception &e) {
|
||||
// Malformed or invalid request body.
|
||||
auto [c, s] = handleParsingException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleParsingException(e));
|
||||
return;
|
||||
}
|
||||
try {
|
||||
@@ -170,8 +180,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
// Operation failure (e.g. WrongDAQState, a failed synchronous start): return the
|
||||
// structured Error_message JSON with a reason field, like the no-arg endpoints, so
|
||||
// clients can distinguish the cause instead of receiving opaque plain text.
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
};
|
||||
};
|
||||
@@ -181,8 +190,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
(this->*method)(req, res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
};
|
||||
};
|
||||
@@ -210,16 +218,14 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
config_internal_generator_image_put(parse_query_value<int64_t>(req, "id"), req, res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
server.Put("/config/internal_generator_image.tiff", [this](const httplib::Request &req, httplib::Response &res) {
|
||||
try {
|
||||
config_internal_generator_image_tiff_put(parse_query_value<int64_t>(req, "id"), req, res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
server.Get("/config/mask", bind_noarg(&JFJochBrokerHttp::config_mask_get));
|
||||
@@ -250,8 +256,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
image_buffer_image_cbor_get(parse_query_value<int64_t>(req, "id"), res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -272,8 +277,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
res
|
||||
);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -281,8 +285,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
image_buffer_image_tiff_get(parse_query_value<int64_t>(req, "id"), res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -298,8 +301,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
parse_query_value<int32_t>(req, "sc"),
|
||||
res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -309,8 +311,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
parse_query_string(req, "roi"),
|
||||
res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -323,8 +324,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
parse_query_string(req, "azint_unit"),
|
||||
res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -337,8 +337,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
statistics_get(res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -350,8 +349,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
wait_till_done_post(parse_query_value<int32_t>(req, "timeout"), res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -359,8 +357,7 @@ void JFJochBrokerHttp::register_routes(httplib::Server &server) {
|
||||
try {
|
||||
wait_until_running_post(parse_query_value<int32_t>(req, "timeout"), res);
|
||||
} catch (const std::exception &e) {
|
||||
auto [c, s] = handleOperationException(e);
|
||||
send_plain(res, c, s);
|
||||
send_error(res, handleOperationException(e));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -451,7 +448,8 @@ void JFJochBrokerHttp::wait_till_done_post(const std::optional<int32_t> &timeout
|
||||
// is never reported as an error. A Warning - a receiver that could not keep up - is
|
||||
// still a 200: the data is there, and the run is done.
|
||||
if (status.message_severity == BrokerStatus::MessageSeverity::Error) {
|
||||
send_plain(response, 500, error_message_json(status.message.value_or("Unknown error"), "Other"));
|
||||
send_error(response, {500, error_message_json(status.message.value_or("Unknown error"), "Other"),
|
||||
"application/json"});
|
||||
return;
|
||||
}
|
||||
response.status = 200;
|
||||
|
||||
@@ -45,18 +45,28 @@ class JFJochBrokerHttp {
|
||||
JFJochStateMachine state_machine;
|
||||
std::string frontend_directory;
|
||||
|
||||
static void send_plain(httplib::Response &res, int code, const std::string &body) {
|
||||
res.status = code;
|
||||
res.set_content(body, "text/plain");
|
||||
// The API declares 400 as a plain-text exception string and 500 as an error_message object,
|
||||
// so an error response has to carry the content type its body actually is - a JSON body
|
||||
// labelled text/plain is what the generated clients refuse to deserialize.
|
||||
struct HttpError {
|
||||
int code;
|
||||
std::string body;
|
||||
const char *content_type;
|
||||
};
|
||||
|
||||
static void send_error(httplib::Response &res, const HttpError &error) {
|
||||
res.status = error.code;
|
||||
res.set_content(error.body, error.content_type);
|
||||
}
|
||||
|
||||
static std::string error_message_json(const std::string &msg, const std::string &reason);
|
||||
|
||||
template<class T>
|
||||
void ProcessOutput(const T &output, httplib::Response &response) {
|
||||
std::stringstream s;
|
||||
if (!output.validate(s)) {
|
||||
logger.Error(s.str());
|
||||
response.status = 500;
|
||||
response.set_content(s.str(), "text/plain");
|
||||
send_error(response, {500, error_message_json(s.str(), "Other"), "application/json"});
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -65,8 +75,8 @@ class JFJochBrokerHttp {
|
||||
response.set_content(j.dump(), "application/json");
|
||||
}
|
||||
|
||||
std::pair<int, std::string> handleParsingException(const std::exception &ex) const noexcept;
|
||||
std::pair<int, std::string> handleOperationException(const std::exception &ex) const noexcept;
|
||||
HttpError handleParsingException(const std::exception &ex) const noexcept;
|
||||
HttpError handleOperationException(const std::exception &ex) const noexcept;
|
||||
|
||||
void register_routes(httplib::Server &server);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user