transition to cli errors

R/connection-pane.R

@@ -197,7 +197,7 @@ computeDisplayName <- function(connection) {
  display_name
 }
 
-validateObjectName <- function(table, view, ...) {
+validateObjectName <- function(table, view, ..., call = caller_env()) {
 
  # Handle view look-alike object types
  # ( e.g. PostgreSQL/"matview" )
@@ -207,14 +207,7 @@ validateObjectName <- function(table, view, ...) {
  view <- Reduce(`%||%`, args[viewlike], view)
 
  # Error if both table and view are passed
- if (!is.null(table) && !is.null(view)) {
- stop("`table` and `view` can not both be used", call. = FALSE)
- }
-
- # Error if neither table and view are passed
- if (is.null(table) && is.null(view)) {
- stop("`table` and `view` can not both be `NULL`", call. = FALSE)
- }
+ check_exclusive(table, view, .frame = call)
 
  table %||% view
 }
@@ -231,9 +224,10 @@ odbcListColumns.OdbcConnection <- function(connection,
  check_string(catalog, allow_null = TRUE)
  check_string(schema, allow_null = TRUE)
 
+ name <- validateObjectName(table, view, ...)
  # specify schema or catalog if given
  cols <- odbcConnectionColumns_(connection,
- name = validateObjectName(table, view, ...),
+ name = name,
  catalog_name = catalog,
  schema_name = schema
  )

R/dbi-connection.R

@@ -148,7 +148,7 @@ setMethod("dbQuoteIdentifier", c("OdbcConnection", "character"),
  return(DBI::SQL(character()))
  }
  if (any(is.na(x))) {
- stop("Cannot pass NA to dbQuoteIdentifier()", call. = FALSE)
+ cli::cli_abort("{.arg x} can't be {.code NA}.")
  }
  if (nzchar(conn@quote)) {
  x <- gsub(conn@quote, paste0(conn@quote, conn@quote), x, fixed = TRUE)

R/dbi-driver.R

@@ -251,14 +251,10 @@ odbc_data_type_df <- function(dbObj, obj, ...) {
  res <- character(NCOL(obj))
  nms <- names(obj)
  for (i in seq_along(obj)) {
- tryCatch(
+ withCallingHandlers(
  res[[i]] <- odbcDataType(con = dbObj, obj[[i]]),
- error = function(e) {
- if (conditionMessage(e) == "Unsupported type") {
- stop("Column '", nms[[i]], "' is of unsupported type: '", object_type(obj[[i]]), "'", call. = FALSE)
- } else {
- stop(e)
- }
+ error = function(err) {
+ cli::cli_abort("Can't determine type for column {nms[[i]]}.", parent = err, call = quote(odbcDataType()))
  }
  )
  }

R/dbi-result.R

@@ -63,7 +63,8 @@ setMethod("dbClearResult", "OdbcResult",
 #' @export
 setMethod("dbFetch", "OdbcResult",
  function(res, n = -1, ...) {
- n <- check_n(n)
+ check_number_whole(n, min = -1, allow_infinite = TRUE)
+ if (is.infinite(n)) n <- -1
  result_fetch(res@ptr, n)
  }
 )
@@ -92,7 +93,7 @@ setMethod("dbIsValid", "OdbcResult",
 setMethod("dbGetStatement", "OdbcResult",
  function(res, ...) {
  if (!dbIsValid(res)) {
- stop("Result already cleared", call. = FALSE)
+ cli::cli_abort("Result already cleared.")
  }
  res@statement
  }

R/dbi-table.R

@@ -344,7 +344,7 @@ setMethod("dbExistsTable", c("OdbcConnection", "SQL"),
 #' @export
 setMethod("dbExistsTable", c("OdbcConnection", "character"),
  function(conn, name, ...) {
- stopifnot(length(name) == 1)
+ check_string(name)
  df <- odbcConnectionTables(conn, name = name, ..., exact = TRUE)
  NROW(df) > 0
  }

R/driver-sql-server.R

@@ -70,7 +70,7 @@ setMethod("isTempTable", c("Microsoft SQL Server", "SQL"),
 #' @usage NULL
 setMethod("dbExistsTable", c("Microsoft SQL Server", "character"),
  function(conn, name, ...) {
- stopifnot(length(name) == 1)
+ check_string(name)
  if (isTempTable(conn, name, ...)) {
  query <- paste0("SELECT OBJECT_ID('tempdb..", name, "')")
  !is.na(dbGetQuery(conn, query)[[1]])

R/odbc-connection.R

@@ -14,11 +14,10 @@ OdbcConnection <- function(
  timeout = Inf,
  dbms.name = NULL,
  attributes = NULL,
- .connection_string = NULL
+ .connection_string = NULL,
+ call = caller_env(2)
 ) {
-
- stopifnot(all(has_names(attributes)))
- stopifnot(all(names(attributes) %in% SUPPORTED_CONNECTION_ATTRIBUTES))
+ check_attributes(attributes, call = call)
 
  args <- compact(list(...))
  check_args(args)
@@ -51,7 +50,13 @@ OdbcConnection <- function(
  info$dbms.name <- dbms.name
  }
  if (!nzchar(info$dbms.name)) {
- stop("The ODBC driver returned an invalid `dbms.name`. Please provide one manually with the `dbms.name` parameter.", call. = FALSE)
+ cli::cli_abort(
+ c(
+ "!" = "The ODBC driver failed to generate a DBMS name.",
+ "i" = "Please provide one manually with {.arg dbms.name}."
+ ),
+ call = call
+ )
  }
 
  class <- getClassDef(info$dbms.name, inherits = FALSE)

R/utils.R

@@ -40,17 +40,9 @@ lengths <- function(x) {
 }
 
 # A 'size' must be an integer greater than 1, returned as a double so we have a larger range
-parse_size <- function(x) {
- nme <- substitute(x) %||% "NULL"
-
- if (rlang::is_scalar_integerish(x) && !is.na(x) && !is.infinite(x) && x > 0) {
- return(as.numeric(x))
- }
-
- stop(
- sprintf("`%s` is not a valid size:\n Must be a positive integer.", as.character(nme)),
- call. = FALSE
- )
+parse_size <- function(x, arg = caller_arg(x), call = caller_env()) {
+ check_number_whole(x, min = 1, arg = arg, call = call)
+ as.numeric(x)
 }
 
 id_field <- function(id,
@@ -81,14 +73,6 @@ id_field <- function(id,
  }
 }
 
-check_n <- function(n) {
- if (length(n) != 1) stop("`n` must be scalar", call. = FALSE)
- if (n < -1) stop("`n` must be nonnegative or -1", call. = FALSE)
- if (is.infinite(n)) n <- -1
- if (trunc(n) != n) stop("`n` must be a whole number", call. = FALSE)
- n
-}
-
 isPatternValue <- function(val) {
  grepl("[%|_]", val)
 }
@@ -193,6 +177,23 @@ check_field.types <- function(field.types, call = caller_env()) {
  }
 }
 
+check_attributes <- function(attributes, call = caller_env()) {
+ if (!all(has_names(attributes))) {
+ cli::cli_abort("All elements of {.arg attributes} must be named.", call = call)
+ }
+
+ attributes_supported <- names(attributes) %in% SUPPORTED_CONNECTION_ATTRIBUTES
+ if (!all(attributes_supported)) {
+ cli::cli_abort(
+ c("!" = "{.arg attributes} does not support the connection attribute{?s} \\
+ {.val {names(attributes)[!attributes_supported]}}.",
+ "i" = "Allowed connection attribute{?s} {?is/are} \\
+ {.val {SUPPORTED_CONNECTION_ATTRIBUTES}}."),
+ call = call
+ )
+ }
+}
+
 # apple + spark drive config (#651) --------------------------------------------
 configure_spark <- function(call = caller_env()) {
  if (!is_macos()) {

tests/testthat/_snaps/dbi-connection.md

@@ -20,3 +20,11 @@
  Output
  <OdbcConnection>
 
+# dbQuoteIdentifier() errors informatively
+
+ Code
+ dbQuoteIdentifier(con, NA_character_)
+ Condition
+ Error in `dbQuoteIdentifier()`:
+ ! `x` can't be `NA`.
+

tests/testthat/_snaps/driver-sqlite.md

@@ -1,3 +1,13 @@
+# unsupported types gives informative error
+
+ Code
+ dbWriteTable(con, "df", df)
+ Condition
+ Error in `odbcDataType()`:
+ ! Can't determine type for column foo.
+ Caused by error:
+ ! Unsupported type
+
 # dbWriteTable() with `field.types` with `append = TRUE`
 
  Code

tests/testthat/_snaps/odbc-connection.md

@@ -46,3 +46,19 @@
  Error in `quote_value()`:
  ! Don't know how to escape a value with both single and double quotes.
 
+# validateObjectName() errors informatively
+
+ Code
+ odbcListColumns(con, table = "boop", view = "bop")
+ Condition
+ Error in `odbcListColumns()`:
+ ! Exactly one of `table` or `view` must be supplied.
+
+---
+
+ Code
+ odbcListColumns(con)
+ Condition
+ Error in `odbcListColumns()`:
+ ! One of `table` or `view` must be supplied.
+

tests/testthat/_snaps/utils.md

@@ -1,3 +1,11 @@
+# parse_size works
+
+ Code
+ parse_size("foo")
+ Condition
+ Error:
+ ! `"foo"` must be a whole number, not the string "foo".
+
 # id_field checks inputs
 
  Code
@@ -27,6 +35,24 @@
  Error in `dbWriteTable()`:
  ! `field.types` must be `NULL` or a named vector of field types, not a string.
 
+# check_attributes()
+
+ Code
+ con <- test_con("SQLITE", attributes = list(boop = "bop"))
+ Condition
+ Error in `dbConnect()`:
+ ! `attributes` does not support the connection attribute "boop".
+ i Allowed connection attribute is "azure_token".
+
+---
+
+ Code
+ con <- test_con("SQLITE", attributes = list(boop = "bop", beep = "boop"))
+ Condition
+ Error in `dbConnect()`:
+ ! `attributes` does not support the connection attributes "boop" and "beep".
+ i Allowed connection attribute is "azure_token".
+
 # configure_spark() errors informatively on failure to install unixODBC
 
  Code

tests/testthat/test-dbi-connection.R

@@ -25,3 +25,12 @@ test_that("show method does not print server if it is not available", {
  )
  expect_snapshot(con)
 })
+
+test_that("dbQuoteIdentifier() errors informatively", {
+ con <- test_con("SQLITE")
+
+ expect_snapshot(
+ error = TRUE,
+ dbQuoteIdentifier(con, NA_character_)
+ )
+})

tests/testthat/test-driver-sqlite.R

@@ -134,7 +134,10 @@ test_that("unsupported types gives informative error", {
  con <- test_con("SQLITE")
 
  df <- data.frame(foo = complex(1))
- expect_error(dbWriteTable(con, "df", df), "Column 'foo' is of unsupported type: 'complex'")
+ expect_snapshot(
+ error = TRUE,
+ dbWriteTable(con, "df", df)
+ )
 })
 
 test_that("odbcPreviewObject works", {

tests/testthat/test-odbc-connection.R

@@ -64,6 +64,21 @@ test_that("automatically picks correct quote type", {
  expect_snapshot(quote_value("'\""), error = TRUE)
 })
 
+# connections pane -------------------------------------------------------------
+test_that("validateObjectName() errors informatively", {
+ con <- test_con("SQLITE")
+
+ expect_snapshot(
+ error = TRUE,
+ odbcListColumns(con, table = "boop", view = "bop")
+ )
+
+ expect_snapshot(
+ error = TRUE,
+ odbcListColumns(con)
+ )
+})
+
 # odbcConnectionColumns deprecation --------------------------------------
 
 test_that("odbcConnectionColumns warns on usage (#699)", {

tests/testthat/test-utils.R

@@ -1,17 +1,5 @@
 test_that("parse_size works", {
- expect_error(parse_size("foo"), "Must be a positive integer")
- expect_error(parse_size(TRUE), "Must be a positive integer")
- expect_error(parse_size(0), "Must be a positive integer")
- expect_error(parse_size(-1), "Must be a positive integer")
- expect_error(parse_size(-.1), "Must be a positive integer")
- expect_error(parse_size(.1), "Must be a positive integer")
- expect_error(parse_size("0"), "Must be a positive integer")
- expect_error(parse_size("1"), "Must be a positive integer")
- expect_error(parse_size(Inf), "Must be a positive integer")
- expect_error(parse_size(NULL), "Must be a positive integer")
- expect_error(parse_size(1:2), "Must be a positive integer")
- expect_error(parse_size(numeric()), "Must be a positive integer")
- expect_error(parse_size(integer()), "Must be a positive integer")
+ expect_snapshot(error = TRUE, parse_size("foo"))
 
  expect_identical(parse_size(1L), 1)
  expect_identical(parse_size(1), 1)
@@ -73,6 +61,18 @@ test_that("check_field.types()", {
  )
 })
 
+test_that("check_attributes()", {
+ expect_snapshot(
+ error = TRUE,
+ con <- test_con("SQLITE", attributes = list(boop = "bop"))
+ )
+
+ expect_snapshot(
+ error = TRUE,
+ con <- test_con("SQLITE", attributes = list(boop = "bop", beep = "boop"))
+ )
+})
+
 test_that("configure_spark() returns early on windows", {
  local_mocked_bindings(is_macos = function() {FALSE})