Skip to content

Improve cache service availability determination and implement conditional error logging #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

Merged
merged 11 commits into from
Jul 28, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 14, 2025

This PR improves how the cache service availability is determined in @actions/toolkit and implements conditional error logging for cache failures based on HTTP status codes.

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 ACTIONS_CACHE_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']
  }
}

2. Conditional error logging for cache failures

Updated cache failure logging to differentiate between server errors and other failures:

  • Server errors (5xx status codes): Logged as errors using core.error()
  • All other cache failures: Logged as warnings using core.warning()

Applied to all cache functions:

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

Implementation:

// Log server errors (5xx) as errors, all other errors as warnings
if (
  typedError instanceof HttpClientError &&
  typeof typedError.statusCode === 'number' &&
  typedError.statusCode >= 500
) {
  core.error(`Failed to restore: ${(error as Error).message}`)
} else {
  core.warning(`Failed to restore: ${(error as Error).message}`)
}

3. Comprehensive test coverage

Added extensive test coverage including:

  • V1 service with ACTIONS_CACHE_URL requirements
  • V2 service with ACTIONS_RESULTS_URL requirements
  • GHES behavior with v2 flag enabled
  • Server error (5xx) vs other error logging differentiation
  • Edge cases with no URLs set
  • Updated existing tests to expect appropriate log levels

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
  • Only server errors (5xx) are elevated to error level, preserving existing warning behavior for most cache failures

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
Contributor 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
@Link- Link- added the bug Something isn't working label Jul 22, 2025
@Link-
Copy link
Member

Link- commented Jul 28, 2025

Environment Screenshot
github.com cs_2025-07-28_16-05-00
GHES 3.16 cs_2025-07-28_16-20-29

@Link-
Copy link
Member

Link- commented Jul 28, 2025

@copilot Update the PR description to match the changes introduced

Copy link
Contributor Author

Copilot AI commented Jul 28, 2025

@copilot Update the PR description to match the changes introduced

Updated the PR description to accurately reflect the actual implementation. The description now correctly describes:

  1. The exact logic for isFeatureAvailable() - v1 only checks ACTIONS_CACHE_URL, v2 checks ACTIONS_RESULTS_URL
  2. Conditional error logging - only server errors (5xx) are logged as errors, other failures remain warnings
  3. The comprehensive test coverage for all scenarios

The updated description matches the current implementation rather than the original planned changes.

@Copilot Copilot AI changed the title Improve cache service availability determination and change failure log levels Improve cache service availability determination and implement conditional error logging Jul 28, 2025
Copilot finished work on behalf of Link- July 28, 2025 14:31
@Link- Link- requested a review from yacaovsnc July 28, 2025 14:35
@yacaovsnc yacaovsnc requested a review from a team July 28, 2025 14:43
@Link- Link- merged commit 6d3feab into main Jul 28, 2025
12 of 18 checks passed
@Link- Link- deleted the copilot/fix-2099 branch July 28, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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