Skip to content

Commit 74015fc

Browse files
rustyconoverclaude
andcommitted
http: worker errors from an HTTP error body carry the call's context
Over HTTP a worker error raised during a request comes back in the response body (X-VGI-RPC-Error), decoded by the shared POST helper with nothing but the request URL and no ClientContext. So, unlike the same error on a subprocess stream, it carried no execution_id, function_name or function_kind, reported the request route (".../vgi.v2/init/exchange") as worker_path, and dropped the worker's log lines that preceded the failure instead of sending them to duckdb_logs. Callers now pass an RpcErrorContext (worker base URL, execution, function fields) through HttpPostArrowIpc / HttpInvokeUnary to the decoder; the request route is reported as `url` and the method as `rpc_method`. An init reply that fails right after init carries the newly minted execution_id in its header stream, which the decoder adopts as it walks past. Requests without a context (the cancel dispatcher's, off the query thread) behave as before. error_info.test asserts execution_id and function_name on a mid-scan worker error, on every transport and SDK; it fails over HTTP without this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 323ae76 commit 74015fc

6 files changed

Lines changed: 127 additions & 24 deletions

File tree

‎src/include/vgi_http_client.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ UnaryResponseResult HttpInvokeUnary(ClientContext &context,
6969
const VgiProtocolId &protocol = VGI_MAIN_PROTOCOL,
7070
duckdb::unique_ptr<HTTPClient> *client_holder = nullptr,
7171
ServerCapabilities *caps = nullptr,
72-
const std::shared_ptr<IrohClientConfig> &iroh_config = nullptr);
72+
const std::shared_ptr<IrohClientConfig> &iroh_config = nullptr,
73+
const std::unordered_map<std::string, std::string> *error_fields = nullptr);
7374

7475
// Small thread-safe pool of keep-alive HTTPClients for unary RPCs against ONE
7576
// base URL (a pool instance lives on a catalog's VgiAttachParameters, whose
@@ -152,7 +153,8 @@ std::string HttpPostArrowIpc(ClientContext &context,
152153
duckdb::unique_ptr<HTTPClient> *client_holder = nullptr,
153154
ServerCapabilities *harvested_caps = nullptr,
154155
const std::shared_ptr<IrohClientConfig> &iroh_config = nullptr,
155-
const std::atomic<bool> *cancellation = nullptr);
156+
const std::atomic<bool> *cancellation = nullptr,
157+
const RpcErrorContext *error_context = nullptr);
156158

157159
// HTTP GET raw bytes from a URL. Used for fetching externalized batches.
158160
// Handles X-VGI-Content-Encoding decompression (zstd or gzip). No auth

‎src/include/vgi_rpc_client.hpp‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <memory>
99
#include <functional>
1010
#include <string>
11+
#include <unordered_map>
1112
#include <string_view>
1213
#include <utility>
1314
#include <vector>
@@ -341,6 +342,19 @@ UnaryResponseResult ReadUnaryResponseFromBuffer(const uint8_t *data, size_t len,
341342
const std::string &transaction_opaque_data_digest = "",
342343
const std::string &conn_id_hex = "");
343344

345+
// What a caller knows about the call an error belongs to, which the decoder of
346+
// a worker error body does not: the worker it is (as opposed to the request
347+
// URL), the function execution, and fields to attach to the error (function
348+
// name / kind). Carried to HandleBatchLogMessage so a worker error decoded
349+
// from an HTTP error body is reported like one read off a subprocess stream.
350+
struct RpcErrorContext {
351+
std::string worker_path; // the worker's base location; empty = the request URL
352+
std::string execution_id_hex;
353+
std::string attach_opaque_data_digest;
354+
std::string conn_id_hex;
355+
std::unordered_map<std::string, std::string> fields;
356+
};
357+
344358
// Dispatch every Arrow IPC stream concatenated in an error response body,
345359
// throwing on the first batch that carries error metadata.
346360
//
@@ -357,7 +371,7 @@ UnaryResponseResult ReadUnaryResponseFromBuffer(const uint8_t *data, size_t len,
357371
// instead of a generic transport failure. Safe for a unary body too, which is
358372
// simply one stream.
359373
void DispatchErrorStreamsFromBuffer(const uint8_t *data, size_t len, ClientContext *context,
360-
const std::string &url = "");
374+
const std::string &url = "", const RpcErrorContext *call = nullptr);
361375

