brbzull0 commented on code in PR #13032:
URL: https://github.com/apache/trafficserver/pull/13032#discussion_r4268642940
##########
src/traffic_ctl/TrafficCtlStatus.h:
##########
@@ -20,10 +20,87 @@ limitations under the License.
*/
#pragma once
+#include <algorithm>
+#include <exception>
+#include <optional>
+#include <string_view>
+
+#include "mgmt/rpc/jsonrpc/error/RPCError.h"
+#include "shared/rpc/RPCRequests.h"
+#include "shared/rpc/yaml_codecs.h"
+#include "swoc/string_view_util.h"
+#include "tsutil/ts_errata.h"
+
constexpr int CTRL_EX_OK = 0;
// EXIT_FAILURE can also be used.
constexpr int CTRL_EX_ERROR = 2;
constexpr int CTRL_EX_UNIMPLEMENTED = 3;
constexpr int CTRL_EX_TEMPFAIL = 75; ///< Temporary failure — operation in
progress, retry later (EX_TEMPFAIL from sysexits.h).
-extern int App_Exit_Status_Code; //!< Global variable to store the exit status
code of the application.
+extern int App_Exit_Status_Code; //!< Global variable
to store the exit status code of the application.
+extern swoc::Errata::severity_type App_Exit_Level_Error; //!< Lowest
annotation severity that makes a server error exit non zero.
+
+/// Thrown by a command step that already printed a server error and graded it
into @c App_Exit_Status_Code, so the command
+/// stops there and the error is not reported a second time.
+struct ServerErrorReported : std::exception {
+ const char *
+ what() const noexcept override
+ {
+ return "server error already reported";
+ }
+};
+
+/// Parse an @c --error-level value: a severity name (case-insensitive), or
"warning" as an alias of "warn".
+inline std::optional<swoc::Errata::severity_type>
+parse_error_level(std::string_view name)
+{
+ if (strcasecmp(name, std::string_view{"warning"}) == 0) {
+ return swoc::Errata::severity_type(ERRATA_WARN);
+ }
+ for (size_t i = 0; i < Severity_Names.size(); ++i) {
+ if (strcasecmp(name, Severity_Names[i]) == 0) {
+ return swoc::Errata::severity_type(i);
+ }
+ }
+ return std::nullopt;
+}
Review Comment:
The overload is declared: libswoc's `swoc/string_view_util.h` has
`strcasecmp(std::string_view, std::string_view)`, this header included it, and
every CI platform built it. Switched to `ts::iequals` from
`tsutil/StringCompare.h` anyway, which the tree prefers for equality checks
(a94d7381d0).
##########
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.
+ }
+ }
+ }
Review Comment:
No change. The vendored yaml-cpp gives a quoted scalar the non-specific tag
`!` and a plain one `?` (`lib/yamlcpp/src/singledocparser.cpp:94`). The unit
test feeds `"0"` and `"4"`, and the AuTest run "a quoted number is not a
severity" sends `"severity": "0"`; both fail if a quoted number is read as a
severity.
##########
src/traffic_ctl/traffic_ctl.cc:
##########
@@ -382,6 +388,21 @@ main([[maybe_unused]] int argc, const char **argv)
signal_register_handler(SIGINT, handle_signal);
auto args = parser.parse(argv);
+
+ // ArgParser refuses it given more than once. Given without a value it is
an error, not the default.
+ if (auto const &error_level = args.get("error-level"); error_level) {
+ if (error_level.size() == 0) {
+ throw std::runtime_error("--error-level needs a value. Use one of
diag, debug, status, note, warn, error, fatal, alert, "
+ "emergency");
+ }
+ if (auto const level = parse_error_level(error_level.value()); level) {
+ App_Exit_Level_Error = *level;
+ } else {
+ throw std::runtime_error("Unknown error level: " + error_level.value()
+
+ ". Use one of diag, debug, status, note,
warn, error, fatal, alert, emergency");
+ }
Review Comment:
Done in a94d7381d0: the help text and both messages print the list built
from `Severity_Names` plus the `warning` alias, and a unit test pins it.
##########
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:
No change. No header declares it, and `traffic_server.cc` forward-declares
it the same way.
--
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]
