Skip to content

Improve cache service availability determination and change failure log levels #2100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link

@Copilot Copilot AI commented Jul 14, 2025

This PR improves how the cache service availability is determined in @actions/toolkit and changes the log level of cache failures from warnings to errors.

Changes Made

1. Enhanced isFeatureAvailable() function

The isFeatureAvailable() function now properly leverages the ACTIONS_CACHE_SERVICE_V2 feature flag to determine cache service availability:

  • For cache service v2: Requires ACTIONS_RESULTS_URL to be set
  • For cache service v1: Requires either ACTIONS_CACHE_URL or ACTIONS_RESULTS_URL to be set
  • For GHES: Always uses v1 logic regardless of the v2 flag setting

Before:

export function isFeatureAvailable(): boolean {
  return !!process.env['ACTIONS_CACHE_URL']
}

After:

export function isFeatureAvailable(): boolean {
  const cacheServiceVersion = getCacheServiceVersion()
  
  switch (cacheServiceVersion) {
    case 'v2':
      return !!process.env['ACTIONS_RESULTS_URL']
    case 'v1':
    default:
      return !!(
        process.env['ACTIONS_CACHE_URL'] || process.env['ACTIONS_RESULTS_URL']
      )
  }
}

2. Changed log levels from warnings to errors

Updated all cache failure logs from core.warning() to core.error() in:

  • restoreCacheV1() function
  • restoreCacheV2() function
  • saveCacheV1() function
  • saveCacheV2() function

3. Comprehensive test coverage

Added extensive test coverage including:

  • V1 service with different URL combinations
  • V2 service with correct and incorrect URL configurations
  • GHES behavior with v2 flag enabled
  • Edge cases with no URLs set
  • Updated existing tests to expect errors instead of warnings

Exit Criteria Met

✅ Even if ACTIONS_CACHE_URL is not set, isFeatureAvailable() now uses alternative ways to determine cache service availability
✅ Changed log level of cache failures from warning to error

Backward Compatibility

This change maintains full backward compatibility:

  • Existing workflows using ACTIONS_CACHE_URL continue to work unchanged
  • V1 cache service behavior is preserved
  • GHES installations are unaffected

Fixes #2099.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits July 14, 2025 10:32
…to errors

- Update isFeatureAvailable() to leverage ACTIONS_CACHE_SERVICE_V2 feature flag
- For v2: check ACTIONS_RESULTS_URL availability
- For v1: check either ACTIONS_CACHE_URL or ACTIONS_RESULTS_URL availability
- Change warning logs to error logs for cache failures
- Add comprehensive tests covering all scenarios

Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
- Update restoreCacheV2.test.ts, restoreCache.test.ts, saveCacheV2.test.ts, and saveCache.test.ts
- Change test expectations from core.warning to core.error for cache operation failures
- All tests now pass successfully

Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Improve how the cache service availability is determined in actions/toolkit Improve cache service availability determination and change failure log levels Jul 14, 2025
@Copilot Copilot AI requested a review from Link- July 14, 2025 10:40
Copilot finished work on behalf of Link- July 14, 2025 10:40
Copy link
Member

@Link- Link- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the implementation as per the comments provided

Copilot AI and others added 2 commits July 14, 2025 12:23
Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
@Copilot Copilot AI requested a review from Link- July 14, 2025 12:27
Copilot finished work on behalf of Link- July 14, 2025 12:27
…n as requested

Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jul 14, 2025

Fix the implementation as per the comments provided

Implemented as requested in commit d48d6b6. Removed the logCacheError function and added inline 5xx server error detection in all four cache functions.

@Copilot Copilot AI requested a review from Link- July 14, 2025 12:43
Copilot finished work on behalf of Link- July 14, 2025 12:43
Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
Copilot finished work on behalf of Link- July 14, 2025 13:01
Copilot finished work on behalf of Link- July 14, 2025 13:18
Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
@Copilot Copilot AI requested a review from Link- July 14, 2025 13:39
Copilot finished work on behalf of Link- July 14, 2025 13:39
Co-authored-by: Link- <568794+Link-@users.noreply.github.com>
@Copilot Copilot AI requested a review from Link- July 14, 2025 13:50
Copilot finished work on behalf of Link- July 14, 2025 13:50
@Link- Link- marked this pull request as ready for review July 15, 2025 09:40
@Link- Link- requested a review from a team as a code owner July 15, 2025 09:40
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.

Improve how the cache service availability is determined in actions/toolkit
4 participants