transition to cli errors

[simonpcouch]

Closes #627.

[simonpcouch]

Using caller_env(2) as caller_env() shows an unhelpful ".local()" frame:

test_con("SQLITE", attributes = list(boop = "bop"))
#> Error in `.local()`:
#> ! `attributes` does not support the connection attribute "boop".
#> ℹ Allowed connection attribute is "azure_token".

with:

 3.   └─odbc::dbConnect(...)
 4.     └─odbc (local) .local(drv, ...)
 5.       └─odbc:::OdbcConnection(...) at odbc/R/dbi-driver.R:184:5

[hadley]

FWIW, S4 adds the intermediate.local wrapper when the arguments of the generic and the method differ in some way. I think it happens here because our method adds the dsn argument before the docs. (No need to change anything, I just thought you might want to know where this was coming from)

[hadley]

Looking good!

[hadley]

FWIW, S4 adds the intermediate.local wrapper when the arguments of the generic and the method differ in some way. I think it happens here because our method adds the dsn argument before the docs. (No need to change anything, I just thought you might want to know where this was coming from)

[hadley]

:chef kiss:

[simonpcouch]

SUCH A SATISFYING DIFF

[mjkallen]

Is there a particular reason why some calls to warning() (like this one) were left in the code and not replaced with cli::cli_warn()? Or was your goal really to only cover errors?

I must say that I personally am not terribly happy with the switch to using cli_abort(), but I understand that the {odbc} package is an integral part of the database connections feature in the RStudio IDE and that it makes sense to follow the tidy design principle of being human centered. I just wish that it would be as easy to just call conditionMessage(x) on a condition object x returned by cli_abort (or any rlang_error object) to retrieve the original (i.e., unmodified) error message. Now, if you want to write these original error messages to a log file or display them in a Shiny application without all the extra bells and wistles added by cli_abort(), then you have to use a combination of rlang::cnd_message() and cli::ansi_strip() to get to the original message and even then I am unsure if this is guaranteed to return the original error/warning message...

[jan-swissre]

@mjkallen I had exactly same issue, not sure if that makes any difference but I invite you to upvote #950 so at least there might be placed to count users looking for the previous behavior.

I also noticed there is a class of errors that cli is not able to handle properly, and instead of getting the error I am interested in, I am getting new error raised by cli.

Could not parse cli `{}` expression: `0C358499-331A-4AB5-…`

As those errors occur randomly from within odbc-mssql, so I have nothing to file new issue reporting that.
I may eventually try odbc version from before this PR to possibly get more info about the actual error.