Merge pull request #4722 from BenPope/move-kafka-error-code-to-kafka

pandaproxy: Invert error_code dependencies

src/v/kafka/protocol/errors.cc

@@ -202,4 +202,18 @@ std::ostream& operator<<(std::ostream& o, error_code code) {
     return o;
 }
 
+struct error_category final : std::error_category {
+    const char* name() const noexcept override { return "kafka"; }
+    std::string message(int ec) const override {
+        return std::string(
+          kafka::error_code_to_str(static_cast<kafka::error_code>(ec)));
+    }
+};
+
+const error_category error_category{};
+
+std::error_code make_error_code(kafka::error_code ec) {
+    return {static_cast<int>(ec), error_category};
+}
+
 } // namespace kafka

src/v/kafka/protocol/errors.h

@@ -13,6 +13,7 @@
 #include <cstdint>
 #include <iosfwd>
 #include <string_view>
+#include <system_error>
 
 namespace kafka {
 
@@ -224,5 +225,13 @@ enum class error_code : int16_t {
 
 std::ostream& operator<<(std::ostream&, error_code);
 std::string_view error_code_to_str(error_code error);
+std::error_code make_error_code(error_code);
 
 } // namespace kafka
+
+namespace std {
+
+template<>
+struct is_error_code_enum<kafka::error_code> : true_type {};
+
+} // namespace std

src/v/pandaproxy/CMakeLists.txt

@@ -15,7 +15,7 @@ v_cc_library(
     v::utils
   )
 
-
+add_subdirectory(test)
 add_subdirectory(rest)
 add_subdirectory(schema_registry)
 add_subdirectory(json)

src/v/pandaproxy/error.cc

@@ -12,6 +12,7 @@
 #include "error.h"
 
 #include "kafka/protocol/errors.h"
+#include "pandaproxy/json/error.h"
 #include "pandaproxy/parsing/error.h"
 
 namespace pandaproxy {
@@ -78,22 +79,24 @@ struct reply_error_category final : std::error_category {
         }
         return "(unrecognized error)";
     }
+    bool
+    equivalent(const std::error_code& ec, int cond) const noexcept override {
+        return make_error_condition(ec).value() == cond;
+    }
 };
 
 const reply_error_category reply_error_category{};
 
-struct kafka_error_category final : std::error_category {
-    const char* name() const noexcept override { return "kafka"; }
-    std::string message(int ec) const override {
-        return std::string(
-          kafka::error_code_to_str(static_cast<kafka::error_code>(ec)));
-    }
-    std::error_condition
-    default_error_condition(int ec) const noexcept override {
-        using rec = reply_error_code;
-        using kec = kafka::error_code;
+}; // namespace
+
+std::error_condition make_error_condition(std::error_code ec) {
+    using rec = reply_error_code;
+    using kec = kafka::error_code;
+    using pec = pandaproxy::parse::error_code;
+    using jec = pandaproxy::json::error_code;
 
-        switch (static_cast<kec>(ec)) {
+    if (ec.category() == make_error_code(kec::none).category()) {
+        switch (static_cast<kec>(ec.value())) {
         case kec::none:
             return {};
         case kec::offset_out_of_range:
@@ -190,12 +193,26 @@ struct kafka_error_category final : std::error_category {
             return rec::kafka_authentication_error;
         }
         return {}; // keep gcc happy
+    } else if (ec.category() == make_error_code(pec::empty_param).category()) {
+        switch (static_cast<pec>(ec.value())) {
+        case pec::empty_param:
+        case pec::invalid_param:
+            return rec::kafka_bad_request;
+        case pec::not_acceptable:
+            return rec::not_acceptable;
+        case pec::unsupported_media_type:
+            return rec::unsupported_media_type;
+        }
+        return {};
+    } else if (ec.category() == make_error_code(jec::invalid_json).category()) {
+        switch (static_cast<jec>(ec.value())) {
+        case jec::invalid_json:
+            return rec::unprocessable_entity;
+        }
+        return {};
     }
-};
-
-const kafka_error_category kafka_error_category{};
-
-}; // namespace
+    return ec.default_error_condition();
+}
 
 std::error_condition make_error_condition(reply_error_code ec) {
     return {static_cast<int>(ec), reply_error_category};
@@ -206,10 +223,3 @@ const std::error_category& reply_category() noexcept {
 }
 
 } // namespace pandaproxy
-
-namespace kafka {
-std::error_code make_error_code(kafka::error_code ec) {
-    return {static_cast<int>(ec), pandaproxy::kafka_error_category};
-}
-
-} // namespace kafka

src/v/pandaproxy/error.h

@@ -54,22 +54,14 @@ enum class reply_error_code : uint16_t {
 };
 
 std::error_condition make_error_condition(reply_error_code);
+std::error_condition make_error_condition(std::error_code ec);
 const std::error_category& reply_category() noexcept;
 
 } // namespace pandaproxy
 
-namespace kafka {
-
-std::error_code make_error_code(error_code);
-
-}
-
 namespace std {
 
 template<>
 struct is_error_condition_enum<pandaproxy::reply_error_code> : true_type {};
 
-template<>
-struct is_error_code_enum<kafka::error_code> : true_type {};
-
 } // namespace std

src/v/pandaproxy/json/CMakeLists.txt

@@ -4,7 +4,6 @@ v_cc_library(
     error.cc
   DEPS
     Seastar::seastar
-    v::pandaproxy_common
 )
 
 add_subdirectory(test)

src/v/pandaproxy/json/error.cc

@@ -11,8 +11,6 @@
 
 #include "error.h"
 