362376
// Move-in overload: adopts the response body string as the owning Arrow buffer
363377
// (arrow::Buffer::FromString) instead of allocating + memcpying a copy of the

‎src/vgi_http_client.cpp‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,8 @@ static std::string HttpPostArrowIpcInternal(
312312
bool allow_codec_retry = true, duckdb::unique_ptr<HTTPClient> *client_holder = nullptr,
313313
ServerCapabilities *harvested_caps = nullptr,
314314
const std::shared_ptr<IrohClientConfig> &iroh_config = nullptr,
315-
const std::atomic<bool> *cancellation = nullptr) {
315+
const std::atomic<bool> *cancellation = nullptr,
316+
const RpcErrorContext *error_context = nullptr) {
316317
const bool is_httpi = IsHttpiTransport(url);
317318
const uint64_t timeout_seconds = GetHttpTimeoutSeconds(context);
318319
const int64_t accepted_max_response_bytes = GetAcceptedMaxResponseBytes(context);
@@ -539,7 +540,7 @@ static std::string HttpPostArrowIpcInternal(
539540
return HttpPostArrowIpcInternal(context, url, body, bearer_token, cookie_jar, out_response,
540541
cached_http_params, alternate,
541542
/*allow_codec_retry=*/false, client_holder, harvested_caps, iroh_config,
542-
cancellation);
543+
cancellation, error_context);
543544
}
544545
}
545546

