Description

This PR introduces a simple check function to look for invalid API tokens. Unlike check_arg_is_valid_token, which checks for length and validity with REDCapR's sanitize_token, this function looks at the output of REDCapR::redcap_metadata_read() for invalid status return values.

Prior to this, redcap_metadata_read failures would give an unhelpful error message related to a filter operation we perform immediately after calling the metadata. Now, check_valid_api_token will look at the $success and $status_code values of the metadata list for 403 failure codes.

Choices and Calls for Input

I decided not to directly capture the error message that redcap_metadata_read provides in $outcome_message since it's not as friendly as our other error messages, but if we want to pass the exact output message from REDCapR we can:

"The REDCapR metadata export operation was not successful. The error message was:\n{\"error\":\"You do not have permissions to use the API\"}"

Otherwise, if we want to make our own informative sub-message as I've made here, happy to incorporate feedback for better language.

Proposed Changes

List changes below in bullet format:

Introduce check_valid_api_token function
Supply associated tests

Sample Output

> read_redcap(redcap_uri = redcap_uri
+             , token = paste(sample(1:9, 32, replace = T), collapse = ""))
Error in `read_redcap()`:
✖ The REDCapR metadata export operation was not successful.
ℹ An API token was provided that either does not have the correct privileges or is
  incorrectly formed.
Run `rlang::last_error()` to see where the error occurred.

Issue Addressed

Closes #107

PR Checklist

Before submitting this PR, please check and verify below that the submission meets the below criteria:

[x] New/revised functions have associated tests
[NA] New/revised functions that update downstream outputs have associated static testing files (.RDS) updated under inst/testdata/create_test_data.R
[NA] New tests that make API calls use httptest::with_mock_api and any new mocks were added to tests/testthat/fixtures/create_httptest_mocks.R
[x] New/revised functions use appropriate naming conventions
[x] New/revised functions don't repeat code
[x] Code changes are less than 250 lines total
[x] Issues linked to the PR using GitHub's list of keywords
[x] The appropriate reviewer is assigned to the PR
[x] The appropriate developers are assigned to the PR
[x] Pre-release package version incremented using usethis::use_dev_version()

Code Review

This section to be used by the reviewer and developers during Code Review after PR submission

Code Review Checklist

[ ] I checked that new files follow naming conventions and are in the right place
[ ] I checked that documentation is complete, clear, and without typos
[ ] I added/edited comments to explain "why" not "how"
[ ] I checked that all new variable and function names follow naming conventions
[ ] I checked that new tests have been written for key business logic and/or bugs that this PR fixes
[ ] I checked that new tests address important edge cases

@skadauke implemented as we discussed in the one location. Was thinking about how/if we should add test coverage for this. Trialing "bad" APIs/URIs would require new mocking I imagine, and generally we're looking to capture things we don't know to test for.

Keep me posted, if this is ok I can start applying to the other API calls to start addressing #110

@rsh52 I've been doing some work implementing error messages but am currently getting stuck at the point of trying to re-create mocks. Let's all take a look in the New Year.

Since this PR evolved a bit, here is a point by point summary of changes.

Rewrite check_arg_is_valid_token() to remove check_arg_is_character() call and wrap sanitize_token() into a try_fetch() block to create a pretty cli_abort() message
- Add this check_arg_is_character() check explicitly in read_redcap()
- Revise relevant tests in test-checks.R
Wrap REDCapR::redcap_metadata_read() into a try_fetch() block that does the following:
- Captures the current call in call. This is needed to be able to report the call that caused an error if necessary.
- Captures the entire output of redcap_metadata_read() in out. This is needed to throw errors about failure modes that REDCapR doesn't throw errors for
- Throws an error if redcap_metadata_read() completed without erroring out but out$success == FALSE indicating the API call was unsuccessful. We want to throw an actual error here.
- Defines an error handler that constructs a message and throws an appropriate error using cli_abort()
Add tests for new error conditions to test-read_redcap.R
Add a mock for a call that leads to a valid REDCap API endpoint rejecting the token

Minor:

Adjust .lintr file
Style package
Update cli_message_examples.R with new errors

Here is a rendered reprex showing the current error messages for read_redcap():

devtools::load_all(path = "/local/path/to/REDCapTidieR")
#> ℹ Loading REDCapTidieR

options(rlang_backtrace_on_error_report = "none")