-#include "pandaproxy/error.h"
-
 namespace pandaproxy::json {
 
 namespace {
@@ -26,14 +24,6 @@ struct error_category final : std::error_category {
         }
         return "(unrecognized error)";
     }
-    std::error_condition
-    default_error_condition(int ec) const noexcept override {
-        switch (static_cast<error_code>(ec)) {
-        case error_code::invalid_json:
-            return reply_error_code::unprocessable_entity;
-        }
-        return {};
-    }
 };
 
 const error_category ppj_error_category{};

src/v/pandaproxy/parsing/error.cc

@@ -11,8 +11,6 @@
 
 #include "error.h"
 
-#include "pandaproxy/error.h"
-
 namespace pandaproxy::parse {
 
 namespace {
@@ -32,20 +30,6 @@ struct error_category final : std::error_category {
         }
         return "(unrecognized error)";
     }
-
-    std::error_condition
-    default_error_condition(int ec) const noexcept override {
-        switch (static_cast<error_code>(ec)) {
-        case error_code::empty_param:
-        case error_code::invalid_param:
-            return reply_error_code::kafka_bad_request;
-        case error_code::not_acceptable:
-            return reply_error_code::not_acceptable;
-        case error_code::unsupported_media_type:
-            return reply_error_code::unsupported_media_type;
-        }
-        return {};
-    }
 };
 
 const error_category ppp_error_category{};

src/v/pandaproxy/parsing/exceptions.h

@@ -22,25 +22,25 @@
 namespace pandaproxy::parse {
 
 struct exception_base : std::exception {
-    explicit exception_base(std::error_condition err)
+    explicit exception_base(std::error_code err)
       : std::exception{}
       , error{err}
       , msg{err.message()} {}
-    exception_base(std::error_condition err, std::string_view msg)
+    exception_base(std::error_code err, std::string_view msg)
       : std::exception{}
       , error{err}
       , msg{msg} {}
     const char* what() const noexcept final { return msg.c_str(); }
-    std::error_condition error;
+    std::error_code error;
     std::string msg;
 };
 
 class error final : public exception_base {
 public:
     explicit error(error_code ec)
-      : exception_base(make_error_code(ec).default_error_condition()) {}
+      : exception_base(ec) {}
     error(error_code ec, std::string_view msg)
-      : exception_base(make_error_code(ec).default_error_condition(), msg) {}
+      : exception_base(ec, msg) {}
 };
 
 } // namespace pandaproxy::parse

src/v/pandaproxy/reply.h

@@ -78,7 +78,7 @@ errored_body(std::error_condition ec, ss::sstring msg) {
 
 inline std::unique_ptr<ss::httpd::reply>
 errored_body(std::error_code ec, ss::sstring msg) {
-    return errored_body(ec.default_error_condition(), std::move(msg));
+    return errored_body(make_error_condition(ec), std::move(msg));
 }
 
 inline std::unique_ptr<ss::httpd::reply> unprocessable_entity(ss::sstring msg) {

src/v/pandaproxy/test/CMakeLists.txt

@@ -0,0 +1,9 @@
+rp_test(
+  UNIT_TEST
+  BINARY_NAME pandaproxy_unit
+  SOURCES
+    errors.cc
+  DEFINITIONS BOOST_TEST_DYN_LINK
+  LIBRARIES Boost::unit_test_framework v::pandaproxy_common v::kafka_protocol
+  LABELS pandaproxy
+)

src/v/pandaproxy/test/errors.cc

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2021 Redpanda Data, Inc.
+ *
+ * Use of this software is governed by the Business Source License
+ * included in the file licenses/BSL.md
+ *
+ * As of the Change Date specified in that file, in accordance with
+ * the Business Source License, use of this software will be governed
+ * by the Apache License, Version 2.0
+ */
+
+#define BOOST_TEST_MODULE pandaproxy
+
+#include "kafka/protocol/errors.h"
+
+#include "pandaproxy/error.h"
+#include "pandaproxy/json/error.h"
+#include "pandaproxy/parsing/error.h"
+
+#include <boost/test/data/monomorphic.hpp>
+#include <boost/test/data/test_case.hpp>
+#include <boost/test/unit_test.hpp>
+
+#include <ostream>
+#include <system_error>
+#include <utility>
+
+namespace bdata = boost::unit_test::data;
+namespace pp = pandaproxy;
+using rec = pp::reply_error_code;
+using kec = kafka::error_code;
+using pec = pandaproxy::parse::error_code;
+using jec = pandaproxy::json::error_code;
+
+struct ec_cond {
+    std::error_code ec;
+    rec cond;
+    friend std::ostream& operator<<(std::ostream& os, const ec_cond& p) {
+        return os << p.ec.message() << ", "
+                  << make_error_condition(p.cond).message();
+    }
+};
+
+static const std::array<ec_cond, 5> conversion_data{
+  {{kec::offset_out_of_range, rec::kafka_bad_request},
+   {kec::unknown_server_error, rec::kafka_error},
+   {kec::unknown_topic_or_partition, rec::partition_not_found},
+   {pec::not_acceptable, rec::not_acceptable},
+   {jec::invalid_json, rec::unprocessable_entity}}};
+
+BOOST_DATA_TEST_CASE(
+  test_error_condition_conversion, bdata::make(conversion_data), sample) {
+    auto ec = sample.ec;
+    auto cond = pp::make_error_condition(ec);
+    BOOST_REQUIRE(sample.cond == cond);
+}
+
+BOOST_DATA_TEST_CASE(
+  test_error_condition_equivalence, bdata::make(conversion_data), sample) {
+    BOOST_REQUIRE(sample.cond == sample.ec);
+    BOOST_REQUIRE(sample.ec == sample.cond);
+}