Skip to content

Fix: Cache key includes query parameters to prevent stale data after 422 errors #250

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 7 commits into from
Aug 12, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 12, 2025

Problem

When users change date ranges in the Copilot Metrics Viewer, the application would stop working correctly after encountering a 422 error (API limit reached). Specifically:

  1. User requests data for an invalid date range → API returns 422 error
  2. User switches back to a valid date range → Application continues to show stale data or errors

Root Cause

The caching mechanism in shared/utils/metrics-util.ts was using only event.path (e.g., /api/metrics) as the cache key, causing all requests to the same endpoint to share the same cache entry regardless of query parameters like date ranges.

// Before: All requests to /api/metrics shared the same cache
if (cache.has(event.path)) {
  // Returns cached data even for different date ranges
}

This meant that:

  • Different date ranges (since=2024-01-01 vs since=2024-02-01) used the same cache entry
  • When a 422 error occurred, the cache wasn't cleared, leading to stale data on subsequent requests

Solution

  1. Updated cache key to include query parameters:

    const queryString = new URLSearchParams(query as Record<string, string>).toString();
    const cacheKey = queryString ? `${event.path}?${queryString}` : event.path;
  2. Added cache clearing on errors:

    catch (error: unknown) {
      // Clear any cached data for this request to prevent stale data on retry
      cache.delete(cacheKey);
      throw new MetricsError(...);
    }
  3. Clear API errors on new requests in the client:

    async fetchMetrics() {
      this.isLoading = true;
      // Clear previous API errors when making a new request
      this.apiError = undefined;
    }

Verification

  • ✅ All existing tests pass (88/88)
  • ✅ Added focused tests for cache key generation logic
  • ✅ Manual testing confirms different date ranges create separate cache entries
  • ✅ Date range changes update metrics correctly without stale data issues
  • ✅ Application handles 422 errors gracefully and recovers on valid requests

The fix ensures that each unique combination of query parameters gets its own cache entry, preventing cross-contamination between different date ranges and eliminating stale data issues after API errors.

Fixes #249.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.fontshare.com
    • Triggering command: node (vitest) (dns block)
    • Triggering command: node /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/.bin/nuxt build (dns block)
  • api.fontsource.org
    • Triggering command: node (vitest) (dns block)
    • Triggering command: node /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/.bin/nuxt build (dns block)
    • Triggering command: /usr/local/bin/node --enable-source-maps /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/@nuxt/cli/bin/nuxi.mjs _dev (dns block)
  • fonts.bunny.net
    • Triggering command: node (vitest) (dns block)
    • Triggering command: node /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/.bin/nuxt build (dns block)
    • Triggering command: /usr/local/bin/node --enable-source-maps /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/@nuxt/cli/bin/nuxi.mjs _dev (dns block)
  • fonts.google.com
    • Triggering command: node (vitest) (dns block)
    • Triggering command: node /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/.bin/nuxt build (dns block)
    • Triggering command: /usr/local/bin/node --enable-source-maps /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/@nuxt/cli/bin/nuxi.mjs _dev (dns block)
  • fonts.googleapis.com
    • Triggering command: node (vitest 1) (dns block)
    • Triggering command: node (vitest 2) (dns block)
    • Triggering command: node (vitest 3) (dns block)
  • https://api.github.com/orgs/mocked-org/copilot/metrics
    • Triggering command: /usr/local/bin/node --enable-source-maps /home/REDACTED/work/copilot-metrics-viewer/copilot-metrics-viewer/node_modules/@nuxt/cli/bin/nuxi.mjs _dev (http block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@Copilot Copilot AI changed the title [WIP] bug: paging stopps working once limit is reached Fix: Cache key includes query parameters to prevent stale data after 422 errors Aug 12, 2025
@Copilot Copilot AI requested a review from karpikpl August 12, 2025 13:29
Copilot finished work on behalf of karpikpl August 12, 2025 13:29
@karpikpl karpikpl requested a review from Copilot August 12, 2025 19:13
@karpikpl karpikpl marked this pull request as ready for review August 12, 2025 19:13
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a critical caching bug where changing date ranges would show stale data after encountering 422 API errors. The root cause was that cache keys only used the endpoint path, causing all requests to the same endpoint to share cache entries regardless of query parameters.

Key changes:

  • Updated cache key generation to include query parameters and Authorization header fingerprints
  • Added cache clearing on errors to prevent stale data retention
  • Removed caching from teams API to simplify the fix
  • Enhanced error messaging for better user guidance

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
shared/utils/metrics-util.ts Core fix: implements auth-bound cache keys with query parameters and error-triggered cache clearing
tests/metrics-cache.nuxt.spec.ts New tests for basic cache key generation logic with different query parameters
tests/metrics-auth-cache.nuxt.spec.ts New tests for auth-bound cache key functionality and edge cases
server/api/teams.ts Removes caching entirely from teams API to simplify the overall caching strategy
app/components/MainComponent.vue Clears API errors on new requests and improves 422 error messaging

@karpikpl karpikpl merged commit f5e2af9 into main Aug 12, 2025
6 checks passed
@karpikpl karpikpl deleted the copilot/fix-249 branch August 12, 2025 19:36
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.

bug: paging stopps working once limit is reached
2 participants