From 9639f3b0377e653aa97a5b85b4568eace5629e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABlle=20Salmon?= Date: Wed, 28 Feb 2024 13:36:55 +0100 Subject: [PATCH] refactor: remove tryCatch() that hides the error message --- DESCRIPTION | 1 - R/client.R | 150 ++++++++++++----------------------- tests/testthat/test-client.R | 30 ------- 3 files changed, 51 insertions(+), 130 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 018a44a..2581968 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -46,7 +46,6 @@ Suggests: skimr, testthat (>= 3.1.2), testthis, - webmockr, withr Config/Needs/readme: rerddap VignetteBuilder: diff --git a/R/client.R b/R/client.R index b82fc04..7bcbdf9 100644 --- a/R/client.R +++ b/R/client.R @@ -18,119 +18,71 @@ #' wfs <- emodnet_init_wfs_client(service = "bathymetry") #' } emodnet_init_wfs_client <- function(service, - service_version = NULL, - logger = NULL) { - deprecate_message_service_version( - service_version, - "deprecate_message_service_version" - ) - - check_service_name(service) - - service_url <- get_service_url(service) - - create_client <- function() { - wfs <- suppressWarnings( - ows4R::WFSClient$new( - service_url, - serviceVersion = "2.0.0", - headers = c("User-Agent" = emodnetwfs_user_agent()), - logger = logger - ) - ) - - check_wfs(wfs) - cli_alert_success("WFS client created successfully") - cli_alert_info("Service: {.val {wfs$getUrl()}}") - cli_alert_info("Version: {.val {wfs$getVersion()}}") - - wfs - } - - tryCatch( - create_client(), - error = function(e) { - check_service(perform_http_request(service_url)) - } - ) + service_version = NULL, + logger = NULL) { + deprecate_message_service_version( + service_version, + "deprecate_message_service_version" + ) + + check_service_name(service) + + service_url <- get_service_url(service) + + wfs <- suppressWarnings( + ows4R::WFSClient$new( + service_url, + serviceVersion = "2.0.0", + headers = c("User-Agent" = emodnetwfs_user_agent()), + logger = logger + ) + ) + + check_wfs(wfs) + cli_alert_success("WFS client created successfully") + cli_alert_info("Service: {.val {wfs$getUrl()}}") + cli_alert_info("Version: {.val {wfs$getVersion()}}") + + wfs } check_service_name <- function(service) { - checkmate::assert_choice(service, emodnet_wfs()$service_name) + checkmate::assert_choice(service, emodnet_wfs()$service_name) } check_wfs <- function(wfs) { - checkmate::assertR6( - wfs, - classes = c("WFSClient", "OWSClient", "OGCAbstractObject", "R6") - ) + checkmate::assertR6( + wfs, + classes = c("WFSClient", "OWSClient", "OGCAbstractObject", "R6") + ) } get_service_url <- function(service) { - emodnet_wfs()$service_url[emodnet_wfs()$service_name == service] + emodnet_wfs()$service_url[emodnet_wfs()$service_name == service] } get_service_name <- function(service_url) { - emodnet_wfs()$service_name[emodnet_wfs()$service_url == service_url] + emodnet_wfs()$service_name[emodnet_wfs()$service_url == service_url] } # Checks if there is internet and performs an HTTP GET request perform_http_request <- function(service_url) { - cli_alert_danger("WFS client creation failed.") - cli_alert_info("Service: {.val {service_url}}", .envir = parent.frame(n = 2)) - - has_internet <- function() { - if (nzchar(Sys.getenv("NO_INTERNET_TEST_EMODNET"))) { - return(FALSE) - } - curl::has_internet() - } - - if (!has_internet()) { - cli_alert_info("Reason: There is no internet connection") - return(NULL) - } - - service_url %>% - paste0("?request=GetCapabilities") %>% - httr::GET() -} - -# Checks if there is internet connection and HTTP status of the service -check_service <- function(request) { - if (is.null(request)) { - cli::cli_abort("WFS client creation failed.") - } - - if (httr::http_error(request)) { - cli_alert_danger("HTTP Status: {httr::http_status(request)$message}") - - is_monitor_up <- !is.null( - curl::nslookup("monitor.emodnet.eu", error = FALSE) - ) - if (interactive() && is_monitor_up) { - browse_monitor <- utils::askYesNo( - "Browse the EMODnet OGC monitor?", - FALSE, - prompts = "yes/no/cancel" - ) - if (is.na(browse_monitor)) browse_monitor <- FALSE - if (browse_monitor) { - utils::browseURL("https://monitor.emodnet.eu/resources?lang=en&resource_type=OGC:WFS") # nolint - } - } - - cli::cli_abort("Service creation failed") - - # If no HTTP status, something else is wrong - } else if (!httr::http_error(request)) { - cli_alert_info("HTTP Status: {.val {httr::http_status(request)$message}}") - - cli::cli_abort( - c( - "An exception has occurred.", - i = "Please raise an issue in {packageDescription('EMODnetWFS')$BugReports}" # nolint - ) - ) - } + cli_alert_danger("WFS client creation failed.") + cli_alert_info("Service: {.val {service_url}}", .envir = parent.frame(n = 2)) + + has_internet <- function() { + if (nzchar(Sys.getenv("NO_INTERNET_TEST_EMODNET"))) { + return(FALSE) + } + curl::has_internet() + } + + if (!has_internet()) { + cli_alert_info("Reason: There is no internet connection") + return(NULL) + } + + service_url %>% + paste0("?request=GetCapabilities") %>% + httr::GET() } diff --git a/tests/testthat/test-client.R b/tests/testthat/test-client.R index 08b688e..e83fab6 100644 --- a/tests/testthat/test-client.R +++ b/tests/testthat/test-client.R @@ -9,36 +9,6 @@ test_that("Error when wrong service", { expect_snapshot_error(emodnet_init_wfs_client("blop")) }) -test_that("Services down handled", { - skip_if_offline() - webmockr::httr_mock() - - test_url <- "https://demo.geo-solutions.it/geoserver/ows?request=GetCapabilities" # nolint - - webmockr::stub_request("get", uri = test_url) %>% - webmockr::wi_th( - headers = - list( - "Accept" = "application/json, text/xml, application/xml, */*" - ) - ) %>% - webmockr::to_return(status = 500) %>% - webmockr::to_return(status = 200) - - req_fail <- httr::GET(test_url) - req_success <- httr::GET(test_url) - - # Test requests - expect_true(httr::http_error(req_fail)) - expect_false(httr::http_error(req_success)) - - # Test check_service behavior - expect_snapshot_error(check_service(req_fail)) - expect_snapshot_error(check_service(req_success)) - - webmockr::disable() -}) - test_that("No internet challenge", { withr::local_envvar(list(NO_INTERNET_TEST_EMODNET = "bla"))