prevent application of inline markup within rethrown messages (#870)

* prevent application of inline markup when rethrown messages fail to parse
* handle case when `parse_database_error()` succeeds

Author: simonpcouch

NEWS.md

@@ -1,5 +1,8 @@
 # odbc (development version)
 
+* Addressed issue where error messages rethrown from some drivers would be
+  garbled when the raw error message contained curly brackets 
+  (#859 by @simonpcouch).
 
 * snowflake: Runtime driver configuration checks on `MacOS` (#857).
 

R/utils.R

@@ -162,7 +162,7 @@ random_name <- function(prefix = "") {
 rethrow_database_error <- function(msg, call = trace_back()$call[[1]]) {
   tryCatch(
     res <- parse_database_error(msg),
-    error = function(e) cli::cli_abort(msg, call = call)
+    error = function(e) cli::cli_abort("{msg}", call = call)
   )
 
   cli::cli_abort(
@@ -232,6 +232,8 @@ set_database_error_names <- function(cnd_body) {
     return(cnd_body)
   }
 
+  cnd_body <- escape_curly_brackets(cnd_body)
+
   if (is.null(names(cnd_body))) {
     return(
       set_names(cnd_body, nm = c("x", rep("*", length(cnd_body) - 1)))
@@ -247,6 +249,19 @@ set_database_error_names <- function(cnd_body) {
   )
 }
 
+# Escape curly brackets before formatting with cli (#859). Do so only to
+# unnamed elements so that formatting is preserved for context
+# added with `contextualize_database_error()`.
+escape_curly_brackets <- function(cnd_body) {
+  if (is.null(names(cnd_body))) {
+    return(gsub("\\{", "{{", gsub("\\}", "}}", cnd_body)))
+  }
+
+  unnamed <- names(cnd_body) == ""
+  cnd_body[unnamed] <- gsub("\\{", "{{", gsub("\\}", "}}", cnd_body[unnamed]))
+  cnd_body
+}
+
 # check helpers for common odbc arguments --------------------------------------
 check_row.names <- function(row.names, call = caller_env()) {
   if (is.null(row.names) || is_scalar_logical(row.names) || is_string(row.names)) {

tests/testthat/_snaps/utils.md

@@ -102,6 +102,25 @@
       *  A network-related or instance-specific error has occurred while establishing a connection to 127.0.0.1. Server is not found or not accessible. Check if instance name is correct and if SQL Server is configured to allow remote connections. For more information see SQL Server Books Online.
       i From 'nanodbc/nanodbc.cpp:1147'.
 
+---
+
+    Code
+      rethrow_database_error(msg, call = NULL)
+    Condition
+      Error:
+      ! ODBC failed with error S1000 from [SAP AG][LIBODBCHDB DLL][HDBODBC][SAP AG][LIBODBCHDB SO][HDBODBC][89013].
+      x  General error;403 internal error: Error opening the cursor for the remote database <***.***> Connection not open;-10807 Connection down: Socket closed by peer {***.**.*.**:***** -> ***.**.***.**:***** TenantName:(none) SiteVolumeID:1:3 SiteType:PRIMARY ConnectionID:****** SessionID:************}
+      *  <SQL> 'SELECT DISTINCT "po_id", ***CENSORED***'
+      i From 'nanodbc/nanodbc.cpp:1722'.
+
+---
+
+    Code
+      rethrow_database_error(msg, call = NULL)
+    Condition
+      Error:
+      ! `parse_database_error()` will not {be able to parse this}, but it should still be successfully rethrown as-is.
+
 # check_row.names()
 
     Code

tests/testthat/test-utils.R

@@ -81,6 +81,15 @@ test_that("parse_database_error() works with messages from the wild", {
           [Microsoft][ODBC Driver 18 for SQL Server]TCP Provider: Error code 0x2726
           [Microsoft][ODBC Driver 18 for SQL Server]A network-related or instance-specific error has occurred while establishing a connection to 127.0.0.1. Server is not found or not accessible. Check if instance name is correct and if SQL Server is configured to allow remote connections. For more information see SQL Server Books Online. "
   expect_snapshot(error = TRUE, rethrow_database_error(msg, call = NULL))
+
+  # message contains brackets that may be interpreted as inline markup (#859)
+  msg <- "nanodbc/nanodbc.cpp:1722: S1000
+          [SAP AG][LIBODBCHDB DLL][HDBODBC] General error;403 internal error: Error opening the cursor for the remote database <***.***> [SAP AG][LIBODBCHDB SO][HDBODBC] Connection not open;-10807 Connection down: [89013] Socket closed by peer {***.**.*.**:***** -> ***.**.***.**:***** TenantName:(none) SiteVolumeID:1:3 SiteType:PRIMARY ConnectionID:****** SessionID:************}
+          <SQL> 'SELECT DISTINCT \"po_id\", ***CENSORED***'"
+  expect_snapshot(error = TRUE, rethrow_database_error(msg, call = NULL))
+
+  msg <- "`parse_database_error()` will not {be able to parse this}, but it should still be successfully rethrown as-is."
+  expect_snapshot(error = TRUE, rethrow_database_error(msg, call = NULL))
 })
 
 test_that("set_database_error_names() works (#840)", {