Functions should fail gracefully with an informative message if the resource is not available

[stitam]

CRAN Team requested to review our compliance with this policy. My current understanding on their issue is that ping() error should not produce a query function error so stop() should be removed from all of these functions. They asked us to correct this before 2022-06-11.

[Aariq]

I think from the wording they only care about "failing gracefully" for tests, examples, and vignettes—otherwise automated tests and stuff don't work. It doesn't sound like they explicitly have a problem with a function erroring for a user when the API is down.

But honestly, I don't know anywhere where we aren't meeting this guideline. Vignette is pre-compiled, all the examples are wrapped in donttest{} (I think?) and the tests all get skipped when the API is down.

[stitam]

I think I found a clue. Sometimes we use donttest{} and sometimes we use dontrun{}. Apparently there is a difference: https://stackoverflow.com/questions/68935126/when-and-when-not-to-use-donttest-and-dontrun-in-r-package-development.

The cir_img() example was wrapped in donttest. When they ran the example, this line produced an error because the API was down: if (!ping_service("cir")) stop(webchem_message("service_down")). However, since this was wrapped in donttest, this should have exited without any error.

I think having dontrun everywhere is the way to go, what do you think?

[Aariq]

I think having dontrun everywhere is the way to go, what do you think?

Sounds like the right solution to me