Polishing logic

NEWS.md

@@ -1,6 +1,6 @@
 # httr2 (development version)
 
-* `req_cache()` now prunes cache _before_ checking if a given key exists, eliminating the occassional error about reading from an invalid RDS file.
+* `req_cache()` now prunes cache _before_ checking if a given key exists, eliminating the occassional error about reading from an invalid RDS file. It also no longer tests for existence then later reads the cache, avoiding potential race conditions.
 * `jwt_encode_hmac()` now calls correct underlying function `jose::jwt_encode_hmac()` and has correct default size parameter value' (@denskh, #508).
 * `req_perform_parallel()` now respects error handling in `req_error()`
 * New function `req_perform_promise()` allows creating a `promises::promise` for a request that runs in the background (#501, @gergness).

R/req-cache.R

@@ -91,7 +91,10 @@ cache_active <- function(req) {
   req_policy_exists(req, "cache_path")
 }
 
-cache_get <- function(req) {
+cache_get <- function(
+    req
+) {
+  # This check should be redudant but we keep it in for safety
   if (!cache_active(req)) {
     return(req)
   }
@@ -101,8 +104,15 @@ cache_get <- function(req) {
     return(NULL)
   }
 
-  touch(path)
-  tryCatch(readRDS(path), error = function(e) NULL)
+  tryCatch(
+    {
+      rds <- readRDS(path)
+      # Update file time if read successfully
+      Sys.setFileTime(path, Sys.time())
+      rds
+    },
+    error = function(e) NULL
+  )
 }
 
 cache_set <- function(req, resp) {
@@ -195,6 +205,7 @@ cache_pre_fetch <- function(req) {
   }
 }
 
+# Always returns response
 cache_post_fetch <- function(req, resp, path = NULL) {
   if (!cache_active(req)) {
     return(resp)
@@ -216,10 +227,8 @@ cache_post_fetch <- function(req, resp, path = NULL) {
 
     # Replace body with cached result
     resp$body <- cache_body(cached_resp, path)
-
     # Combine headers
     resp$headers <- cache_headers(cached_resp, resp)
-
     resp
   } else if (resp_is_cacheable(resp)) {
     signal("", "httr2_cache_save")