# read_redcap

classic_token <- Sys.getenv("REDCAPTIDIER_CLASSIC_API")
redcap_uri <- Sys.getenv("REDCAP_URI")

## args missing

# read_redcap()

# read_redcap(redcap_uri)

# read_redcap(token = classic_token)

## redcap_uri

read_redcap(123, classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `redcap_uri` which is not a valid value
#> ! Must be of type 'character', not 'double'

read_redcap(letters[1:3], classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied `a`, `b`, `c` for `redcap_uri` which is not a valid
#>   value
#> ! Must have length 1, but has length 3

read_redcap("https://www.google.com", classic_token)
#> Error in `read_redcap()`:
#> ✖ The REDCapR metadata export operation was not successful.
#> ! The URL returned the HTTP error code 405 (POST Method not allowed).
#> ℹ The most likely reason is that the provided URI does not point to a REDCap
#>   API endpoint.
#> ℹ URI: `https://www.google.com`

read_redcap("https://www.google.comm", classic_token)
#> Error in `read_redcap()`:
#> ✖ The REDCapR metadata export operation was not successful.
#> ! Could not resolve the hostname.
#> ℹ The most likely reason is that the provided URI is incorrect.
#> ℹ URI: `https://www.google.comm`

## token

read_redcap(redcap_uri, 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `token` which is not a valid value
#> ! Must be of type 'character', not 'double'

read_redcap(redcap_uri, letters[1:3])
#> Error in `read_redcap()`:
#> ✖ You've supplied `a`, `b`, `c` for `token` which is not a valid value
#> ! Must have length 1, but has length 3

read_redcap(redcap_uri, "")
#> Error in `read_redcap()`:
#> ✖ The token is an empty string, not a valid 32-character hexademical
#>   value.
#> ℹ API token: ``

read_redcap(redcap_uri, "CC0CE44238EF65C5DA26A55DD749AF7") # 31 hex characters
#> Error in `read_redcap()`:
#> ✖ The token is not a valid 32-character hexademical value.
#> ℹ API token: `CC0CE44238EF65C5DA26A55DD749AF7`

read_redcap(redcap_uri, "CC0CE44238EF65C5DA26A55DD749AF7A") # will be rejected by server
#> Error in `read_redcap()`:
#> ✖ The REDCapR metadata export operation was not successful.
#> ! You supplied an API token that either does not have the correct privileges or
#>   is incorrectly formed.
#> ℹ API token: `CC0CE44238EF65C5DA26A55DD749AF7A`

## raw_or_label

read_redcap(redcap_uri, classic_token, raw_or_label = "bad option")
#> Error in `read_redcap()`:
#> ✖ You've supplied `bad option` for `raw_or_label` which is not a valid
#>   value
#> ! Must be element of set {'label','raw'}, but is 'bad option'

## forms

read_redcap(redcap_uri, classic_token, forms = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `forms` which is not a valid value
#> ! Must be of type 'character' (or 'NULL'), not 'double'

## export_survey_fields

read_redcap(redcap_uri, classic_token, export_survey_fields = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `export_survey_fields` which is not a valid
#>   value
#> ! Must be of type 'logical', not 'double'

read_redcap(redcap_uri, classic_token, export_survey_fields = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied `TRUE`, `TRUE` for `export_survey_fields` which is not
#>   a valid value
#> ! Must have length 1, but has length 2

## suppress_redcapr_messages

read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `suppress_redcapr_messages` which is not a
#>   valid value
#> ! Must be of type 'logical', not 'double'

read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied `TRUE`, `TRUE` for `suppress_redcapr_messages` which
#>   is not a valid value
#> ! Must have length 1, but has length 2

# bind_tibbles

bind_tibbles(123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

supertbl <- tibble(redcap_data = list())
bind_tibbles(supertbl, environment = "abc")
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `<tibble[,1]>` for `supertbl` which is not a valid
#>   value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

bind_tibbles(supertbl, tbls = 123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `<tibble[,1]>` for `supertbl` which is not a valid
#>   value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

# extract_tibbles

extract_tibbles(letters[1:10])
#> Error in `extract_tibbles()`:
#> ✖ You've supplied `a`, `b`, `c`, …, `i`, `j` for `supertbl` which is not
#>   a valid value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

# extract_tibble

extract_tibble(123, "my_tibble")
#> Error in `extract_tibble()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

supertbl <- tibble(redcap_data = list()) %>%
  as_supertbl()
extract_tibble(supertbl, tbl = 123)
#> Error in `extract_tibble()`:
#> ✖ You've supplied `123` for `tbl` which is not a valid value
#> ! Must be of type 'character', not 'double'

extract_tibble(supertbl, tbl = letters[1:3])
#> Error in `extract_tibble()`:
#> ✖ You've supplied `a`, `b`, `c` for `tbl` which is not a valid value
#> ! Must have length 1, but has length 3

# make_labelled

make_labelled(123)
#> Error in `make_labelled()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcap_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

missing_col_supertbl <- tibble(redcap_data = list()) %>%
  as_supertbl()
make_labelled(missing_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied `<rdcp_spr[,1]>` for `supertbl` which is not a valid
#>   value
#> ! Must contain `supertbl$redcap_metadata`
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

missing_list_col_supertbl <- tibble(redcap_data = list(), redcap_metadata = 123) %>%
  as_supertbl()
make_labelled(missing_list_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied `<rdcp_spr[,2]>` for `supertbl` which is not a valid
#>   value
#> ! `supertbl$redcap_metadata` must be of type 'list'
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

Generally like all the changes, and the return error messages look succinct and informative.

I also think it would keep read_redcap() a little neater if we move the handler code to a named function at the bottom of read_redcap.R rather than keeping it as an anonymous function. Best solution is probably to put all the try_fetch logic in a helper function but that's a little more complicated.

I agree this should be considered, especially as we're about to apply this across every REDCapR API call and should minimize duplication of large code blocks to address #110. We can do the refactor after this PR.

True, what may end up being tricky is to find a way to pass down the call to the helper function. I think it's solvable but I haven't been able to figure it out.

From: Rich Hanna @.> Sent: Wednesday, January 4, 2023 10:02 AM To: CHOP-CGTInformatics/REDCapTidieR @.> Cc: Stephan Kadauke @.>; Mention @.> Subject: [External]Re: [CHOP-CGTInformatics/REDCapTidieR] Add check_valid_api_token function and tests (PR #116)

Generally like all the changes, and the return error messages look succinct and informative.

I also think it would keep read_redcap() a little neater if we move the handler code to a named function at the bottom of read_redcap.R rather than keeping it as an anonymous function. Best solution is probably to put all the try_fetch logic in a helper function but that's a little more complicated.

I agree this should be considered, especially as we're about to apply this across every REDCapR API call and should minimize duplication of large code blocks to address #110https://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FCHOP-CGTInformatics%2FREDCapTidieR%2Fissues%2F110&data=05%7C01%7Ckadaukes%40chop.edu%7C649805cb932b4502b3d208daee64aab8%7Ca611241607b041a59bb1d146b575c975%7C0%7C0%7C638084413386681925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=JWoYVHYLKVNvar%2Fq8J9Yyr5M4LOxfS8hyDFISb13BbI%3D&reserved=0. We can do the refactor after this PR.

— Reply to this email directly, view it on GitHubhttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FCHOP-CGTInformatics%2FREDCapTidieR%2Fpull%2F116%23issuecomment-1371039257&data=05%7C01%7Ckadaukes%40chop.edu%7C649805cb932b4502b3d208daee64aab8%7Ca611241607b041a59bb1d146b575c975%7C0%7C0%7C638084413386681925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4x2KrizCNHNw7j2aSZBrhXN%2FwB2UeFKWOA6%2Brr4f9pg%3D&reserved=0, or unsubscribehttps://nam10.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACTGHWVHSCBJJCFYRY7GODDWQWGHJANCNFSM6AAAAAATFXULRY&data=05%7C01%7Ckadaukes%40chop.edu%7C649805cb932b4502b3d208daee64aab8%7Ca611241607b041a59bb1d146b575c975%7C0%7C0%7C638084413386681925%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=n5F3FZAlc6IZTKrYLpQ6FKrBfXwMFtlAi6%2Bq0D40ABY%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>

This email originated from an EXTERNAL sender to CHOP. Proceed with caution when replying, opening attachments, or clicking links. Do not disclose your CHOP credentials, employee information, or protected health information to a potential hacker.

CHOP-CGTInformatics / REDCapTidieR

Add check_valid_api_token function and tests #116