Skip to content

Commit

Permalink
Merge pull request #24059 from pgellert/fix/missing-error-handling-do…
Browse files Browse the repository at this point in the history
…wnload-manifest

CORE-8082 cloud_io: add missing error handling to `remote::download_object`
  • Loading branch information
pgellert authored Nov 8, 2024
2 parents 8ab558d + ad14537 commit 290ae07
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 8 deletions.
18 changes: 12 additions & 6 deletions src/v/cloud_io/remote.cc
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,18 @@ remote::download_object(download_request download_request) {

if (resp) {
vlog(ctxlog.debug, "Receive OK response from {}", path);
auto buffer
= co_await cloud_storage_clients::util::drain_response_stream(
resp.value());
download_request.payload.append_fragments(std::move(buffer));
transfer_details.on_success();
co_return download_result::success;
try {
auto buffer
= co_await cloud_storage_clients::util::drain_response_stream(
resp.value());
download_request.payload.append_fragments(std::move(buffer));
transfer_details.on_success();
co_return download_result::success;
} catch (...) {
resp
= cloud_storage_clients::util::handle_client_transport_error(
std::current_exception(), ctxlog);
}
}

lease.client->shutdown();
Expand Down
1 change: 1 addition & 0 deletions src/v/cloud_storage_clients/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ redpanda_cc_library(
"//src/v/utils:functional",
"//src/v/utils:log_hist",
"//src/v/utils:named_type",
"//src/v/utils:retry_chain_node",
"//src/v/utils:stop_signal",
"@boost//:beast",
"@boost//:lexical_cast",
Expand Down
9 changes: 8 additions & 1 deletion src/v/cloud_storage_clients/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "base/vlog.h"
#include "bytes/streambuf.h"
#include "net/connection.h"
#include "utils/retry_chain_node.h"

#include <boost/property_tree/xml_parser.hpp>

Expand All @@ -39,8 +40,9 @@ bool has_abort_or_gate_close_exception(const ss::nested_exception& ex) {
|| is_abort_or_gate_close_exception(ex.outer);
}

template<typename Logger>
error_outcome handle_client_transport_error(
std::exception_ptr current_exception, ss::logger& logger) {
std::exception_ptr current_exception, Logger& logger) {
auto outcome = error_outcome::retry;

try {
Expand Down Expand Up @@ -113,6 +115,11 @@ error_outcome handle_client_transport_error(
return outcome;
}

template error_outcome
handle_client_transport_error<ss::logger>(std::exception_ptr, ss::logger&);
template error_outcome handle_client_transport_error<retry_chain_logger>(
std::exception_ptr, retry_chain_logger&);

ss::future<iobuf>
drain_response_stream(http::client::response_stream_ref resp) {
const auto transfer_encoding = resp->get_headers().find(
Expand Down
3 changes: 2 additions & 1 deletion src/v/cloud_storage_clients/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ namespace cloud_storage_clients::util {
/// cloud provider (e.g. connection error).
/// \param current_exception is the current exception thrown by the client
/// \param logger is the logger to use
template<typename Logger>
error_outcome handle_client_transport_error(
std::exception_ptr current_exception, ss::logger& logger);
std::exception_ptr current_exception, Logger& logger);

/// \brief: Drain the reponse stream pointed to by the 'resp' handle into an
/// iobuf
Expand Down

0 comments on commit 290ae07

Please sign in to comment.