callum-savage / bomWater

An R package to download Australian water data
Other
1 stars 1 forks source link

`make_bom_request` doesn't catch all errors #16

Open callum-savage opened 1 year ago

callum-savage commented 1 year ago

In the example below, the error message is not being caught.

params <-
  list(
    request = "getTimeseriesValues",
    ts_id = "168441010",
    from = "1900-01-01",
    to = structure(19500, class = "Date"),
    returnfields = "Timestamp,Value,Quality Code,Interpolation Type"
  )

bomWater:::make_bom_request(params)
#> Request for water data failed. Check your request and make sure http://www.bom.gov.au/waterdata/ is online
#> Error message:
#> Internal Server Error (HTTP 500). Failed to request water data from BoM.
#> Error in httr::content(r, "text"): is.response(x) is not TRUE

bom_url <- "http://www.bom.gov.au/waterdata/services"
base_params <- list(
  "service" = "kisters",
  "type" = "QueryServices",
  "format" = "json"
)
r <- httr::RETRY("GET", bom_url, query = c(base_params, params), times = 5, quiet = TRUE)
jsonlite::fromJSON(httr::content(r, "text"))
#> $type
#> [1] "error"
#> 
#> $code
#> [1] "TooManyResults"
#> 
#> $message
#> [1] "Maximum number of timeseries values surpassed. Please narrow your request. Limit is: 250000"

Created on 2023-05-23 with reprex v2.0.2

callum-savage commented 1 year ago

Additionally, the request is being made 5 times when it's obviously going to get the same error every time. This makes the response much slower than it needs to be. "TooManyResults" should be returned after the first error

callum-savage commented 1 year ago

I think the best way to deal with this will be to upgrade to json2, which will require a full rewrite of make_bom_request

callum-savage commented 1 year ago

This article has some good pointers: https://httr2.r-lib.org/articles/wrapping-apis.html