Merge pull request #34308 from backstage/freben/cached-service-cleanup

Fix cache eviction race in CachedUserInfoService

.changeset/cached-service-cleanup.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-defaults': patch
+---
+
+Fixed a race condition in `CachedUserInfoService` where a failed request could incorrectly evict a newer cache entry for the same token. The error handler now verifies the map entry is still the same promise before deleting it.

packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.test.ts

@@ -19,6 +19,7 @@ import {
   UserInfoService,
 } from '@backstage/backend-plugin-api';
 import { mockCredentials } from '@backstage/backend-test-utils';
+import { createDeferred } from '@backstage/types';
 import {
   CachedUserInfoService,
   UserInfoCacheEntry,
@@ -129,10 +130,8 @@ describe('CachedUserInfoService', () => {
   });
 
   it('evicts eagerly so concurrent waiters see the rejection and the next call retries', async () => {
-    let rejectFirst: (error: Error) => void;
-    const firstCall = new Promise<BackstageUserInfo>((_resolve, reject) => {
-      rejectFirst = reject;
-    });
+    const firstCall = createDeferred<BackstageUserInfo>();
+    firstCall.catch(() => {});
 
     const delegate: UserInfoService = {
       getUserInfo: jest
@@ -146,7 +145,7 @@ describe('CachedUserInfoService', () => {
     const p1 = service.getUserInfo(creds);
     const p2 = service.getUserInfo(creds);
 
-    rejectFirst!(new Error('boom'));
+    firstCall.reject(new Error('boom'));
 
     await expect(p1).rejects.toThrow('boom');
     await expect(p2).rejects.toThrow('boom');

packages/backend-defaults/src/entrypoints/userInfo/CachedUserInfoService.ts

@@ -73,7 +73,9 @@ export class CachedUserInfoService implements UserInfoService {
     }
 
     const promise = this.#delegate.getUserInfo(credentials).catch(error => {
-      this.#entries.delete(token);
+      if (this.#entries.get(token)?.promise === promise) {
+        this.#entries.delete(token);
+      }
       throw error;
     });