Copilot commented on code in PR #13032:
URL: https://github.com/apache/trafficserver/pull/13032#discussion_r4257221141
##########
src/traffic_ctl/CtrlCommands.cc:
##########
@@ -93,6 +93,16 @@ display_errors(BasePrinter *printer,
std::vector<ConfigReloadResponse::Error> co
}
}
+/// A request the command depends on failed, and its error is already printed
and graded. Below --error-level that grade is 0,
+/// but what was asked was not done, so the command exits CTRL_EX_TEMPFAIL
instead.
+void
+mark_not_done()
+{
+ if (App_Exit_Status_Code == CTRL_EX_OK) {
+ App_Exit_Status_Code = CTRL_EX_TEMPFAIL;
+ }
+}
Review Comment:
`mark_not_done()` is a file-local helper but currently has external linkage,
which unnecessarily exports a symbol and risks name collisions at link time.
Make it `static` or place it in an anonymous namespace in this `.cc` file.
##########
doc/appendices/command-line/traffic_ctl.en.rst:
##########
@@ -116,6 +116,70 @@ Options
Path to the runroot file.
+.. option:: --error-level <level>
+
+ Lowest severity that makes a server error exit with ``2``. When a method
handler fails, each
+ entry in the ``data`` array of the error can carry a ``severity`` (see
+ :ref:`jsonrpc-node-errors`). :program:`traffic_ctl` takes the most severe
entry: at or above
Review Comment:
This reference points to `:ref:`jsonrpc-node-errors`` but the new details
are in the dedicated severity section (`.. _jsonrpc-node-errors-severity:`).
Consider linking directly to `:ref:`jsonrpc-node-errors-severity`` here to make
it easier for readers to find the relevant semantics.
##########
include/shared/rpc/yaml_codecs.h:
##########
@@ -63,11 +63,33 @@ template <> struct convert<shared::rpc::JSONRPCError> {
static bool
decode(Node const &node, shared::rpc::JSONRPCError &error)
{
+ using DataEntry = shared::rpc::JSONRPCError::DataEntry;
+
error.code = helper::try_extract<int32_t>(node, "code");
error.message = helper::try_extract<std::string>(node, "message");
- if (auto data = node["data"]) {
- for (auto &&err : data) {
- error.data.emplace_back(helper::try_extract<int32_t>(err, "code"),
helper::try_extract<std::string>(err, "message"));
+ // Read only: the same tree is printed as received in json output.
+ if (auto const data = node["data"]; data && !data.IsSequence()) {
+ // Not a list of annotations: nothing in it can be read as one.
+ error.data.push_back({0, DataEntry::INVALID_SEVERITY, {}});
+ } else if (data) {
+ for (auto const &err : data) {
+ DataEntry entry;
+ entry.code = helper::try_extract<int32_t>(err, "code");
+ entry.message = helper::try_extract<std::string>(err, "message");
+ if (!err.IsMap()) {
+ entry.severity = DataEntry::INVALID_SEVERITY;
+ } else if (auto const severity = err["severity"]) {
+ entry.severity = DataEntry::INVALID_SEVERITY;
+ // A quoted value is text, not a severity.
+ if (severity.IsScalar() && severity.Tag() != "!") {
+ try {
+ entry.severity = severity.as<int32_t>();
+ } catch (YAML::Exception const &) {
+ // Not an integer: stays INVALID_SEVERITY.
+ }
+ }
+ }
+ error.data.push_back(std::move(entry));
}
}
Review Comment:
The YAML-cpp `convert<...>::decode()` specialization appends to `error.data`
without clearing it first. This is easy to trip over if the same
`shared::rpc::JSONRPCError` instance is decoded more than once (e.g., reuse
across calls), producing duplicated entries. Consider clearing `error.data`
(and/or resetting other fields) at the start of `decode()` so decoding always
overwrites the destination object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
