Skip to content

Don't cache NULL error returns from Jolpica#302

Draft
Copilot wants to merge 2 commits into
masterfrom
copilot/avoid-caching-error-returns
Draft

Don't cache NULL error returns from Jolpica#302
Copilot wants to merge 2 commits into
masterfrom
copilot/avoid-caching-error-returns

Conversation

Copy link
Copy Markdown

Copilot AI commented Apr 24, 2026

When Jolpica is unreachable, load_* functions return NULL. memoise treats NULL as a valid return value and caches it — so retrying the same call returns the cached failure instead of hitting the network again.

Approach

memoise stores results via withVisible(), so a NULL return is stored as list(value = NULL, visible = ...). A thin cache wrapper intercepts $set() and silently drops writes where value$value is NULL, causing memoise to see a cache miss on the next call and re-execute the function.

null_filtering_cache <- function(cache) {
  list(
    set = function(key, value) {
      if (is.list(value) && is.null(value$value)) return(invisible(NULL))
      invisible(cache$set(key, value))
    },
    # all other methods delegate unchanged
    ...
  )
}

Changes

  • R/zzz.R — Added null_filtering_cache() wrapper; applied to both cache_disk and cache_mem in .onLoad. All 14 memoised functions benefit automatically. The functions remain proper memoised-class objects, so has_cache(), forget(), and clear_f1_cache() are unaffected.
  • tests/testthat/test-utils.R — Unit tests for null_filtering_cache() covering NULL suppression and delegation of other cache methods.
  • tests/testthat/test-load_laps.R — Asserts has_cache(load_laps)(2021, 1) returns FALSE after a failed (no-network) call.
  • .github/workflows/copilot-setup-steps.yml — R + Python environment setup for the Copilot agent.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/r-lib/memoise/contents/R
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix caching of NULL when Jolpica is down Don't cache NULL error returns from Jolpica Apr 24, 2026
Copilot AI requested a review from pbulsink April 24, 2026 23:39
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.

Cache stores Jolpica error and prevents retry.

2 participants