feat: Add rich ErrorData-based error handling with structured error information (#1479)

Co-authored-by: krlmlr <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: Kirill Müller <[email protected]>
Co-authored-by: Kirill Müller <[email protected]>

Author: 3 people

R/duckdb.R

@@ -5,3 +5,52 @@
 #' @keywords internal
 "_PACKAGE"
 NULL
+
+# Internal error function for C++ layer
+rapi_error <- function(context, message, error_type = NULL, raw_message = NULL, extra_info = NULL) {
+  if (is.null(error_type) && is.null(raw_message) && is.null(extra_info)) {
+    extra_text <- ""
+  } else {
+    extra_text <- paste0(
+      " (",
+      if (!is.null(error_type)) paste0("error_type: ", error_type, "; ") else "",
+      if (!is.null(raw_message)) paste0("raw_message: ", raw_message, "; ") else "",
+      if (!is.null(extra_info)) paste0(paste0(names(extra_info), ": ", extra_info, collapse = "; ")) else "",
+      ")"
+    )
+  }
+
+  stop(paste0(context, ": ", message, extra_text), call. = FALSE)
+}
+
+# rlang error function (will be conditionally replaced in .onLoad)
+rapi_error_rlang <- function(context, message, error_type = NULL, raw_message = NULL, extra_info = NULL) {
+  # Avoid initializing NULL fields
+  fields <- list()
+  fields$context <- context
+  fields$error_type <- error_type
+  fields$raw_message <- raw_message
+  fields$extra_info <- extra_info
+
+  # Create error message with context
+  error_parts <- c(message, i = paste0("Context: ", context))
+
+  # Add error type if available
+  if (!is.null(error_type)) {
+    error_parts <- c(error_parts, i = paste0("Error type: ", error_type))
+  }
+
+  # Add raw message if different from processed message
+  if (!is.null(raw_message) && raw_message != message) {
+    error_parts <- c(error_parts, i = paste0("Raw message: ", raw_message))
+  }
+
+  # Add extra info if available
+  if (length(extra_info) > 0) {
+    info_text <- paste0(names(extra_info), ": ", extra_info)
+    names(info_text) <- rep_len("i", length(info_text))
+    error_parts <- c(error_parts, info_text)
+  }
+
+  rlang::abort(error_parts, class = "duckdb_error", !!!fields)
+}

R/zzz.R

@@ -13,8 +13,10 @@
   if (requireNamespace("rlang", quietly = TRUE)) {
     is_interactive <<- rlang::is_interactive
     local_interactive <<- rlang::local_interactive
+    rapi_error <<- rapi_error_rlang
   } else {
     rethrow_restore()
+    # Overwrite rapi_error with base version when rlang is not available
   }
 
   invisible()

src/connection.cpp

@@ -56,11 +56,11 @@ static void SetDefaultConfigArguments(ClientContext &context) {
 
 [[cpp11::register]] duckdb::conn_eptr_t rapi_connect(duckdb::db_eptr_t dual, duckdb::ConvertOpts convert_opts) {
 	if (!dual || !dual.get()) {
-		cpp11::stop("rapi_connect: Invalid database reference");
+		rapi_error_with_context("rapi_connect", "Invalid database reference");
 	}
 	auto db = dual->get();
 	if (!db || !db->db) {
-		cpp11::stop("rapi_connect: Database already closed");
+		rapi_error_with_context("rapi_connect", "Database already closed");
 	}
 
 	auto conn_wrapper = make_uniq<ConnWrapper>(std::move(db), std::move(convert_opts));

src/convert.cpp

@@ -11,23 +11,23 @@ ConvertOpts::TzOutConvert string_to_tz_out_convert(const std::string &str) {
 		return ConvertOpts::TzOutConvert::WITH;
 	if (str == "force")
 		return ConvertOpts::TzOutConvert::FORCE;
-	cpp11::stop("Invalid tz_out_convert value: %s", str.c_str());
+	rapi_error_with_context("string_to_tz_out_convert", "Invalid tz_out_convert value: " + str);
 }
 
 ConvertOpts::BigIntType string_to_bigint_type(const std::string &str) {
 	if (str == "numeric")
 		return ConvertOpts::BigIntType::NUMERIC;
 	if (str == "integer64")
 		return ConvertOpts::BigIntType::INTEGER64;
-	cpp11::stop("Invalid bigint value: %s", str.c_str());
+	rapi_error_with_context("string_to_bigint_type", "Invalid bigint value: " + str);
 }
 
 ConvertOpts::ArrayConversion string_to_array_conversion(const std::string &str) {
 	if (str == "none")
 		return ConvertOpts::ArrayConversion::NONE;
 	if (str == "matrix")
 		return ConvertOpts::ArrayConversion::MATRIX;
-	cpp11::stop("Invalid array value: %s", str.c_str());
+	rapi_error_with_context("string_to_array_conversion", "Invalid array value: " + str);
 }
 
 ConvertOpts::ArrowConversion bool_to_arrow_conversion(bool use_arrow) {

src/database.cpp

@@ -40,7 +40,7 @@ static bool CastRstringToVarchar(Vector &source, Vector &result, idx_t count, Ca
 		try {
 			config.SetOptionByName(key, Value(val));
 		} catch (std::exception &e) {
-			cpp11::stop("rapi_startup: Failed to set configuration option: %s", e.what());
+			rapi_error_with_context("rapi_startup", e);
 		}
 	}
 
@@ -70,7 +70,7 @@ static bool CastRstringToVarchar(Vector &source, Vector &result, idx_t count, Ca
 			function.global_initialization = TableFunctionInitialization::INITIALIZE_ON_SCHEDULE;
 		}
 	} catch (std::exception &e) {
-		cpp11::stop("rapi_startup: Failed to open database: %s", e.what());
+		rapi_error_with_context("rapi_startup", e);
 	}
 	D_ASSERT(wrapper->db);
 
@@ -98,22 +98,22 @@ static bool CastRstringToVarchar(Vector &source, Vector &result, idx_t count, Ca
 
 [[cpp11::register]] bool rapi_lock(duckdb::db_eptr_t dual) {
 	if (!dual || !dual.get()) {
-		cpp11::stop("rapi_lock: Invalid database reference");
+		rapi_error_with_context("rapi_lock", "Invalid database reference");
 	}
 	dual->lock();
 	return dual->has();
 }
 
 [[cpp11::register]] void rapi_unlock(duckdb::db_eptr_t dual) {
 	if (!dual || !dual.get()) {
-		cpp11::stop("rapi_unlock: Invalid database reference");
+		rapi_error_with_context("rapi_unlock", "Invalid database reference");
 	}
 	dual->unlock();
 }
 
 [[cpp11::register]] bool rapi_is_locked(duckdb::db_eptr_t dual) {
 	if (!dual || !dual.get()) {
-		cpp11::stop("rapi_is_locked: Invalid database reference");
+		rapi_error_with_context("rapi_is_locked", "Invalid database reference");
 	}
 	return dual->is_locked();
 }

src/include/rapi.hpp

@@ -11,13 +11,19 @@
 #include "duckdb/common/unordered_map.hpp"
 #include "duckdb/parser/tableref/table_function_ref.hpp"
 #include "duckdb/common/mutex.hpp"
+#include "duckdb/common/error_data.hpp"
 
 #include "convert.hpp"
 
 #if defined(R_VERSION) && R_VERSION >= R_Version(4, 3, 0)
 #define R_HAS_ALTLIST
 #endif
 
+// Helper functions to communicate errors via R's stop() function with context information
+[[noreturn]] void rapi_error_with_context(const std::string &context, const std::string &message);
+[[noreturn]] void rapi_error_with_context(const std::string &context, const std::exception &e);
+[[noreturn]] void rapi_error_with_context(const std::string &context, const duckdb::ErrorData &error_data);
+
 namespace duckdb {
 
 typedef unordered_map<std::string, cpp11::list> arrow_scans_t;
@@ -39,7 +45,7 @@ class DualWrapper {
 	}
 	DualWrapper(DualWrapper *dual) : precious_(dual->get()) {
 		if (!precious_) {
-			cpp11::stop("dual is already released");
+			rapi_error_with_context("DualWrapper", "dual is already released");
 		}
 	}
 	~DualWrapper() {

src/register.cpp

@@ -15,13 +15,13 @@ using namespace duckdb;
 [[cpp11::register]] void rapi_register_df(duckdb::conn_eptr_t conn, std::string name, cpp11::data_frame value,
                                           duckdb::ConvertOpts convert_opts, bool overwrite) {
 	if (!conn || !conn.get() || !conn->conn) {
-		cpp11::stop("rapi_register_df: Invalid connection");
+		rapi_error_with_context("rapi_register_df", "Invalid connection");
 	}
 	if (name.empty()) {
-		cpp11::stop("rapi_register_df: Name cannot be empty");
+		rapi_error_with_context("rapi_register_df", "Name cannot be empty");
 	}
 	if (value.ncol() < 1) {
-		cpp11::stop("rapi_register_df: Data frame with at least one column required");
+		rapi_error_with_context("rapi_register_df", "Data frame with at least one column required");
 	}
 
 	ScopedInterruptHandler signal_handler(conn->conn->context);
@@ -38,7 +38,7 @@ using namespace duckdb;
 
 		static_cast<cpp11::sexp>(conn).attr("_registered_df_" + name) = value;
 	} catch (std::exception &e) {
-		cpp11::stop("rapi_register_df: Failed to register data frame: %s", e.what());
+		rapi_error_with_context("rapi_register_df", e);
 	}
 }
 
@@ -55,7 +55,7 @@ using namespace duckdb;
 	signal_handler.HandleInterrupt();
 
 	if (res->HasError()) {
-		cpp11::stop("%s", res->GetError().c_str());
+		rapi_error_with_context("rapi_unregister_df", res->GetError());
 	}
 }
 
@@ -279,10 +279,10 @@ unique_ptr<TableRef> duckdb::ArrowScanReplacement(ClientContext &context, Replac
 [[cpp11::register]] void rapi_register_arrow(duckdb::conn_eptr_t conn, std::string name, cpp11::list export_funs,
                                              cpp11::sexp valuesexp) {
 	if (!conn || !conn.get() || !conn->conn) {
-		cpp11::stop("rapi_register_arrow: Invalid connection");
+		rapi_error_with_context("rapi_register_arrow", "Invalid connection");
 	}
 	if (name.empty()) {
-		cpp11::stop("rapi_register_arrow: Name cannot be empty");
+		rapi_error_with_context("rapi_register_arrow", "Name cannot be empty");
 	}
 
 	auto stream_factory =
@@ -297,7 +297,8 @@ unique_ptr<TableRef> duckdb::ArrowScanReplacement(ClientContext &context, Replac
 		auto &arrow_scans = conn->db->arrow_scans;
 
 		for (auto e = arrow_scans.find(name); e != arrow_scans.end(); ++e) {
-			cpp11::stop("rapi_register_arrow: Arrow table '%s' already registered", name.c_str());
+			std::string error_msg = "Arrow table '" + name + "' already registered";
+			rapi_error_with_context("rapi_register_arrow", error_msg);
 		}
 
 		arrow_scans[name] = state_list;

src/reltoaltrep.cpp

@@ -90,18 +90,18 @@ void RelToAltrep::Initialize(DllInfo *dll) {
 template <class T>
 static T *GetFromExternalPtr(SEXP x) {
 	if (!x) {
-		cpp11::stop("GetFromExternalPtr: need a SEXP pointer");
+		rapi_error_with_context("GetFromExternalPtr", "need a SEXP pointer");
 	}
 	if (!ALTREP(x)) {
-		cpp11::stop("GetFromExternalPtr: not an ALTREP");
+		rapi_error_with_context("GetFromExternalPtr", "not an ALTREP");
 	}
 	auto ptr = R_altrep_data1(x);
 	if (TYPEOF(ptr) != EXTPTRSXP) {
-		cpp11::stop("GetFromExternalPtr: data1 is not an external pointer");
+		rapi_error_with_context("GetFromExternalPtr", "data1 is not an external pointer");
 	}
 	auto wrapper = (T *)R_ExternalPtrAddr(ptr);
 	if (!wrapper) {
-		cpp11::stop("GetFromExternalPtr: This looks like it has been freed");
+		rapi_error_with_context("GetFromExternalPtr", "This looks like it has been freed");
 	}
 	return wrapper;
 }
@@ -120,12 +120,12 @@ bool AltrepRelationWrapper::HasQueryResult() const {
 
 MaterializedQueryResult *AltrepRelationWrapper::GetQueryResult() {
 	if (!mat_error.empty()) {
-		cpp11::stop(mat_error);
+		rapi_error_with_context("GetQueryResult", mat_error);
 	}
 
 	if (!mat_result) {
 		if (n_cells == 0) {
-			cpp11::stop("Materialization is disabled, use collect() or as_tibble() to materialize.");
+			rapi_error_with_context("GetQueryResult", "Materialization is disabled, use `collect()` or `as_tibble()` to materialize.");
 		}
 
 		auto materialize_callback = Rf_GetOption1(RStrings::get().materialize_callback_sym);
@@ -153,7 +153,7 @@ MaterializedQueryResult *AltrepRelationWrapper::GetQueryResult() {
 		Materialize();
 
 		if (!mat_error.empty()) {
-			cpp11::stop(mat_error);
+			rapi_error_with_context("GetQueryResult", mat_error);
 		}
 
 		// FIXME: Use std::experimental::scope_exit
@@ -204,7 +204,7 @@ void AltrepRelationWrapper::Materialize() {
 		auto local_mat_res = (MaterializedQueryResult *)local_res.get();
 		if (local_mat_res->RowCount() > max_rows) {
 			mat_error = duckdb_fmt::format(
-			    "Materialization would result in more than {} rows. Use collect() or as_tibble() to materialize.",
+			    "Materialization would result in more than {} rows. Use `collect()` or `as_tibble()` to materialize.",
 			    max_rows);
 			return;
 		}
@@ -322,7 +322,7 @@ void *RelToAltrep::DoRownamesDataptrGet(SEXP x) {
 	auto rownames_wrapper = AltrepRownamesWrapper::Get(x);
 	auto row_count = rownames_wrapper->rel->GetQueryResult()->RowCount();
 	if (row_count > (idx_t)NumericLimits<int32_t>::Maximum()) {
-		cpp11::stop("Integer overflow for row.names attribute");
+		rapi_error_with_context("altrep_rownames_integer_Elt", "Integer overflow for row.names attribute");
 	}
 	rownames_wrapper->rowlen_data[1] = -row_count;
 	return rownames_wrapper->rowlen_data;
@@ -386,21 +386,21 @@ static R_altrep_class_t LogicalTypeToAltrepType(const LogicalType &type, const d
 #endif
 
 	default:
-		cpp11::stop("LogicalTypeToAltrepType: Column `%s` has no type for altrep: %s", name.c_str(),
-		            type.ToString().c_str());
+		std::string error_msg = "Column `" + name + "` has no type for altrep: " + type.ToString();
+		rapi_error_with_context("LogicalTypeToAltrepType", error_msg);
 	}
 }
 
 size_t DoubleToSize(double d) {
 	if (d < 0) {
-		cpp11::stop("rel_to_altrep: Negative size");
+		rapi_error_with_context("DoubleToSize", "Negative size");
 	}
 	if (!std::isfinite(d)) {
 		// Return maximum size_t for Inf
 		return MAX_SIZE_T;
 	}
 	if (d >= (double)MAX_SIZE_T) {
-		cpp11::stop("rel_to_altrep: Size overflow");
+		rapi_error_with_context("DoubleToSize", "Size overflow");
 	}
 	return (size_t)d;
 }
@@ -462,7 +462,7 @@ size_t DoubleToSize(double d) {
 shared_ptr<AltrepRelationWrapper> rapi_rel_wrapper_from_altrep_df(SEXP df, bool strict, bool allow_materialized) {
 	if (!Rf_inherits(df, "data.frame")) {
 		if (strict) {
-			cpp11::stop("rapi_rel_from_altrep_df: Not a data.frame");
+			rapi_error_with_context("rapi_rel_from_altrep_df", "Not a data.frame");
 		} else {
 			return nullptr;
 		}
@@ -471,7 +471,7 @@ shared_ptr<AltrepRelationWrapper> rapi_rel_wrapper_from_altrep_df(SEXP df, bool
 	auto row_names = get_attrib(df, R_RowNamesSymbol);
 	if (row_names == R_NilValue || !ALTREP(row_names)) {
 		if (strict) {
-			cpp11::stop("rapi_rel_from_altrep_df: Not a 'special' data.frame, row names are not ALTREP");
+			rapi_error_with_context("rapi_rel_from_altrep_df", "Not a 'special' data.frame, row names are not ALTREP");
 		} else {
 			return nullptr;
 		}
@@ -480,7 +480,7 @@ shared_ptr<AltrepRelationWrapper> rapi_rel_wrapper_from_altrep_df(SEXP df, bool
 	auto altrep_data = R_altrep_data1(row_names);
 	if (TYPEOF(altrep_data) != EXTPTRSXP) {
 		if (strict) {
-			cpp11::stop("rapi_rel_from_altrep_df: Not our 'special' data.frame, data1 is not external pointer");
+			rapi_error_with_context("rapi_rel_from_altrep_df", "Not our 'special' data.frame, data1 is not external pointer");
 		} else {
 			return nullptr;
 		}
@@ -489,7 +489,7 @@ shared_ptr<AltrepRelationWrapper> rapi_rel_wrapper_from_altrep_df(SEXP df, bool
 	auto tag = R_ExternalPtrTag(altrep_data);
 	if (tag != RStrings::get().duckdb_row_names_sym) {
 		if (strict) {
-			cpp11::stop("rapi_rel_from_altrep_df: Not our 'special' data.frame, tag missing");
+			rapi_error_with_context("rapi_rel_from_altrep_df", "Not our 'special' data.frame, tag missing");
 		} else {
 			return nullptr;
 		}