osdc: Catch exceptions thrown in CLS client decoders

And return them to the client by setting the error code and result in
the vector and returning an error from the operation as a whole.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index f006597e8273976ce7a4e65f3160b31112eb2228..eee03c1189877e5b93bdcfdc2ce83fb0118ac3de 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -3537,6 +3537,7 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
   ceph_assert(op->out_bl.size() == op->out_rval.size());
   ceph_assert(op->out_bl.size() == op->out_handler.size());
   auto p = out_ops.begin();
+  bs::error_code handler_error;
   for (unsigned i = 0;
        p != out_ops.end() && pb != op->out_bl.end();
        ++i, ++p, ++pb, ++pr, ++pe, ++ph) {
@@ -3552,10 +3553,32 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
       **pe = p->rval < 0 ? bs::error_code(-p->rval, osd_category()) :
        bs::error_code();
     if (*ph) {
-      std::move((*ph))(p->rval < 0 ?
-                      bs::error_code(-p->rval, osd_category()) :
-                      bs::error_code(),
-                      p->rval, p->outdata);
+      try {
+       std::move((*ph))(p->rval < 0 ?
+                        bs::error_code(-p->rval, osd_category()) :
+                        bs::error_code(),
+                        p->rval, p->outdata);
+      } catch (const bs::system_error& e) {
+       ldout(cct, 10) << "ERROR: tid " << op->tid << ": handler function threw "
+                      << e.what() << dendl;
+       handler_error = e.code();
+       if (*pe) {
+         **pe = e.code();
+       }
+       if (*pr) {
+         **pr = ceph::from_error_code(e.code());
+       }
+      } catch (const std::exception& e) {
+       ldout(cct, 0) << "ERROR: tid " << op->tid << ": handler function threw "
+                     << e.what() << dendl;
+       handler_error = osdc_errc::handler_failed;
+       if (*pe) {
+         **pe = osdc_errc::handler_failed;
+       }
+       if (*pr) {
+         **pr = -EIO;
+       }
+      }
     }
   }
 
@@ -3587,7 +3610,11 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
 
   // do callbacks
   if (Op::has_completion(onfinish)) {
-    Op::complete(std::move(onfinish), osdcode(rc), rc);
+    if (rc == 0 && handler_error) {
+      Op::complete(std::move(onfinish), handler_error, -EIO);
+    } else {
+      Op::complete(std::move(onfinish), osdcode(rc), rc);
+    }
   }
   if (completion_lock.mutex()) {
     completion_lock.unlock();

diff --git a/src/osdc/error_code.cc b/src/osdc/error_code.cc
index 7823e8b088c4eab2e13584a99cfec209d5c664ea..d60faa587bccf64c474953fa67355e96ced5fe86 100644 (file)
--- a/src/osdc/error_code.cc
+++ b/src/osdc/error_code.cc
@@ -73,6 +73,9 @@ const char* osdc_error_category::message(int ev, char*,
 
   case osdc_errc::pool_eio:
     return "Pool EIO flag set";
+
+  case osdc_errc::handler_failed:
+    return "Handler function threw unknown exception";
   }
 
   return "Unknown error";
@@ -101,6 +104,8 @@ osdc_error_category::default_error_condition(int ev) const noexcept {
     return bs::errc::timed_out;
   case osdc_errc::pool_eio:
     return bs::errc::io_error;
+  case osdc_errc::handler_failed:
+    return bs::errc::io_error;
   }
 
   return { ev, *this };
@@ -156,6 +161,8 @@ int osdc_error_category::from_code(int ev) const noexcept {
     return -ETIMEDOUT;
   case osdc_errc::pool_eio:
     return -EIO;
+  case osdc_errc::handler_failed:
+    return -EIO;
   }
   return -EDOM;
 }

diff --git a/src/osdc/error_code.h b/src/osdc/error_code.h
index eb78a5110b014528a94c678e35baa8de7763b259..88d6f080a8a41f19975e3171f1ffb300e57341fe 100644 (file)
--- a/src/osdc/error_code.h
+++ b/src/osdc/error_code.h
@@ -30,7 +30,8 @@ enum class osdc_errc {
   snapshot_exists,
   snapshot_dne,
   timed_out,
-  pool_eio
+  pool_eio,
+  handler_failed
 };
 
 namespace boost::system {

diff --git a/src/test/neorados/CMakeLists.txt b/src/test/neorados/CMakeLists.txt
index e3caaaf07d9e5895743e41e6a3a960ab46d70d2c..fd76b09cf952c2c2be24925a80d091b541d1744d 100644 (file)
--- a/src/test/neorados/CMakeLists.txt
+++ b/src/test/neorados/CMakeLists.txt
@@ -1,4 +1,3 @@
-
 add_executable(ceph_test_neorados test_neorados.cc)
 target_link_libraries(ceph_test_neorados global libneorados
   ${unittest_libs}
@@ -26,3 +25,19 @@ target_link_libraries(neoradostest-support
 add_executable(ceph_test_neorados_list_pool list_pool.cc)
 target_link_libraries(ceph_test_neorados_list_pool
   libneorados neoradostest-support global fmt::fmt ${unittest_libs})
+
+add_executable(ceph_test_neorados_handler_error
+  handler_error.cc
+  )
+target_link_libraries(ceph_test_neorados_handler_error
+  libneorados
+  ${BLKID_LIBRARIES}
+  ${CMAKE_DL_LIBS}
+  ${CRYPTO_LIBS}
+  ${EXTRALIBS}
+  neoradostest-support
+  ${UNITTEST_LIBS}
+  )
+install(TARGETS
+  ceph_test_neorados_handler_error
+  DESTINATION ${CMAKE_INSTALL_BINDIR})

diff --git a/src/test/neorados/handler_error.cc b/src/test/neorados/handler_error.cc
new file mode 100644 (file)
index 0000000..26d468b
--- /dev/null
+++ b/src/test/neorados/handler_error.cc
@@ -0,0 +1,62 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2023 IBM
+ *
+ * See file COPYING for license information.
+ *
+ */
+
+#include <coroutine>
+#include <memory>
+#include <string_view>
+#include <utility>
+
+#include <boost/asio/use_awaitable.hpp>
+#include <boost/asio/redirect_error.hpp>
+
+#include <boost/system/errc.hpp>
+#include <boost/system/error_code.hpp>
+
+#include "include/neorados/RADOS.hpp"
+
+#include "cls/version/cls_version_types.h"
+
+#include "test/neorados/common_tests.h"
+
+#include "gtest/gtest.h"
+
+namespace asio = boost::asio;
+namespace sys = boost::system;
+namespace buffer = ceph::buffer;
+
+CORO_TEST_F(neocls_handler_error, test_handler_error, NeoRadosTest)
+{
+  std::string_view oid = "obj";
+  co_await create_obj(rados(), oid, pool(), asio::use_awaitable);
+
+  {
+    neorados::ReadOp op;
+    op.exec("version", "read", {},
+           [](sys::error_code ec, const buffer::list& bl) {
+             throw buffer::end_of_buffer{};
+           });
+    co_await expect_error_code(rados().execute(oid, pool(), std::move(op),
+                                              nullptr, asio::use_awaitable),
+                              buffer::errc::end_of_buffer);
+  }
+
+  {
+    neorados::ReadOp op;
+    op.exec("version", "read", {},
+           [](sys::error_code ec, const buffer::list& bl) {
+             throw std::exception();
+           });
+    co_await expect_error_code(rados().execute(oid, pool(), std::move(op),
+                                              nullptr, asio::use_awaitable),
+                              sys::errc::io_error);
+  }
+  co_return;
+}