Skip to content

Commit ee16aa2

Browse files
DemetrisChrDemetrisChr
authored
CXXCBC-743: Update http_command to take handler with decoded response type (#853)
1 parent ad4836b commit ee16aa2

File tree

3 files changed

+56
-115
lines changed

3 files changed

+56
-115
lines changed

‎core/io/http_command.hxx‎

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,14 @@
3535

3636
namespace couchbase::core::operations
3737
{
38-
39-
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
40-
using http_command_handler = utils::movable_function<void(error_union, io::http_response&&)>;
41-
#else
42-
using http_command_handler = utils::movable_function<void(std::error_code, io::http_response&&)>;
43-
#endif
44-
4538
template<typename Request>
4639
struct http_command : public std::enable_shared_from_this<http_command<Request>> {
4740
using encoded_request_type = typename Request::encoded_request_type;
4841
using encoded_response_type = typename Request::encoded_response_type;
4942
using error_context_type = typename Request::error_context_type;
43+
using response_type = typename Request::response_type;
44+
using handler_type = utils::movable_function<void(response_type&&)>;
45+
5046
asio::steady_timer deadline;
5147
Request request;
5248
encoded_request_type encoded;
@@ -55,7 +51,7 @@ struct http_command : public std::enable_shared_from_this<http_command<Request>>
5551
std::shared_ptr<metrics::meter_wrapper> meter_{};
5652
std::shared_ptr<core::app_telemetry_meter> app_telemetry_meter_{ nullptr };
5753
std::shared_ptr<io::http_session> session_{};
58-
http_command_handler handler_{};
54+
handler_type handler_{};
5955
std::chrono::milliseconds timeout_{};
6056
std::string client_context_id_;
6157
std::shared_ptr<couchbase::tracing::request_span> parent_span_{ nullptr };
@@ -114,7 +110,7 @@ struct http_command : public std::enable_shared_from_this<http_command<Request>>
114110
span_ = nullptr;
115111
}
116112

117-
void start(http_command_handler&& handler)
113+
void start(handler_type&& handler)
118114
{
119115
span_ = tracer_->create_span(tracing::span_name_for_http_service(request.type), parent_span_);
120116
if (span_->uses_tags()) {
@@ -172,7 +168,7 @@ struct http_command : public std::enable_shared_from_this<http_command<Request>>
172168
span_->end();
173169
span_ = nullptr;
174170
}
175-
if (auto handler = std::move(handler_); handler) {
171+
if (handler_type handler = std::move(handler_); handler) {
176172
const auto& node_uuid = session_ ? session_->node_uuid() : "";
177173
auto telemetry_recorder = app_telemetry_meter_->value_recorder(node_uuid, {});
178174
telemetry_recorder->update_counter(total_counter_for_service_type(request.type));
@@ -189,13 +185,39 @@ struct http_command : public std::enable_shared_from_this<http_command<Request>>
189185
} else if (ec == errc::common::request_canceled) {
190186
telemetry_recorder->update_counter(canceled_counter_for_service_type(request.type));
191187
}
188+
encoded_response_type encoded_resp{ std::move(msg) };
189+
error_context_type ctx{};
192190
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
193-
handler(error, std::move(msg));
194-
}
195-
dispatch_deadline_.cancel();
191+
if (!std::holds_alternative<std::monostate>(error)) {
192+
if (std::holds_alternative<impl::bootstrap_error>(error)) {
193+
auto bootstrap_error = std::get<impl::bootstrap_error>(error);
194+
if (bootstrap_error.ec == errc::common::unambiguous_timeout) {
195+
CB_LOG_DEBUG("Timeout caused by bootstrap error. code={}, ec_message={}, message={}.",
196+
bootstrap_error.ec.value(),
197+
bootstrap_error.ec.message(),
198+
bootstrap_error.error_message);
199+
}
200+
ctx.ec = bootstrap_error.ec;
201+
} else {
202+
ctx.ec = std::get<std::error_code>(error);
203+
}
204+
}
196205
#else
197-
handler(ec, std::move(msg));
206+
ctx.ec = ec;
207+
#endif
208+
ctx.client_context_id = client_context_id_;
209+
ctx.method = encoded.method;
210+
ctx.path = encoded.path;
211+
ctx.http_status = encoded_resp.status_code;
212+
ctx.http_body = encoded_resp.body.data();
213+
ctx.last_dispatched_from = session_->local_address();
214+
ctx.last_dispatched_to = session_->remote_address();
215+
ctx.hostname = session_->http_context().hostname;
216+
ctx.port = session_->http_context().port;
217+
handler(request.make_response(std::move(ctx), std::move(encoded_resp)));
198218
}
219+
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
220+
dispatch_deadline_.cancel();
199221
#endif
200222
deadline.cancel();
201223
}

‎core/io/http_session_manager.hxx‎

Lines changed: 17 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -238,47 +238,26 @@ public:
238238
app_telemetry_meter_,
239239
options_.default_timeout_for(request.type));
240240
#endif
241-
242241
cmd->start([start = std::chrono::steady_clock::now(),
243242
self = shared_from_this(),
244243
type,
245244
cmd,
246-
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
247-
handler = collector->build_reporter()](error_union err,
248-
io::http_response&& msg) {
249-
diag::ping_state state = diag::ping_state::ok;
250-
std::optional<std::string> error{};
251-
if (!std::holds_alternative<std::monostate>(err)) {
252-
auto ec = std::holds_alternative<impl::bootstrap_error>(err)
253-
? std::get<impl::bootstrap_error>(err).ec
254-
: std::get<std::error_code>(err);
255-
if (ec) {
256-
if (ec == errc::common::unambiguous_timeout ||
257-
ec == errc::common::ambiguous_timeout) {
258-
state = diag::ping_state::timeout;
259-
} else {
260-
state = diag::ping_state::error;
261-
}
262-
error.emplace(fmt::format(
263-
"code={}, message={}, http_code={}", ec.value(), ec.message(), msg.status_code));
264-
}
265-
}
266-
#else
267-
handler = collector->build_reporter()](std::error_code ec,
268-
io::http_response&& msg) {
245+
handler =
246+
collector->build_reporter()](operations::http_noop_response&& resp) {
269247
diag::ping_state state = diag::ping_state::ok;
270248
std::optional<std::string> error{};
271-
if (ec) {
249+
if (auto ec = resp.ctx.ec; ec) {
272250
if (ec == errc::common::unambiguous_timeout ||
273251
ec == errc::common::ambiguous_timeout) {
274252
state = diag::ping_state::timeout;
275253
} else {
276254
state = diag::ping_state::error;
277255
}
278-
error.emplace(fmt::format(
279-
"code={}, message={}, http_code={}", ec.value(), ec.message(), msg.status_code));
256+
error.emplace(fmt::format("code={}, message={}, http_code={}",
257+
ec.value(),
258+
ec.message(),
259+
resp.ctx.http_status));
280260
}
281-
#endif
282261
auto remote_address = cmd->session_->remote_address();
283262
// If not connected, the remote address will be empty. Better to
284263
// give the user some context on the "attempted" remote address.
@@ -481,8 +460,8 @@ public:
481460
if (error) {
482461
typename Request::error_context_type ctx{};
483462
ctx.ec = error;
484-
using response_type = typename Request::encoded_response_type;
485-
return handler(request.make_response(std::move(ctx), response_type{}));
463+
using encoded_response_type = typename Request::encoded_response_type;
464+
return handler(request.make_response(std::move(ctx), encoded_response_type{}));
486465
}
487466

488467
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
@@ -494,8 +473,6 @@ public:
494473
app_telemetry_meter_,
495474
options_.default_timeout_for(request.type),
496475
dispatch_timeout_);
497-
cmd->start([self = shared_from_this(), cmd, handler = std::forward<Handler>(handler)](
498-
error_union err, io::http_response&& msg) mutable {
499476
#else
500477
auto cmd = std::make_shared<operations::http_command<Request>>(
501478
ctx_,
@@ -504,42 +481,12 @@ public:
504481
meter_,
505482
app_telemetry_meter_,
506483
options_.default_timeout_for(request.type));
507-
cmd->start([self = shared_from_this(), cmd, handler = std::forward<Handler>(handler)](
508-
std::error_code ec, io::http_response&& msg) mutable {
509-
#endif
510-
using command_type = typename decltype(cmd)::element_type;
511-
using encoded_response_type = typename command_type::encoded_response_type;
512-
using error_context_type = typename command_type::error_context_type;
513-
encoded_response_type resp{ std::move(msg) };
514-
error_context_type ctx{};
515-
#ifdef COUCHBASE_CXX_CLIENT_COLUMNAR
516-
if (!std::holds_alternative<std::monostate>(err)) {
517-
if (std::holds_alternative<impl::bootstrap_error>(err)) {
518-
auto bootstrap_error = std::get<impl::bootstrap_error>(err);
519-
if (bootstrap_error.ec == errc::common::unambiguous_timeout) {
520-
CB_LOG_DEBUG("Timeout caused by bootstrap error. code={}, ec_message={}, message={}.",
521-
bootstrap_error.ec.value(),
522-
bootstrap_error.ec.message(),
523-
bootstrap_error.error_message);
524-
}
525-
ctx.ec = bootstrap_error.ec;
526-
} else {
527-
ctx.ec = std::get<std::error_code>(err);
528-
}
529-
}
530-
#else
531-
ctx.ec = ec;
532484
#endif
533-
ctx.client_context_id = cmd->client_context_id_;
534-
ctx.method = cmd->encoded.method;
535-
ctx.path = cmd->encoded.path;
536-
ctx.http_status = resp.status_code;
537-
ctx.http_body = resp.body.data();
538-
ctx.last_dispatched_from = cmd->session_->local_address();
539-
ctx.last_dispatched_to = cmd->session_->remote_address();
540-
ctx.hostname = cmd->session_->http_context().hostname;
541-
ctx.port = cmd->session_->http_context().port;
542-
handler(cmd->request.make_response(std::move(ctx), std::move(resp)));
485+
486+
using response_type = typename Request::response_type;
487+
cmd->start([self = shared_from_this(), cmd, handler = std::forward<Handler>(handler)](
488+
response_type&& resp) mutable {
489+
handler(std::move(resp));
543490
self->check_in(cmd->request.type, cmd->session_);
544491
});
545492
cmd->set_command_session(session);
@@ -794,39 +741,10 @@ private:
794741
app_telemetry_meter_,
795742
options_.default_timeout_for(request.type),
796743
dispatch_timeout_);
744+
using response_type = typename Request::response_type;
797745
cmd->start([self = shared_from_this(), cmd, handler = std::forward<Handler>(handler)](
798-
error_union err, io::http_response&& msg) mutable {
799-
using command_type = typename decltype(cmd)::element_type;
800-
using encoded_response_type = typename command_type::encoded_response_type;
801-
using error_context_type = typename command_type::error_context_type;
802-
encoded_response_type resp{ std::move(msg) };
803-
error_context_type ctx{};
804-
if (!std::holds_alternative<std::monostate>(err)) {
805-
if (std::holds_alternative<impl::bootstrap_error>(err)) {
806-
auto bootstrap_error = std::get<impl::bootstrap_error>(err);
807-
if (bootstrap_error.ec == errc::common::unambiguous_timeout) {
808-
CB_LOG_DEBUG("Timeout caused by bootstrap error. code={}, ec_message={}, message={}.",
809-
bootstrap_error.ec.value(),
810-
bootstrap_error.ec.message(),
811-
bootstrap_error.error_message);
812-
}
813-
ctx.ec = bootstrap_error.ec;
814-
} else {
815-
ctx.ec = std::get<std::error_code>(err);
816-
}
817-
}
818-
ctx.client_context_id = cmd->client_context_id_;
819-
ctx.method = cmd->encoded.method;
820-
ctx.path = cmd->encoded.path;
821-
ctx.http_status = resp.status_code;
822-
ctx.http_body = resp.body.data();
823-
if (cmd->session_) {
824-
ctx.last_dispatched_from = cmd->session_->local_address();
825-
ctx.last_dispatched_to = cmd->session_->remote_address();
826-
ctx.hostname = cmd->session_->http_context().hostname;
827-
ctx.port = cmd->session_->http_context().port;
828-
}
829-
handler(cmd->request.make_response(std::move(ctx), std::move(resp)));
746+
response_type&& resp) mutable {
747+
handler(std::move(resp));
830748
self->check_in(cmd->request.type, cmd->session_);
831749
});
832750
CB_LOG_DEBUG(R"(Adding HTTP request to deferred queue: {}, client_context_id="{}")",

‎core/io/mcbp_command.hxx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ struct mcbp_command : public std::enable_shared_from_this<mcbp_command<Manager,
403403
}
404404

405405
private:
406-
auto create_orphan_attributes() -> orphan_attributes
406+
[[nodiscard]] auto create_orphan_attributes() -> orphan_attributes
407407
{
408408
orphan_attributes attrs;
409409

@@ -424,7 +424,8 @@ private:
424424
return attrs;
425425
}
426426

427-
auto create_dispatch_span() const -> std::shared_ptr<couchbase::tracing::request_span>
427+
[[nodiscard]] auto create_dispatch_span() const
428+
-> std::shared_ptr<couchbase::tracing::request_span>
428429
{
429430
std::shared_ptr<couchbase::tracing::request_span> dispatch_span =
430431
manager_->tracer()->create_span(tracing::operation::step_dispatch, span_);

0 commit comments

Comments
 (0)