diff --git a/DESCRIPTION b/DESCRIPTION index 30a7c0d..2ef565c 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -31,7 +31,8 @@ Imports: fauxpas, crul (>= 0.7.0), base64enc, - rlang + rlang, + cli Suggests: testthat, xml2, diff --git a/NAMESPACE b/NAMESPACE index daa9675..c1b30d8 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -63,12 +63,21 @@ export(wi_th) export(wi_th_) importFrom(R6,R6Class) importFrom(base64enc,base64encode) +importFrom(cli,ansi_collapse) +importFrom(cli,cli_abort) +importFrom(cli,format_error) importFrom(crul,mock) importFrom(fauxpas,HTTPRequestTimeout) importFrom(magrittr,"%>%") importFrom(rlang,abort) +importFrom(rlang,caller_arg) +importFrom(rlang,caller_env) importFrom(rlang,check_installed) importFrom(rlang,is_empty) +importFrom(rlang,is_error) +importFrom(rlang,is_function) importFrom(rlang,is_list) importFrom(rlang,is_na) importFrom(rlang,is_null) +importFrom(rlang,try_fetch) +importFrom(rlang,warn) diff --git a/R/RequestPattern.R b/R/RequestPattern.R index 0aaeedb..2e2f31b 100644 --- a/R/RequestPattern.R +++ b/R/RequestPattern.R @@ -116,7 +116,7 @@ RequestPattern <- R6::R6Class( #' @param request_signature a [RequestSignature] object #' @return a boolean matches = function(request_signature) { - assert(request_signature, "RequestSignature") + assert_is(request_signature, "RequestSignature") c_type <- if (!is.null(request_signature$headers)) request_signature$headers$`Content-Type` else NULL if (!is.null(c_type)) c_type <- strsplit(c_type, ";")[[1]][1] self$method_pattern$matches(request_signature$method) && diff --git a/R/RequestRegistry.R b/R/RequestRegistry.R index 7c3c932..f8829bd 100644 --- a/R/RequestRegistry.R +++ b/R/RequestRegistry.R @@ -24,7 +24,7 @@ HashCounter <- R6::R6Class( #' @return nothing returned; registers request and iterates #' internal counter put = function(req_sig) { - assert(req_sig, "RequestSignature") + assert_is(req_sig, "RequestSignature") key <- req_sig$to_s() self$hash[[key]] <- list( key = key, @@ -37,7 +37,7 @@ HashCounter <- R6::R6Class( #' @param req_sig an object of class `RequestSignature` #' @return (integer) the count of how many times the request has been made get = function(req_sig) { - assert(req_sig, "RequestSignature") + assert_is(req_sig, "RequestSignature") self$hash[[req_sig$to_s()]]$count %||% 0 } ) diff --git a/R/StubbedRequest.R b/R/StubbedRequest.R index fbbd921..f2b6fea 100644 --- a/R/StubbedRequest.R +++ b/R/StubbedRequest.R @@ -23,7 +23,7 @@ StubCounter <- R6::R6Class( #' @param x an object of class `RequestSignature` #' @return nothing returned; registers request & iterates internal counter put = function(x) { - assert(x, "RequestSignature") + assert_is(x, "RequestSignature") key <- x$to_s() self$hash[[key]] <- list(key = key, sig = x) private$total <- private$total + 1 @@ -417,7 +417,7 @@ StubbedRequest <- R6::R6Class( ) basic_auth_header <- function(x) { - assert(x, "character") + assert_is(x, "character") stopifnot(length(x) == 1) encoded <- base64enc::base64encode(charToRaw(x)) return(paste0("Basic ", encoded)) diff --git a/R/adapter.R b/R/adapter.R index a986be6..193d13d 100644 --- a/R/adapter.R +++ b/R/adapter.R @@ -69,7 +69,7 @@ Adapter <- R6::R6Class("Adapter", #' @param quiet (logical) suppress messages? default: `FALSE` #' @return `TRUE`, invisibly enable = function(quiet = FALSE) { - assert(quiet, "logical") + assert_is(quiet, "logical") if (!quiet) message(sprintf("%s enabled!", self$name)) webmockr_lightswitch[[self$client]] <- TRUE @@ -84,7 +84,7 @@ Adapter <- R6::R6Class("Adapter", #' @param quiet (logical) suppress messages? default: `FALSE` #' @return `FALSE`, invisibly disable = function(quiet = FALSE) { - assert(quiet, "logical") + assert_is(quiet, "logical") if (!quiet) message(sprintf("%s disabled!", self$name)) webmockr_lightswitch[[self$client]] <- FALSE self$remove_stubs() diff --git a/R/error-handling.R b/R/error-handling.R new file mode 100644 index 0000000..c370211 --- /dev/null +++ b/R/error-handling.R @@ -0,0 +1,37 @@ +errors_to_skip_stub_removal <- function() { + mssgs <- c( + "\".data\" is missing", + "must be of class StubbedRequest", + "not registered", + "Unknown" + # , + # "all objects must be error classes" + ) + paste0(mssgs, collapse = "|") +} + +stub_removal_message <- c( + "Encountered an error constructing stub", + "Removed stub", + "To see a list of stubs run stub_registry()" +) + +#' Handle stub removal +#' @keywords internal +#' @param .data an object of class `StubbedRequest` required +#' @param code a code block. required +#' @return if no error, the result of running `code`; if an error occurs +#' [withCallingHandlers()] throws a warning and then the stub is removed +handle_stub_removal <- function(.data, code) { + withCallingHandlers({ + force(code) + }, + error = function(cnd) { + if (!grepl(errors_to_skip_stub_removal(), cnd$message)) { + warn(stub_removal_message) + remove_request_stub(.data) + } + }) +} + +# FIXME: add envir handling so that error message says the exported user fxn that the error occurred in diff --git a/R/last.R b/R/last.R index 8bf8d8d..9aa3735 100644 --- a/R/last.R +++ b/R/last.R @@ -24,6 +24,7 @@ last_request <- function() { #' Get the last stub created #' +#' @importFrom rlang is_empty #' @export #' @return `NULL` if no stubs found; otherwise the last stub created #' as a `StubbedRequest` class diff --git a/R/mock_file.R b/R/mock_file.R index f37c49c..13835d2 100644 --- a/R/mock_file.R +++ b/R/mock_file.R @@ -8,8 +8,8 @@ #' @examples #' mock_file(path = tempfile(), payload = "{\"foo\": \"bar\"}") mock_file <- function(path, payload) { - assert(path, "character") - assert(payload, c("character", "json")) + assert_is(path, "character") + assert_is(payload, c("character", "json")) structure(list(path = path, payload = payload), class = "mock_file") } #' @export diff --git a/R/partial.R b/R/partial.R index 7fad770..418f95b 100644 --- a/R/partial.R +++ b/R/partial.R @@ -40,7 +40,7 @@ #' # cleanup #' stub_registry_clear() including <- function(x) { - assert(x, "list") + assert_is(x, "list") class(x) <- "partial" attr(x, "partial_match") <- TRUE attr(x, "partial_type") <- "include" @@ -50,7 +50,7 @@ including <- function(x) { #' @export #' @rdname including excluding <- function(x) { - assert(x, "list") + assert_is(x, "list") class(x) <- "partial" attr(x, "partial_match") <- TRUE attr(x, "partial_type") <- "exclude" diff --git a/R/pluck_body.R b/R/pluck_body.R index cfe9220..2459429 100644 --- a/R/pluck_body.R +++ b/R/pluck_body.R @@ -42,7 +42,9 @@ pluck_body <- function(x) { assert_request <- function(x) { request_slots <- c("url", "method", "options", "headers") if (!is.list(x) || !all(request_slots %in% names(x))) { - stop(deparse(substitute(x)), " is not a valid request ", call. = FALSE) + webmockr_abort( + format_error("{.arg {deparse(substitute(x))}} is not a valid request") + ) } } diff --git a/R/stub_body_diff.R b/R/stub_body_diff.R index c7ebcd9..bf4abe3 100644 --- a/R/stub_body_diff.R +++ b/R/stub_body_diff.R @@ -44,7 +44,7 @@ stub_body_diff <- function(stub = last_stub(), request = last_request()) { abort(c("`stub` and/or `request` are NULL or otherwise empty", "see `?stub_body_diff`")) } - assert(stub, "StubbedRequest") - assert(request, "RequestSignature") + assert_is(stub, "StubbedRequest") + assert_is(request, "RequestSignature") diffobj::diffObj(stub$body, request$body) } diff --git a/R/stub_request.R b/R/stub_request.R index a0c77b0..87eb7a4 100644 --- a/R/stub_request.R +++ b/R/stub_request.R @@ -21,7 +21,7 @@ #' want to use a stub that was created after an earlier one that matches, #' remove the earlier one(s). #' -#' Note on `wi_th()`: If you pass `query` values are coerced to character +#' Note on `wi_th()`: If you pass `query`, values are coerced to character #' class in the recorded stub. You can pass numeric, integer, etc., but #' all will be coerced to character. #' @@ -41,6 +41,30 @@ #' #' @section Mocking writing to disk: #' See [mocking-disk-writing] +#' @section Error handling: +#' To construct stubs, one uses [stub_request()] first - which registers +#' the stub in the stub registry. Any additional calls to modify the stub +#' with for example [wi_th()] or [to_return()] can error. In those error +#' cases we ideally want to remove (unregister) the stub because you +#' certainly don't want a registered stub that is not exactly what you +#' intended. +#' +#' When you encounter an error creating a stub you should see a warning +#' message that the stub has been removed, for example: +#' +#' ``` +#' stub_request("get", "https://httpbin.org/get") %>% +#' wi_th(query = mtcars) +#' #> Error in `wi_th()`: +#' #> ! z$query must be of class list or partial +#' #> Run `rlang::last_trace()` to see where the error occurred. +#' #> Warning message: +#' #> Encountered an error constructing stub +#' #> • Removed stub +#' #> • To see a list of stubs run stub_registry() +#' ``` +#' +#' #' @seealso [wi_th()], [to_return()], [to_timeout()], [to_raise()], #' [mock_file()] #' @examples \dontrun{ @@ -190,7 +214,7 @@ #' stub_registry_clear() #' } stub_request <- function(method = "get", uri = NULL, uri_regex = NULL) { - if (is.null(uri) && is.null(uri_regex)) { + if (is_null(uri) && is_null(uri_regex)) { abort("one of uri or uri_regex is required") } tmp <- StubbedRequest$new(method = method, uri = uri, uri_regex = uri_regex) diff --git a/R/to_raise.R b/R/to_raise.R index 1d9e119..e7e2cd5 100644 --- a/R/to_raise.R +++ b/R/to_raise.R @@ -27,17 +27,20 @@ #' But for now, only the first exception is used until we get that fixed #' @note see examples in [stub_request()] to_raise <- function(.data, ...) { - assert(.data, "StubbedRequest") - tmp <- list(...) - if (!all(vapply( - tmp, function(x) inherits(x, "R6ClassGenerator"), - logical(1) - ))) { - abort("all objects must be error classes from fauxpas") - } - if (!all(vapply(tmp, function(x) grepl("HTTP", x$classname), logical(1)))) { - abort("all objects must be error classes from fauxpas") - } - .data$to_raise(tmp) + handle_stub_removal(.data, { + assert_is(.data, "StubbedRequest") + assert_stub_registered(.data) + tmp <- list(...) + if (!all(vapply( + tmp, function(x) inherits(x, "R6ClassGenerator"), + logical(1) + ))) { + abort("all objects must be error classes from fauxpas") + } + if (!all(vapply(tmp, function(x) grepl("HTTP", x$classname), logical(1)))) { + abort("all objects must be error classes from fauxpas") + } + .data$to_raise(tmp) + }) return(.data) } diff --git a/R/to_return.R b/R/to_return.R index e6d68d5..07938ba 100644 --- a/R/to_return.R +++ b/R/to_return.R @@ -72,23 +72,26 @@ #' # many of the same response using the times parameter #' foo() %>% to_return(body = "stuff", times = 3) to_return <- function(.data, ..., .list = list(), times = 1) { - assert(.data, "StubbedRequest") - assert(.list, "list") - assert(times, c("integer", "numeric")) - assert_gte(times, 1) - z <- list(...) - if (length(z) == 0) z <- NULL - z <- c(z, .list) - if ( - !any(c("status", "body", "headers") %in% names(z)) && - length(z) != 0 - ) { - abort("'to_return' only accepts status, body, headers") - } - assert(z$status, c("numeric", "integer")) - assert(z$headers, "list") - if (!all(hz_namez(z$headers))) abort("'headers' must be a named list") - replicate(times, - .data$to_return(status = z$status, body = z$body, headers = z$headers)) + handle_stub_removal(.data, { + assert_is(.data, "StubbedRequest") + assert_stub_registered(.data) + assert_is(.list, "list") + assert_is(times, c("integer", "numeric")) + assert_gte(times, 1) + z <- list(...) + if (length(z) == 0) z <- NULL + z <- c(z, .list) + if ( + !any(c("status", "body", "headers") %in% names(z)) && + length(z) != 0 + ) { + abort("'to_return' only accepts status, body, headers") + } + assert_is(z$status, c("numeric", "integer")) + assert_is(z$headers, "list") + if (!all(hz_namez(z$headers))) abort("'headers' must be a named list") + replicate(times, + .data$to_return(status = z$status, body = z$body, headers = z$headers)) + }) return(.data) } diff --git a/R/to_timeout.R b/R/to_timeout.R index 0ea3959..8ccd14e 100644 --- a/R/to_timeout.R +++ b/R/to_timeout.R @@ -7,7 +7,10 @@ #' the stub #' @note see examples in [stub_request()] to_timeout <- function(.data) { - assert(.data, "StubbedRequest") - .data$to_timeout() + handle_stub_removal(.data, { + assert_is(.data, "StubbedRequest") + assert_stub_registered(.data) + .data$to_timeout() + }) return(.data) } diff --git a/R/webmockr-opts.R b/R/webmockr-opts.R index 0481c3a..26ab572 100644 --- a/R/webmockr-opts.R +++ b/R/webmockr-opts.R @@ -83,7 +83,7 @@ webmockr_allow_net_connect <- function() { #' @export #' @rdname webmockr_configure webmockr_disable_net_connect <- function(allow = NULL) { - assert(allow, "character") + assert_is(allow, "character") message("net connect disabled") assign('allow_net_connect', FALSE, envir = webmockr_conf_env) assign('allow', allow, envir = webmockr_conf_env) @@ -92,7 +92,7 @@ webmockr_disable_net_connect <- function(allow = NULL) { #' @export #' @rdname webmockr_configure webmockr_net_connect_allowed <- function(uri = NULL) { - assert(uri, c("character", "list")) + assert_is(uri, c("character", "list")) if (is.null(uri)) return(webmockr_conf_env$allow_net_connect) uri <- normalize_uri(uri) webmockr_conf_env$allow_net_connect || diff --git a/R/webmockr-package.R b/R/webmockr-package.R index 79be6b6..d5b3b9c 100644 --- a/R/webmockr-package.R +++ b/R/webmockr-package.R @@ -22,6 +22,8 @@ #' @importFrom fauxpas HTTPRequestTimeout #' @importFrom crul mock #' @importFrom base64enc base64encode -#' @importFrom rlang abort check_installed is_list +#' @importFrom rlang abort warn check_installed is_list is_function is_error +#' caller_arg try_fetch caller_env +#' @importFrom cli cli_abort ansi_collapse format_error ## usethis namespace: end NULL diff --git a/R/wi_th.R b/R/wi_th.R index fb06a89..6f0b820 100644 --- a/R/wi_th.R +++ b/R/wi_th.R @@ -91,29 +91,34 @@ #' ## excluding #' wi_th(req, body = excluding(list(foo = "bar"))) wi_th <- function(.data, ..., .list = list()) { - assert(.data, "StubbedRequest") - assert(.list, "list") - z <- list(...) - if (length(z) == 0) z <- NULL - z <- c(z, .list) - if ( - !any(c("query", "body", "headers", "basic_auth") %in% names(z)) - && length(z) != 0 - ) { - abort("'wi_th' only accepts query, body, headers, basic_auth") - } - if (any(duplicated(names(z)))) abort("can not have duplicated names") - assert(z$query, c("list", "partial")) - if (!all(hz_namez(z$query))) abort("'query' must be a named list") - assert(z$headers, "list") - if (!all(hz_namez(z$headers))) abort("'headers' must be a named list") - assert(z$basic_auth, "character") - assert_eq(z$basic_auth, 2) - .data$with( - query = z$query, - body = z$body, - headers = z$headers, - basic_auth = z$basic_auth - ) + handle_stub_removal(.data, { + assert_is(.data, "StubbedRequest") + assert_stub_registered(.data) + assert_is(.list, "list") + z <- list(...) + if (length(z) == 0) z <- NULL + z <- c(z, .list) + if ( + !any(c("query", "body", "headers", "basic_auth") %in% names(z)) + && length(z) != 0 + ) { + abort("'wi_th' only accepts query, body, headers, basic_auth") + } + if (any(duplicated(names(z)))) abort("can not have duplicated names") + assert_is(z$query, c("list", "partial")) + if (!all(hz_namez(z$query))) abort("'query' must be a named list") + assert_is(z$headers, "list") + if (!all(hz_namez(z$headers))) abort("'headers' must be a named list") + assert_is(z$basic_auth, "character") + assert_eq(z$basic_auth, 2) + assert_not_function(z) + + .data$with( + query = z$query, + body = z$body, + headers = z$headers, + basic_auth = z$basic_auth + ) + }) return(.data) } diff --git a/R/zzz.R b/R/zzz.R index bb1503e..21c42f0 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -1,6 +1,6 @@ http_verbs <- c("any", "get", "post", "put", "patch", "head", "delete") -cc <- function(x) Filter(Negate(is.null), x) +cc <- function(x) Filter(Negate(is_null), x) is_nested <- function(x) { stopifnot(is.list(x)) @@ -28,11 +28,11 @@ subs <- function(x, n) { l2c <- function(w) paste(names(w), as.character(w), sep = " = ", collapse = "") has_attr <- function(x, at) { - !is.null(attr(x, at, exact = TRUE)) + !is_null(attr(x, at, exact = TRUE)) } hdl_lst <- function(x) { - if (is.null(x) || length(x) == 0) { + if (is_null(x) || length(x) == 0) { return("") } if (is.raw(x)) { @@ -71,7 +71,7 @@ hdl_lst <- function(x) { } hdl_lst2 <- function(x) { - if (is.null(x) || length(x) == 0) { + if (is_null(x) || length(x) == 0) { return("") } if (is.raw(x)) { @@ -116,13 +116,13 @@ url_builder <- function(uri, regex) { if (regex) uri else normalize_uri(uri) } # url_builder <- function(uri, args = NULL) { -# if (is.null(args)) return(uri) +# if (is_null(args)) return(uri) # paste0(uri, "?", paste(names(args), args, sep = "=", collapse = "&")) # } `%||%` <- function(x, y) { if ( - is.null(x) || length(x) == 0 || all(nchar(x) == 0) || all(is.na(x)) + is_null(x) || length(x) == 0 || all(nchar(x) == 0) || all(is.na(x)) ) { y } else { @@ -137,7 +137,7 @@ url_builder <- function(uri, regex) { return(y) } if ( - is.null(z) || length(z) == 0 || all(nchar(z) == 0) || all(is.na(z)) + is_null(z) || length(z) == 0 || all(nchar(z) == 0) || all(is.na(z)) ) { y } else { @@ -145,36 +145,54 @@ url_builder <- function(uri, regex) { } } -`!!` <- function(x) if (is.null(x) || is.na(x)) FALSE else TRUE +`!!` <- function(x) if (is_null(x) || is.na(x)) FALSE else TRUE -assert <- function(x, y) { - if (!is.null(x)) { +wr_col <- function(x) { + ansi_collapse(x, sep2 = " or ", last = ", or ") +} + +webmockr_abort <- function(message, call = caller_env(2)) { + cli_abort(message, call = call) +} + +assert_is <- function(x, y, arg = caller_arg(x)) { + if (!is_null(x)) { if (!inherits(x, y)) { - stop(deparse(substitute(x)), " must be of class ", - paste0(y, collapse = ", "), - call. = FALSE - ) + msg <- format_error("{.arg {arg}} must be of class {wr_col(y)}") + webmockr_abort(msg) } } } -assert_gte <- function(x, y) { +assert_gte <- function(x, y, arg = caller_arg(x)) { if (!x >= y) { - stop(sprintf( - "%s must be greater than or equal to %s", - deparse(substitute(x)), y - ), call. = FALSE) + msg <- format_error("{.arg {arg}} must be greater than or equal to {y}") + webmockr_abort(msg) } } -assert_eq <- function(x, y) { - if (!is.null(x)) { +assert_eq <- function(x, y, args = caller_arg(x)) { + if (!is_null(x)) { if (!length(x) == y) { - stop(sprintf( - "length of %s must be equal to %s", - deparse(substitute(x)), y - ), call. = FALSE) + msg <- format_error("length of {.arg {arg}} must be equal to {y}") + webmockr_abort(msg) } } } +assert_not_function <- function(x) { + for (i in seq_along(x)) { + if (!is_null(x[[i]])) { + if (is_function(x[[i]])) { + msg <- format_error("{names(x)[i]} must not be a function") + webmockr_abort(msg) + } + } + } +} +assert_stub_registered <- function(x) { + if (!webmockr_stub_registry$is_stubbed(x)) { + msg <- format_error("stub {substitute(x)} is not registered") + webmockr_abort(msg) + } +} crul_head_parse <- function(z) { if (grepl("HTTP\\/", z)) { @@ -194,9 +212,9 @@ crul_headers_parse <- function(x) do.call("c", lapply(x, crul_head_parse)) #' @param x an object #' @return a curl response webmockr_crul_fetch <- function(x) { - if (is.null(x$disk) && is.null(x$stream)) { + if (is_null(x$disk) && is_null(x$stream)) { curl::curl_fetch_memory(x$url$url, handle = x$url$handle) - } else if (!is.null(x$disk)) { + } else if (!is_null(x$disk)) { curl::curl_fetch_disk(x$url$url, x$disk, handle = x$url$handle) } else { curl::curl_fetch_stream(x$url$url, x$stream, handle = x$url$handle) @@ -207,7 +225,7 @@ webmockr_crul_fetch <- function(x) { along_rep <- function(x, y) rep(y, length.out = length(x)) hz_namez <- function(x) { nms <- names(x) - if (is.null(nms)) { + if (is_null(nms)) { along_rep(x, FALSE) } else { !(is.na(nms) | nms == "") diff --git a/man/handle_stub_removal.Rd b/man/handle_stub_removal.Rd new file mode 100644 index 0000000..0ac748a --- /dev/null +++ b/man/handle_stub_removal.Rd @@ -0,0 +1,21 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/error-handling.R +\name{handle_stub_removal} +\alias{handle_stub_removal} +\title{Handle stub removal} +\usage{ +handle_stub_removal(.data, code) +} +\arguments{ +\item{.data}{an object of class \code{StubbedRequest} required} + +\item{code}{a code block. required} +} +\value{ +if no error, the result of running \code{code}; if an error occurs +\code{\link[=withCallingHandlers]{withCallingHandlers()}} throws a warning and then the stub is removed +} +\description{ +Handle stub removal +} +\keyword{internal} diff --git a/man/stub_request.Rd b/man/stub_request.Rd index 178b043..5624533 100644 --- a/man/stub_request.Rd +++ b/man/stub_request.Rd @@ -36,7 +36,7 @@ If multiple stubs match the same request, we use the first stub. So if you want to use a stub that was created after an earlier one that matches, remove the earlier one(s). -Note on \code{wi_th()}: If you pass \code{query} values are coerced to character +Note on \code{wi_th()}: If you pass \code{query}, values are coerced to character class in the recorded stub. You can pass numeric, integer, etc., but all will be coerced to character. @@ -62,6 +62,30 @@ for a call to \link{grepl} See \link{mocking-disk-writing} } +\section{Error handling}{ + +To construct stubs, one uses \code{\link[=stub_request]{stub_request()}} first - which registers +the stub in the stub registry. Any additional calls to modify the stub +with for example \code{\link[=wi_th]{wi_th()}} or \code{\link[=to_return]{to_return()}} can error. In those error +cases we ideally want to remove (unregister) the stub because you +certainly don't want a registered stub that is not exactly what you +intended. + +When you encounter an error creating a stub you should see a warning +message that the stub has been removed, for example: + +\if{html}{\out{