@@ -740,8 +741,14 @@ static std::string HttpPostArrowIpcInternal(
740741
// header, returns without throwing, and loses the message to the
741742
// generic throw below. DispatchErrorStreamsFromBuffer dispatches
742743
// the error batch, which throws with the worker's own message.
744+
//
745+
// With a caller-supplied error context the errors name the worker,
746+
// the execution and the function, and the worker's own log lines
747+
// before the failure reach duckdb_logs, as on a subprocess stream.
748+
// Without one (e.g. the cancel dispatcher's request, which runs off
749+
// the query thread) nothing is logged, as before.
743750
DispatchErrorStreamsFromBuffer(reinterpret_cast<const uint8_t *>(error_body.data()), error_body.size(),
744-
nullptr, url);
751+
error_context ? &context : nullptr, url, error_context);
745752
}
746753
throw IOException(BuildHttpExtraInfo(http_error::kProtocolViolation, url),
747754
"VGI HTTP RPC error [url: %s]", url);
@@ -756,7 +763,7 @@ std::string HttpPostArrowIpc(ClientContext &context, const std::string &url, con
756763
const std::shared_ptr<HTTPParams> &cached_http_params,
757764
duckdb::unique_ptr<HTTPClient> *client_holder, ServerCapabilities *harvested_caps,
758765
const std::shared_ptr<IrohClientConfig> &iroh_config,
759-
const std::atomic<bool> *cancellation) {
766+
const std::atomic<bool> *cancellation, const RpcErrorContext *error_context) {
760767
// Get cached token from per-catalog auth (if any)
761768
std::string token;
762769
if (auth) {
@@ -772,7 +779,7 @@ std::string HttpPostArrowIpc(ClientContext &context, const std::string &url, con
772779
std::unique_ptr<RpcHttpResponse> response;
773780
auto result = HttpPostArrowIpcInternal(context, url, body, token, cookie_jar, response, cached_http_params,
774781
request_encoding, /*allow_codec_retry=*/true, client_holder, harvested_caps,
775-
iroh_config, cancellation);
782+
iroh_config, cancellation, error_context);
776783

777784
if (response->Status() != HTTPStatusCode::Unauthorized_401) {
778785
return result;
@@ -845,7 +852,7 @@ std::string HttpPostArrowIpc(ClientContext &context, const std::string &url, con
845852
result = HttpPostArrowIpcInternal(context, url, body, new_token, cookie_jar, response, cached_http_params,
846853
harvested_caps ? ChooseRequestEncoding(*harvested_caps) : request_encoding,
847854
/*allow_codec_retry=*/true, client_holder, harvested_caps, iroh_config,
848-
cancellation);
855+
cancellation, error_context);
849856
if (response->Status() == HTTPStatusCode::Unauthorized_401) {
850857
throw IOException(BuildHttpExtraInfo(http_error::kAuthFailed, url, 401),
851858
"VGI HTTP authentication failed after auth flow (HTTP 401) [url: %s]. "
@@ -865,7 +872,8 @@ UnaryResponseResult HttpInvokeUnary(ClientContext &context, const std::string &w
865872
const std::string &transaction_opaque_data_digest, const std::string &conn_id_hex,
866873
const VgiProtocolId &protocol,
867874
duckdb::unique_ptr<HTTPClient> *client_holder, ServerCapabilities *caps,
868-
const std::shared_ptr<IrohClientConfig> &iroh_config) {
875+
const std::shared_ptr<IrohClientConfig> &iroh_config,
876+
const std::unordered_map<std::string, std::string> *error_fields) {
869877
std::string base_url = NormalizeBaseUrl(worker_path);
870878
// Route shape: {base}/{protocol}/{method}. The protocol segment is the
871879
// projection of the vgi_rpc.protocol routing key the body also carries --
@@ -905,9 +913,21 @@ UnaryResponseResult HttpInvokeUnary(ClientContext &context, const std::string &w
905913
}
906914

907915
// POST to the route built above, using the standard HTTP timeout
916+
// A worker error answered in the body names the worker and this call, not
917+
// just the request URL; the URL rides along as its own field.
918+
RpcErrorContext error_context;
919+
error_context.worker_path = base_url;
920+
error_context.execution_id_hex = execution_id_hex;
921+
error_context.attach_opaque_data_digest = attach_opaque_data_digest;
922+
error_context.conn_id_hex = conn_id_hex;
923+
if (error_fields) {
924+
error_context.fields = *error_fields;
925+
}
926+
error_context.fields[error_key::kUrl] = RedactCredentials(url);
927+
error_context.fields.emplace(error_key::kRpcMethod, method_name);
908928
auto response_body =
909929
HttpPostArrowIpc(context, url, body, auth, cookie_jar, cached_http_params, client_holder,
910-
effective_caps, iroh_config);
930+
effective_caps, iroh_config, /*cancellation=*/nullptr, &error_context);
911931

912932
// Parse the Arrow IPC response. Move the body in — the string becomes the
913933
// owning Arrow buffer, avoiding an alloc+memcpy of the whole payload.

‎src/vgi_http_function_connection.cpp‎

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,26 @@ static ErrorInfo HttpFunctionInfo(const char *subtype, const std::string &base_u
5656
return info;
5757
}
5858

59+
// Error context for a request whose worker error comes back in the HTTP body:
60+
// the worker (base URL), this execution, and the function. `url` is the
61+
// request route, reported as its own field.
62+
static RpcErrorContext HttpCallErrorContext(const std::string &base_url, const std::string &url,
63+
const std::string &method, const std::string &function_name,
64+
const std::string &function_type, const std::string &execution_id_hex,
65+
const std::string &attach_opaque_data_digest,
66+
const std::string &conn_id_hex) {
67+
RpcErrorContext ctx;
68+
ctx.worker_path = base_url;
69+
ctx.execution_id_hex = execution_id_hex;
70+
ctx.attach_opaque_data_digest = attach_opaque_data_digest;
71+
ctx.conn_id_hex = conn_id_hex;
72+
auto info = HttpFunctionInfo(nullptr, base_url, function_name, function_type);
73+
info.erase(error_key::kErrorSubtype);
74+
info.Url(url).Rpc(method);
75+
ctx.fields.insert(info.begin(), info.end());
76+
return ctx;
77+
}
78+
5979
// Helper: parse ``vgi_batch_index`` off Arrow custom_metadata.
6080
// Returns INVALID_INDEX when the key is absent. Raises IOException
6181
// when present but un-parseable. The cap and monotonicity checks
@@ -423,10 +443,12 @@ BindResult HttpFunctionConnection::PerformBindRpc() {
423443
// In/out capability snapshot: picks this request's Content-Encoding and
424444
// is refreshed from the response (see CurrentCapabilities).
425445
ServerCapabilities b_caps = CurrentCapabilities();
446+
const auto call_error_fields = HttpFunctionInfo(nullptr, base_url_, function_name_, function_type_);
426447
auto resp = HttpInvokeUnary(context_, base_url_, "bind", rpc_params, auth,
427448
/*cookie_jar=*/nullptr, cached_params,
428449
"", "", "", "", VGI_MAIN_PROTOCOL, &http_client_, &b_caps,
429-
attach_params_ ? attach_params_->iroh() : nullptr);
450+
attach_params_ ? attach_params_->iroh() : nullptr,
451+
&call_error_fields);
430452
PublishHarvestedCapabilities(b_caps);
431453
if (!resp.batch || resp.batch->num_rows() == 0) {
432454
throw IOException(HttpFunctionInfo(error_subtype::kProtocolViolation, base_url_, function_name_, function_type_).Rpc("bind"), "Empty bind response from HTTP server [url: %s]", base_url_);
@@ -578,10 +600,12 @@ InitResult HttpFunctionConnection::PerformInit(const BindResult &bind_result,
578600
// codec is right on the first try (see CurrentCapabilities); refreshed from
579601
// the response below.
580602
ServerCapabilities harvested = CurrentCapabilities();
603+
const auto call_error_context = HttpCallErrorContext(base_url_, init_url, "init", function_name_, function_type_,
604+
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), GetConnIdHex());
581605
auto response_body = HttpPostArrowIpc(context_, init_url, body, auth,
582606
/*cookie_jar=*/nullptr, cached_params_init, &http_client_,
583607
&harvested, attach_params_ ? attach_params_->iroh() : nullptr,
584-
&context_.interrupted);
608+
&context_.interrupted, &call_error_context);
585609
PublishHarvestedCapabilities(harvested);
586610
#ifdef __EMSCRIPTEN__
587611
#endif
@@ -806,11 +830,13 @@ HttpFunctionConnection::RpcTableBufferingProcess(const std::string &function_nam
806830
auto cached_params = attach_params_ && IsHttpTransport(base_url_)
807831
? attach_params_->GetOrInitHttpParams(context_, base_url_) : nullptr;
808832
ServerCapabilities tb_caps = CurrentCapabilities();
833+
const auto call_error_fields = HttpFunctionInfo(nullptr, base_url_, function_name_, function_type_);
809834
auto resp = HttpInvokeUnary(context_, base_url_, "table_buffering_process", rpc_params, auth,
810835
/*cookie_jar=*/nullptr, cached_params,
811836
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), "", GetConnIdHex(),
812837
VGI_MAIN_PROTOCOL, &http_client_, &tb_caps,
813-
attach_params_ ? attach_params_->iroh() : nullptr);
838+
attach_params_ ? attach_params_->iroh() : nullptr,
839+
&call_error_fields);
814840
PublishHarvestedCapabilities(tb_caps);
815841
auto inner = DecodeHttpOuterResponse(resp, "table_buffering_process", base_url_);
816842
if (!inner || inner->num_rows() == 0) {
@@ -834,11 +860,13 @@ HttpFunctionConnection::RpcTableBufferingCombine(const std::string &function_nam
834860
auto cached_params = attach_params_ && IsHttpTransport(base_url_)
835861
? attach_params_->GetOrInitHttpParams(context_, base_url_) : nullptr;
836862
ServerCapabilities tb_caps = CurrentCapabilities();
863+
const auto call_error_fields = HttpFunctionInfo(nullptr, base_url_, function_name_, function_type_);
837864
auto resp = HttpInvokeUnary(context_, base_url_, "table_buffering_combine", rpc_params, auth,
838865
/*cookie_jar=*/nullptr, cached_params,
839866
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), "", GetConnIdHex(),
840867
VGI_MAIN_PROTOCOL, &http_client_, &tb_caps,
841-
attach_params_ ? attach_params_->iroh() : nullptr);
868+
attach_params_ ? attach_params_->iroh() : nullptr,
869+
&call_error_fields);
842870
PublishHarvestedCapabilities(tb_caps);
843871
auto inner = DecodeHttpOuterResponse(resp, "table_buffering_combine", base_url_);
844872
if (!inner || inner->num_rows() == 0) {
@@ -868,11 +896,13 @@ void HttpFunctionConnection::RpcTableBufferingDestructor(const std::string &func
868896
auto cached_params = attach_params_ && IsHttpTransport(base_url_)
869897
? attach_params_->GetOrInitHttpParams(context_, base_url_) : nullptr;
870898
ServerCapabilities tb_caps = CurrentCapabilities();
899+
const auto call_error_fields = HttpFunctionInfo(nullptr, base_url_, function_name_, function_type_);
871900
auto resp = HttpInvokeUnary(context_, base_url_, "table_buffering_destructor", rpc_params, auth,
872901
/*cookie_jar=*/nullptr, cached_params,
873902
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), "", GetConnIdHex(),
874903
VGI_MAIN_PROTOCOL, &http_client_, &tb_caps,
875-
attach_params_ ? attach_params_->iroh() : nullptr);
904+
attach_params_ ? attach_params_->iroh() : nullptr,
905+
&call_error_fields);
876906
PublishHarvestedCapabilities(tb_caps);
877907
auto inner = DecodeHttpOuterResponse(resp, "table_buffering_destructor", base_url_);
878908
(void)inner;
@@ -929,10 +959,12 @@ std::shared_ptr<arrow::RecordBatch> HttpFunctionConnection::ReadDataBatch() {
929959
auto p_cached_params = attach_params_ && IsHttpTransport(exchange_url)
930960
? attach_params_->GetOrInitHttpParams(context_, exchange_url) : nullptr;
931961
ServerCapabilities p_harvested = CurrentCapabilities();
962+
const auto call_error_context = HttpCallErrorContext(base_url_, exchange_url, "exchange", function_name_, function_type_,
963+
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), GetConnIdHex());
932964
auto response_body = HttpPostArrowIpc(context_, exchange_url, body, p_auth,
933965
/*cookie_jar=*/nullptr, p_cached_params, &http_client_,
934966
&p_harvested, attach_params_ ? attach_params_->iroh() : nullptr,
935-
&context_.interrupted);
967+
&context_.interrupted, &call_error_context);
936968
PublishHarvestedCapabilities(p_harvested);
937969

938970
// Parse response — buffer new data batches (adopt the body, no copy)
@@ -1007,10 +1039,12 @@ std::shared_ptr<arrow::RecordBatch> HttpFunctionConnection::ReadDataBatch() {
10071039
auto x_cached_params = attach_params_ && IsHttpTransport(exchange_url)
10081040
? attach_params_->GetOrInitHttpParams(context_, exchange_url) : nullptr;
10091041
ServerCapabilities x_harvested = CurrentCapabilities();
1042+
const auto call_error_context = HttpCallErrorContext(base_url_, exchange_url, "exchange", function_name_, function_type_,
1043+
GetExecutionIdHex(), GetAttachOpaqueDataDigest(), GetConnIdHex());
10101044
auto response_body = HttpPostArrowIpc(context_, exchange_url, body, x_auth,
10111045
/*cookie_jar=*/nullptr, x_cached_params, &http_client_,
10121046
&x_harvested, attach_params_ ? attach_params_->iroh() : nullptr,
1013-
&context_.interrupted);
1047+
&context_.interrupted, &call_error_context);
10141048
PublishHarvestedCapabilities(x_harvested);
10151049

10161050
// Parse response — copy into owning buffer since Arrow IPC reads zero-copy reference it

0 commit comments

Comments
 (0)