brbzull0 commented on code in PR #13032:
URL: https://github.com/apache/trafficserver/pull/13032#discussion_r4269730715
##########
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:
No change. For a non-map entry `try_extract` finds no key, or catches the
subscript error, and returns the default, so the entry ends up as code 0, no
message and an invalid severity. The unit test "Data entries that are not maps
always fail" feeds a string, `null`, a list and a number.
##########
src/mgmt/rpc/jsonrpc/unit_tests/test_basic_protocol.cc:
##########
@@ -21,11 +21,15 @@
#include <catch2/catch_test_macros.hpp> /* catch unit-test framework */
#include <tsutil/ts_bw_format.h>
+#include <tsutil/ts_errata.h>
#include "mgmt/rpc/jsonrpc/JsonRPCManager.h"
#include "mgmt/rpc/jsonrpc/JsonRPC.h"
#include "mgmt/rpc/handlers/common/ErrorUtils.h"
+// Defined in src/tsutil/ts_diags.cc; traffic_server calls it at startup.
+void Initialize_Errata_Settings();
+
Review Comment:
Same as
https://github.com/apache/trafficserver/pull/13032#discussion_r4268644015: no
header declares `Initialize_Errata_Settings()`. It is defined in
`src/tsutil/ts_diags.cc`, and `traffic_server.cc` forward-declares it the same
way.
##########
src/mgmt/rpc/jsonrpc/unit_tests/test_basic_protocol.cc:
##########
@@ -79,6 +83,66 @@ test_callback_ok_or_error(std::string_view const & /* id
ATS_UNUSED */, YAML::No
return resp;
}
+inline swoc::Rv<YAML::Node>
+test_callback_with_severity(std::string_view const & /* id ATS_UNUSED */,
YAML::Node const & /* params ATS_UNUSED */)
+{
+ swoc::Rv<YAML::Node> resp;
+ resp.errata()
+ .assign(ERR1)
+ .note(ERRATA_WARN, "this is a warning")
+ .note("this has no severity")
+ .note(ERRATA_FATAL, "this is fatal")
+ .note(ERRATA_DIAG, "this is a diag");
+ return resp;
+}
+
+inline swoc::Rv<YAML::Node>
+test_callback_note_only(std::string_view const & /* id ATS_UNUSED */,
YAML::Node const & /* params ATS_UNUSED */)
+{
+ swoc::Rv<YAML::Node> resp;
+ resp.errata().assign(ERR1).note(ERRATA_NOTE, "just a note");
+ return resp;
+}
+
+inline swoc::Rv<YAML::Node>
+test_callback_warn_only(std::string_view const & /* id ATS_UNUSED */,
YAML::Node const & /* params ATS_UNUSED */)
+{
+ swoc::Rv<YAML::Node> resp;
+ resp.errata().assign(ERR1).note(ERRATA_WARN, "already in that state");
+ return resp;
+}
+
+inline swoc::Rv<YAML::Node>
+test_callback_note_next_to_failure(std::string_view const & /* id ATS_UNUSED
*/, YAML::Node const & /* params ATS_UNUSED */)
+{
+ swoc::Rv<YAML::Node> resp;
+ resp.errata().assign(ERR1).note(ERRATA_NOTE, "just a note").note("the real
failure");
+ return resp;
+}
+
+inline swoc::Rv<YAML::Node>
+test_callback_errata_severity_apart(std::string_view const & /* id ATS_UNUSED
*/, YAML::Node const & /* params ATS_UNUSED */)
+{
+ swoc::Rv<YAML::Node> resp;
+ resp.errata().assign(ERR1).assign(ERRATA_ERROR).note(ERRATA_NOTE, "just a
note");
+ return resp;
+}
+
+// traffic_server's errata settings, restored on scope exit. The unit tests
run with libswoc's defaults otherwise, which treat
+// lower severities as failures.
+struct ProductionErrataSeverities {
+ swoc::Errata::Severity
saved_default{swoc::Errata::DEFAULT_SEVERITY};
+ swoc::Errata::Severity
saved_failure{swoc::Errata::FAILURE_SEVERITY};
+ swoc::MemSpan<swoc::TextView const>
saved_names{swoc::Errata::SEVERITY_NAMES};
+ ProductionErrataSeverities() { Initialize_Errata_Settings(); }
+ ~ProductionErrataSeverities()
+ {
+ swoc::Errata::DEFAULT_SEVERITY = saved_default;
+ swoc::Errata::FAILURE_SEVERITY = saved_failure;
+ swoc::Errata::SEVERITY_NAMES = saved_names;
+ }
+};
Review Comment:
No change. Catch2 runs the test cases of a binary one at a time on one
thread, and ctest runs each test case as its own process (`test_jsonrpc
"Severity field in error data entries"`), so nothing runs alongside the helper.
It also restores the saved settings on scope exit.
##########
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:
Same as
https://github.com/apache/trafficserver/pull/13032#discussion_r4258265722:
`mark_not_done()` is inside the anonymous namespace that opens at line 44 and
closes at line 132, so it has internal linkage; `nm` shows `t (anonymous
namespace)::mark_not_done()`.
--
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]
