Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve error handling of non-existent/non-accessible filepaths #232

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ahl27
Copy link

@ahl27 ahl27 commented Feb 14, 2025

Found a small bug, figured it would be faster to include a fix for it with the report.

Running rcmdcheck on a filepath that doesn't exist produces an uninformative error:

> rcmdcheck::rcmdcheck("this path is not a real path!")
## Error in if (file.info(path)$isdir) { : 
##  missing value where TRUE/FALSE needed

This is because file.info(path) returns all NA values if path is either non-existent or not readable. I encountered this when I accidentally had a small typo in the path to the package I wanted to test.

This fix wraps the file.info(path)$isdir call with some other error handling to produce more informative errors (and potentially allow for other handling / custom error messages later). It also handles the case where the file/directory exists, but isn't readable. Also included a unit test.

New version:

> rcmdcheck::rcmdcheck("this path is not a real path!")
## Error in safecheck_isdir(path) : path 'this is a test' does not exist!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ahl27
Copy link
Author

ahl27 commented Feb 14, 2025

tests seem to be failing due to a CRAN error, not because of changes in this PR.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ahl27
Copy link
Author

ahl27 commented Feb 17, 2025

added extra_cols = FALSE argument to file.info, since we only need res$isdir and this can slightly improve performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant