actions / cache

Cache dependencies and build outputs in GitHub Actions
MIT License
4.37k stars 1.16k forks source link

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

Closed wagoodman closed 3 months ago

wagoodman commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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
vchernin commented 8 months ago

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.

s0undt3ch commented 8 months ago

This has worked in the past for us.

dovisutu commented 8 months ago

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?

NickLaMuro commented 7 months ago

@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.

eXigentCoder commented 6 months ago

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

PaulRBerg commented 5 months ago

Same issue. Any ETA for a fix?

miluoshi commented 5 months ago

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

james-pickle commented 5 months ago

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..outcome but it is 'success'.

Failure is failing to fail.

ruben-hoenle-fnt commented 5 months ago

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..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'
cdce8p commented 4 months ago

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