actions / actions/toolkit

cache.saveCache should raise an error on reserve failure, but doesn't

Open
#1,642 1 comment 12 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
TypeScript
Stars
5.9k
Forks
1.8k
PR merge metrics
No merged PRs in 30d

Description

Describe the bug

https://github.com/actions/toolkit/blob/main/packages/cache/src/cache.ts#L157-L165

The function signature for cache.saveCache mentions:

@returns number returns cacheId if the cache was saved successfully and throws an error if save fails

When there is a failure to reserve the cache key, this exception is caught, and a job warning annotation is set, but the exception is not propagated. Therefore the error is not actually returned as per the defined interface of the function. This means that reservation failures wrapped in a try/catch block will not execute the exception handler in the caller function.

I believe both of these code paths should re-raise the exception. Otherwise it is being swallowed and the only way to know there was a problem, is to check that the cacheId is still -1:

https://github.com/actions/toolkit/blob/main/packages/cache/src/cache.ts#L245-L248

As it stands, we get a log message like this:

Failed to save: Unable to reserve cache with key XXXX, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/master, Key: XXXX, Version: 50059a988699d11b5543e65bd21ddbb819f05df288843af627d295d329cbda93

But no error is thrown, and the build continues despite the failure.

To Reproduce
Steps to reproduce the behavior:

  1. Use actions/cache in a github actions workflow, using the cache.saveCache function to save a value at a static key.
  2. Create a race condition using your own creativity, ensuring that multiple actions runs are saving the cache concurrently with the same key.
  3. Await one of the actions to fail to save the cache.
  4. See error

Expected behavior

As per the function documentation, the exception should be thrown rather than logged and suppressed.

Screenshots
N/A

Desktop (please complete the following information):

  • OS: N/A
  • Browser N/A
  • Version N/A

This is not a client side error. It occurs in the latest released version of actions/cache.

Additional context

N/A

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/cache/src/cache.ts, especially the saveCache handling around lines 157-165 and the cacheId checks around lines 245-248. Trace the reserve-failure path and compare it with the documented behavior that saveCache throws when saving fails; done means the exception reaches callers instead of being only logged or represented by a -1 cacheId.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, typescript
Domain
devtools
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.