cache icon indicating copy to clipboard operation
cache copied to clipboard

`fail-on-cache-miss` for restore action not failing the workflow

Open wagoodman opened this issue 2 years ago • 12 comments

My interpretation of the fail-on-cache-miss feature on the actions/cache and actions/cache/restore actions is that the workflow should stop and be considered a fail if there is no cache found with the given key.

I'm seeing evidence in workflows where using the actions/cache/restore action will not fail even if this option is set to true.

Snippet from the workflow file:

  Acceptance-Linux:
    name: "Acceptance tests (Linux)"
    needs: [Build-Snapshot-Artifacts]
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 #v4.1.0

      - name: Bootstrap environment
        uses: ./.github/actions/bootstrap

      - name: Download snapshot build
        uses: actions/cache/restore@704facf57e6136b1bc63b828d79edcd491f0ee84 #v3.3.2
        with:
          path: snapshot
          fail-on-cache-miss: true
          key: snapshot-build-${{ github.run_id }}

      - name: Run comparison tests (Linux)
        run: make compare-linux

Evidence that the workflow continues past the failed cache restore (failing on the subsequent step, which it should have never reached. Screenshot 2023-10-20 at 1 27 58 PM

wagoodman avatar Oct 20 '23 17:10 wagoodman

My interpretation of the fail-on-cache-miss feature on the actions/cache and actions/cache/restore actions is that the workflow should stop and be considered a fail if there is no cache found with the given key.

I'm seeing evidence in workflows where using the actions/cache/restore action will not fail even if this option is set to true.

Snippet from the workflow file:

  Acceptance-Linux:
    name: "Acceptance tests (Linux)"
    needs: [Build-Snapshot-Artifacts]
    runs-on: ubuntu-20.04
    steps:
      - uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 #v4.1.0

      - name: Bootstrap environment
        uses: ./.github/actions/bootstrap

      - name: Download snapshot build
        uses: actions/cache/restore@704facf57e6136b1bc63b828d79edcd491f0ee84 #v3.3.2
        with:
          path: snapshot
          fail-on-cache-miss: true
          key: snapshot-build-${{ github.run_id }}

      - name: Run comparison tests (Linux)
        run: make compare-linux

Evidence that the workflow continues past the failed cache restore (failing on the subsequent step, which it should have never reached. Screenshot 2023-10-20 at 1 27 58 PM

ghost avatar Oct 21 '23 23:10 ghost

erg, thanks @Rolexminus1 ? (you might have intended to write something different than echoing my description... or you're a bot, in which case... beep beep, boop )

For what it's worth, I think I found the problem that was causing the cache miss, which I'm also a little confused at:

      - name: Upload snapshot artifacts
        uses: actions/cache/save@704facf57e6136b1bc63b828d79edcd491f0ee84 #v3.3.2
        with:
          path: |
            snapshot
            .task
          key: snapshot-build-${{ github.run_id }}

followed by:

      - name: Download snapshot build
        id: snapshot-cache
        uses: actions/cache/restore@704facf57e6136b1bc63b828d79edcd491f0ee84 #v3.3.2
        with:
          path: snapshot
          fail-on-cache-miss: true
          key: snapshot-build-${{ github.run_id }}

Note the difference in the path provided (snapshot vs [snapshot, .task] logically).

My understanding is that I'd be able to store multiple paths and restore one of those paths, which does not appear to be working. However, in my case, this was a bug and I should have been restoring both paths anyway.

All that being said, it still doesn't address the original problem (fail-on-cache-miss doesn't fail the workflow) so I have the following workaround for now as a subsequent step:

      - name: (cache-miss) Snapshot build missing
        if: steps.snapshot-cache.outputs.cache-hit != 'true'
        run: echo "unable to download snapshots from previous job" && false

wagoodman avatar Oct 23 '23 20:10 wagoodman

I am seeing the same behaviour, using the same path in save and restore in my cache. Also on actions/cache/restore@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3.3.2

Doing some investigating:

The thrown error is already being catched here: https://github.com/actions/cache/blob/704facf57e6136b1bc63b828d79edcd491f0ee84/src/restoreImpl.ts#L84-L86

But it will only exit with an error if the calling function catches an error. (earlyExit seems to always be set to true). https://github.com/actions/cache/blob/704facf57e6136b1bc63b828d79edcd491f0ee84/src/restoreImpl.ts#L93-L100

The try catch of the callee function probably just needs to be removed for this to work. So core.setFailed((error as Error).message) should be moved to the caller error handler.

vchernin avatar Oct 28 '23 17:10 vchernin

This has worked in the past for us.

s0undt3ch avatar Oct 28 '23 17:10 s0undt3ch

Also run into the said issue that fail-on-cache-miss not failing, although initially I suspected that it was me that was doing things wrong...

I wonder, though, if cache and restore use pretty much the same (not exactly) logic on catching [if I could read Typescript correctly], why should cache work fine on fali-on-cache-miss? Or is it also failing, yet neglected for its (maybe?) rare usage?

if (earlyExit) { 
    process.exit(1); 
}

If I understood correctly, shouldn't exiting with return value of 1 already fail a step, then?

dovisutu avatar Nov 04 '23 14:11 dovisutu

@dovisutu I believe you are on the right track, as we were running into this issue at my work place as well.

This PR added the earlyExit condition:

https://github.com/actions/cache/pull/1217

however, it wasn't implemented beyond just adding a default of undefined (which will always evaluate to false for that if). I assume what should have been done instead is the default should be based off that setting, and not just set to undefined.

cc @chkimes @kgrzebien @cwille97 as you were active on that PR.


For now, it seems like this was only merged into the most recent release (v3.3.2) to address this issue:

https://github.com/actions/cache/issues/810

So you can downgrade/pin your action/cache to v3.3.1 and you should be able to get this functionality back.

NickLaMuro avatar Nov 20 '23 18:11 NickLaMuro

Can confirm, setting the version to v3.3.1 forced the workflow to fail correctly

eXigentCoder avatar Dec 18 '23 15:12 eXigentCoder

Same issue. Any ETA for a fix?

PaulRBerg avatar Jan 17 '24 11:01 PaulRBerg

This issue still exists in the latest v3 (https://github.com/actions/cache/commit/e12d46a63a90f2fae62d114769bbf2a179198b5c)

miluoshi avatar Jan 19 '24 14:01 miluoshi

I am seeing this issue with actions/cache/restore@v4. I have set fail-on-cache-miss: true and the step reports an error "Error: Failed to restore cache entry. Exiting as fail-on-cache-miss is set." but the step is marked complete and the rest of the workflow runs. I even tried adding a check for steps.<step_id>.outcome but it is 'success'.

Failure is failing to fail.

james-pickle avatar Feb 01 '24 20:02 james-pickle

I am seeing this issue with actions/cache/restore@v4. I have set fail-on-cache-miss: true and the step reports an error "Error: Failed to restore cache entry. Exiting as fail-on-cache-miss is set." but the step is marked complete and the rest of the workflow runs. I even tried adding a check for steps.<step_id>.outcome but it is 'success'.

Failure is failing to fail.

Try wagoodman's solution from his comment (last code block): https://github.com/actions/cache/issues/1265#issuecomment-1775989076

if: steps.snapshot-cache.outputs.cache-hit != 'true'

ruben-hoenle-fnt avatar Feb 06 '24 09:02 ruben-hoenle-fnt

Opened #1327 with a simple fix. Let's hope someone can get to it quickly so this is fixed.

cdce8p avatar Feb 15 '24 21:02 cdce8p