Handle non-JSON Databricks error responses (#247)

Author: zacdav

NEWS.md

@@ -1,6 +1,8 @@
 # brickster (development version)
 
 -   Fixed `dbWriteTable()` and `dbAppendTable()` standard-path writes for binary columns, which now use Databricks `BINARY` types and `X'...'` literals when no staging volume is configured (#245)
+-   `dbConnect()` now preserves Databricks API error details when its validation query fails
+-   `db_perform_request()` and `db_perform_response()` now preserve useful error messages when Databricks returns non-JSON error bodies such as empty, plain-text, or HTML responses (#242)
 -   Fixed `git_source()` erroring when `type` was left at its default
 -   Fixed Unity Catalog volume file requests so `db_volume_*` paths containing spaces are encoded correctly (#231)
 -   `db_cluster_events()` now forwards the `event_types` argument to the API, which was previously ignored

R/databricks-dbi.R

@@ -168,7 +168,9 @@ setMethod(
       },
       error = function(e) {
         cli::cli_abort(
-          "Failed to connect to warehouse {.val {warehouse_id}}: {e$message}"
+          "Failed to connect to warehouse {.val {warehouse_id}}.",
+          parent = e,
+          call = NULL
         )
       }
     )

R/request-helpers.R

@@ -90,15 +90,29 @@ db_request <- function(
 #'
 #' @family Request Helpers
 db_req_error_body <- function(resp) {
-  json <- resp |> httr2::resp_body_json()
-  # if there is "message":
-  if ("message" %in% names(json)) {
-    paste(json$error_code, json$message, sep = ": ")
-  } else if (length(json) == 1) {
-    json[[1]]
-  } else {
-    paste(json, collapse = " ")
+  content_type <- httr2::resp_content_type(resp)
+
+  if (identical(content_type, "application/json")) {
+    json <- httr2::resp_body_json(resp)
+
+    # if there is "message":
+    if ("message" %in% names(json)) {
+      return(paste(json$error_code, json$message, sep = ": "))
+    } else {
+      return(paste(json, collapse = " "))
+    }
   }
+
+  body <- tryCatch(
+    httr2::resp_body_string(resp),
+    error = function(cnd) NULL
+  )
+
+  if (is.null(body) || !nzchar(trimws(body))) {
+    return(httr2::resp_status_desc(resp))
+  }
+
+  trimws(body)
 }
 
 #' Perform Databricks API Request

tests/testthat/test-databricks-dbi-offline-helpers.R

@@ -96,6 +96,35 @@ test_that("dbConnect validates tuning inputs and persists connection settings",
   )
 })
 
+test_that("dbConnect preserves validation query error details", {
+  drv <- DatabricksSQL()
+
+  local_mocked_bindings(
+    db_sql_query = function(...) {
+      cli::cli_abort(c(
+        "HTTP 403 Forbidden.",
+        "i" = "PERMISSION_DENIED: You do not have permission to use the SQL Warehouse."
+      ))
+    },
+    .package = "brickster"
+  )
+
+  expect_error(
+    dbConnect(
+      drv,
+      warehouse_id = "wh-1",
+      host = "mock_host",
+      token = "mock_token"
+    ),
+    regexp = paste(
+      "Failed to connect to warehouse",
+      "HTTP 403 Forbidden",
+      "PERMISSION_DENIED",
+      sep = ".*"
+    )
+  )
+})
+
 test_that("dbWriteTable validates user input", {
   con <- make_dbi_test_con(staging_volume = "/Volumes/c/s/v")
   value <- data.frame(x = 1:3)

tests/testthat/test-request-helpers.R

@@ -15,6 +15,14 @@ local_clear_auth_env <- function() {
   )
 }
 
+local_error_response <- function(body, content_type = "application/json", status = 400) {
+  httr2::response(
+    status_code = status,
+    headers = list("content-type" = content_type),
+    body = charToRaw(body)
+  )
+}
+
 test_that("request helpers - building requests", {
   local_clear_auth_env()
 
@@ -52,6 +60,44 @@ test_that("request helpers - building requests", {
   expect_null(db_request_json(NULL))
 })
 
+test_that("request error body handles standard Databricks JSON errors", {
+  resp <- local_error_response(
+    paste0(
+      '{"error_code":"PERMISSION_DENIED",',
+      '"message":"You do not have permission to use the SQL Warehouse."}'
+    ),
+    status = 403
+  )
+
+  expect_identical(
+    db_req_error_body(resp),
+    "PERMISSION_DENIED: You do not have permission to use the SQL Warehouse."
+  )
+})
+
+test_that("request error body handles non-JSON Databricks errors", {
+  text_html_resp <- local_error_response(
+    "Invalid Token",
+    content_type = "text/html; charset=utf-8"
+  )
+  string_resp <- local_error_response(
+    "DEADLINE_EXCEEDED: Deadline exceeded when awaiting statement ID",
+    content_type = "text/plain"
+  )
+  empty_resp <- local_error_response(
+    "",
+    content_type = "text/html; charset=utf-8",
+    status = 504
+  )
+
+  expect_identical(db_req_error_body(text_html_resp), "Invalid Token")
+  expect_identical(
+    db_req_error_body(string_resp),
+    "DEADLINE_EXCEEDED: Deadline exceeded when awaiting statement ID"
+  )
+  expect_identical(db_req_error_body(empty_resp), "Gateway Timeout")
+})
+
 test_that("request helpers - m2m auth flow", {
   local_clear_auth_env()