refactor: remove tryCatch() that hides the error message

EMODnet · Feb 28, 2024 · 605153f · 605153f
1 parent db36e14
commit 605153f
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 145 deletions.
diff --git 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
@@ -18,119 +18,49 @@
 #' 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]
-}
-
-# 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
-      )
-    )
-  }
+	emodnet_wfs()$service_name[emodnet_wfs()$service_url == service_url]
 }
diff --git a/tests/testthat/test-client.R b/tests/testthat/test-client.R
@@ -8,44 +8,3 @@ test_that("Specified connection works", {
 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"))
-
-  test_url <- "https://demo.geo-solutions.it/geoserver/ows?"
-
-  req_no_internet <- perform_http_request(test_url)
-
-  expect_null(req_no_internet)
-  expect_snapshot_error(check_service(req_no_internet))
-})