Skip to content

Commit 6ee8ffa

Browse files
author
alex-omophub
committed
Improve error handling in fhir_batch_to_tibble and enhance safe_get_concept function
- Updated fhir_batch_to_tibble to handle zero-length and non-character error values gracefully, preventing crashes from empty vectors. - Refined safe_get_concept to return structured results, allowing callers to distinguish between "not found" and other error types for better logging and reporting. - Adjusted example scripts to reflect the new structured error handling approach.
1 parent 466c0b3 commit 6ee8ffa

4 files changed

Lines changed: 96 additions & 16 deletions

File tree

R/fhir.R

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,10 @@ fhir_batch_to_tibble <- function(result, codings) {
216216
return(paste(names(err) %||% "", unlist(lapply(err, as.character)),
217217
sep = "=", collapse = "; "))
218218
}
219-
as.character(err)[[1L]]
219+
# Zero-length / non-character fallbacks: single-bracket indexing
220+
# returns NA on a length-0 vector instead of throwing. Guards
221+
# against `character(0)`, `integer(0)`, etc.
222+
as.character(err)[1L]
220223
}
221224

222225
make_row <- function(i) {

inst/examples/error_handling.R

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -158,32 +158,40 @@ cat("\n")
158158
cat("5. Graceful degradation\n")
159159
cat("-----------------------\n")
160160

161-
# Function that returns NULL on error instead of failing
161+
# Function that returns a structured result instead of failing. The
162+
# caller can distinguish "not found" from other error categories so
163+
# downstream logging / reporting is honest about what went wrong.
162164
safe_get_concept <- function(client, concept_id) {
163165
tryCatch(
164-
client$concepts$get(concept_id),
165-
httr2_http_404 = function(e) NULL,
166+
list(ok = TRUE, value = client$concepts$get(concept_id), reason = NA_character_),
167+
httr2_http_404 = function(e) {
168+
list(ok = FALSE, value = NULL, reason = "not found")
169+
},
166170
httr2_http = function(e) {
167-
message("HTTP error for concept ", concept_id, ": ",
168-
conditionMessage(e)[[1]])
169-
NULL
171+
list(
172+
ok = FALSE,
173+
value = NULL,
174+
reason = sprintf("HTTP error: %s", conditionMessage(e)[[1]])
175+
)
170176
},
171177
error = function(e) {
172-
message("Error fetching concept ", concept_id, ": ",
173-
conditionMessage(e)[[1]])
174-
NULL
178+
list(
179+
ok = FALSE,
180+
value = NULL,
181+
reason = sprintf("error: %s", conditionMessage(e)[[1]])
182+
)
175183
}
176184
)
177185
}
178186

179187
concept_ids <- c(201826, 999999999, 320128)
180-
cat(" Fetching concepts with fallback to NULL:\n")
188+
cat(" Fetching concepts with structured results:\n")
181189
for (id in concept_ids) {
182-
concept <- safe_get_concept(client, id)
183-
if (!is.null(concept)) {
184-
cat(sprintf(" [%d] %s\n", id, concept$concept_name))
190+
result <- safe_get_concept(client, id)
191+
if (isTRUE(result$ok)) {
192+
cat(sprintf(" [%d] %s\n", id, result$value$concept_name))
185193
} else {
186-
cat(sprintf(" [%d] (not found)\n", id))
194+
cat(sprintf(" [%d] (%s)\n", id, result$reason))
187195
}
188196
}
189197
cat("\n")

inst/examples/navigate_hierarchy.R

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,12 @@ for (type_name in names(by_type)) {
134134
items <- by_type[[type_name]]
135135
cat(sprintf("\n %s (%d):\n", type_name, length(items)))
136136
for (r in head(items, 3)) {
137+
# Prefer the nested ``concept_2`` object (full concept details) but
138+
# fall back to the flat ``concept_id_2`` field so deprecated or
139+
# invalid target concepts without a populated nested object still
140+
# surface an ID instead of rendering as unknown.
137141
target <- r$concept_2 %||% list()
138-
target_id <- target$concept_id %||% "?"
142+
target_id <- target$concept_id %||% r$concept_id_2 %||% "?"
139143
target_name <- target$concept_name %||% "Unknown"
140144
cat(sprintf(" -> [%s] %s\n", target_id, target_name))
141145
}

tests/testthat/test-fhir.R

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,3 +442,68 @@ test_that("fhir_resolve_codeable_concept standalone matches R6 method", {
442442

443443
expect_identical(r6_result, wrapper_result)
444444
})
445+
446+
# ==============================================================================
447+
# resolve_batch(as_tibble = TRUE) with unusual `error` shapes
448+
# ==============================================================================
449+
450+
test_that("resolve_batch(as_tibble = TRUE) handles zero-length error values", {
451+
# Regression for a bug where `as.character(err)[[1L]]` crashed with
452+
# "subscript out of bounds" when the API returned an empty character
453+
# vector (or any other length-0 value) in the `error` field.
454+
base_req <- httr2::request("https://api.omophub.com/v1")
455+
resource <- FhirResource$new(base_req)
456+
457+
local_mocked_bindings(
458+
perform_post = function(req, path, body = NULL, query = NULL) {
459+
list(
460+
results = list(
461+
list(error = character(0)), # empty character vector
462+
list(error = integer(0)), # empty integer vector
463+
list(error = list()) # empty list
464+
),
465+
summary = list(total = 3L, resolved = 0L, failed = 3L)
466+
)
467+
}
468+
)
469+
470+
codings <- list(
471+
list(system = "http://snomed.info/sct", code = "a"),
472+
list(system = "http://snomed.info/sct", code = "b"),
473+
list(system = "http://snomed.info/sct", code = "c")
474+
)
475+
476+
# Must not throw, must return one row per input
477+
tbl <- resource$resolve_batch(codings, as_tibble = TRUE)
478+
expect_s3_class(tbl, "tbl_df")
479+
expect_equal(nrow(tbl), 3L)
480+
expect_true(all(tbl$status == "failed"))
481+
})
482+
483+
test_that("resolve_batch(as_tibble = TRUE) handles unusual scalar error shapes", {
484+
# Coverage for the non-character fallback: numeric and logical errors
485+
# must be coerced via as.character() without throwing.
486+
base_req <- httr2::request("https://api.omophub.com/v1")
487+
resource <- FhirResource$new(base_req)
488+
489+
local_mocked_bindings(
490+
perform_post = function(req, path, body = NULL, query = NULL) {
491+
list(
492+
results = list(
493+
list(error = 42L),
494+
list(error = TRUE)
495+
),
496+
summary = list(total = 2L, resolved = 0L, failed = 2L)
497+
)
498+
}
499+
)
500+
501+
codings <- list(
502+
list(system = "http://snomed.info/sct", code = "a"),
503+
list(system = "http://snomed.info/sct", code = "b")
504+
)
505+
506+
tbl <- resource$resolve_batch(codings, as_tibble = TRUE)
507+
expect_equal(nrow(tbl), 2L)
508+
expect_equal(tbl$status_detail, c("42", "TRUE"))
509+
})

0 commit comments

Comments
 (0)