From 205795f23712a9f7e160e474c0bf3dddb98ac7db Mon Sep 17 00:00:00 2001 From: Wilmund Date: Tue, 22 Sep 2026 16:44:41 +0200 Subject: [PATCH] Fix graceful failure in check_api()/check_query() and geoentities_overlap validation - check_api()/check_query(): the return(NULL) inside the tryCatch error handler returned from the handler, not the function, so an unreachable server printed the intended message and then raised an uncaught httr2 error from a second, unguarded req_perform(). The result is now captured and NULL is returned from the function itself; the same response is reused for the status and body checks (one request instead of three). - GIFT_no_overlap(): the geoentities_overlap validation chained its clauses with && where the logic needs ||, so any malformed data frame passed validation and failed later inside dplyr::mutate_at() with an obscure message. Rewritten to the same shape used for taxonomy/species validation in GIFT_taxgroup(): require a data frame containing the seven expected columns. - Regression tests for both (test-check_functions.R is offline-safe: a refused local port takes the same path with or without internet). - NEWS.md entries. --- NEWS.md | 6 ++++ R/GIFT_no_overlap.R | 13 +++++---- R/check_functions.R | 41 +++++++++++++-------------- tests/testthat/test-GIFT_no_overlap.R | 8 ++++++ tests/testthat/test-check_functions.R | 13 +++++++++ 5 files changed, 54 insertions(+), 27 deletions(-) create mode 100644 tests/testthat/test-check_functions.R diff --git a/NEWS.md b/NEWS.md index c4fc9bde..318238b3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,12 @@ * Typo in the documentation of GIFT_taxonomy() * Disclaimer added to the vignettes +* check_api() and check_query() now return NULL with an informative message +when the server cannot be reached, instead of raising an uncaught error; +they also perform a single request instead of up to three +* Fixed the validation of the geoentities_overlap argument in +GIFT_no_overlap(): a malformed table is now rejected with the intended +error message instead of failing later with an obscure one # GIFT 1.3.4 * Fix in GIFT_env_meta_misc and GIFT_env_meta_raster diff --git a/R/GIFT_no_overlap.R b/R/GIFT_no_overlap.R index 17e82b3d..865a5439 100644 --- a/R/GIFT_no_overlap.R +++ b/R/GIFT_no_overlap.R @@ -87,15 +87,16 @@ GIFT_no_overlap <- function( kept.") } - if(!is.null(geoentities_overlap) && !is.data.frame(geoentities_overlap) && - ncol(geoentities_overlap) != 7 && - colnames(geoentities_overlap) != c("entity1", "entity2", "overlap12", - "overlap21", "area1", "area2", - "entity_class2")){ - stop("'geoentities_overlap' is a table coming from GIFT indicating the + if(!is.null(geoentities_overlap)){ + if(!is.data.frame(geoentities_overlap) || + !(all(c("entity1", "entity2", "overlap12", "overlap21", "area1", + "area2", "entity_class2") %in% + colnames(geoentities_overlap)))){ + stop("'geoentities_overlap' is a table coming from GIFT indicating the overlap in km^2 between pairs of polygons. It is automatically retrieved when 'geoentities_overlap' = NULL (default value of the function).") + } } GIFT_version <- check_gift_version_simple(GIFT_version) diff --git a/R/check_functions.R b/R/check_functions.R index 94158664..b08b2cd6 100644 --- a/R/check_functions.R +++ b/R/check_functions.R @@ -30,18 +30,18 @@ check_api <- function(api) { # Second check: web API available req <- httr2::request(api) - resp <- httr2::req_error(req, is_error = function(resp) FALSE) - + req <- httr2::req_error(req, is_error = function(resp) FALSE) + # test for non-existing APIs - tryCatch(expr = httr2::req_perform(resp), - error = function(expr) { - message( - "Either the API is wrongly specified or the server is down.") - return(NULL) - }) - + resp <- tryCatch(expr = httr2::req_perform(req), + error = function(expr) NULL) + if(is.null(resp)){ + message("Either the API is wrongly specified or the server is down.") + return(NULL) + } + # If the server was found, check for status - resp_status <- httr2::resp_status_desc(httr2::req_perform(resp)) + resp_status <- httr2::resp_status_desc(resp) if(resp_status == "Unauthorized"){ message("A password is needed for this restricted API.") return(NULL) @@ -49,7 +49,6 @@ check_api <- function(api) { message("Either the API is wrongly specified or the server is down.") return(NULL) } else{ - resp <- httr2::req_perform(req) if(httr2::resp_body_string(resp) != "No query selected."){ message("Either the API is wrongly specified or the server is down.") return(NULL) @@ -500,18 +499,18 @@ check_query <- function(api) { # Second check: web API available req <- httr2::request(api) - resp <- httr2::req_error(req, is_error = function(resp) FALSE) - + req <- httr2::req_error(req, is_error = function(resp) FALSE) + # test for non-existing APIs - tryCatch(expr = httr2::req_perform(resp), - error = function(expr) { - message( - "Either the API is wrongly specified or the server is down.") - return(NULL) - }) - + resp <- tryCatch(expr = httr2::req_perform(req), + error = function(expr) NULL) + if(is.null(resp)){ + message("Either the API is wrongly specified or the server is down.") + return(NULL) + } + # If the server was found, check for status - resp_status <- httr2::resp_status_desc(httr2::req_perform(resp)) + resp_status <- httr2::resp_status_desc(resp) if(resp_status == "Unauthorized"){ message("A password is needed for this restricted API.") return(NULL) diff --git a/tests/testthat/test-GIFT_no_overlap.R b/tests/testthat/test-GIFT_no_overlap.R index 4845e0a6..2c8a7627 100644 --- a/tests/testthat/test-GIFT_no_overlap.R +++ b/tests/testthat/test-GIFT_no_overlap.R @@ -45,4 +45,12 @@ test_that("invalid inputs", { of GIFT you want to use. Available options are 'latest', 'beta' and the different named stable versions of GIFT.", fixed = TRUE) + + expect_error( + GIFT_no_overlap(entity_IDs = c(1, 2), + geoentities_overlap = data.frame(foo = 1, bar = 2)), + "'geoentities_overlap' is a table coming from GIFT indicating the + overlap in km^2 between pairs of polygons. It is automatically + retrieved when 'geoentities_overlap' = NULL (default value of the + function).", fixed = TRUE) }) diff --git a/tests/testthat/test-check_functions.R b/tests/testthat/test-check_functions.R new file mode 100644 index 00000000..04f38aab --- /dev/null +++ b/tests/testthat/test-check_functions.R @@ -0,0 +1,13 @@ +# Tests for internal check functions ---- + +test_that("check_api() fails gracefully when the server is unreachable", { + # 127.0.0.1:9 refuses the connection immediately; before the fix the + # graceful message was followed by an uncaught httr2 error + expect_null(suppressMessages(check_api("http://127.0.0.1:9/"))) + expect_message(check_api("http://127.0.0.1:9/")) +}) + +test_that("check_query() fails gracefully when the server is unreachable", { + expect_null(suppressMessages(check_query("http://127.0.0.1:9/"))) + expect_message(check_query("http://127.0.0.1:9/")) +})