Skip to content

Commit 50a0f69

Browse files
committed
chore: changelog and test updates from cursor review
1 parent 92c25a4 commit 50a0f69

File tree

4 files changed

+59
-4
lines changed

4 files changed

+59
-4
lines changed

packages/ramps-controller/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10-
### Changed
10+
### Added
1111

12-
- Bump `@metamask/controller-utils` from `^11.16.0` to `^11.17.0` ([#7534](https://github.com/MetaMask/core/pull/7534))
12+
- Add request caching infrastructure with TTL, deduplication, and abort support ([#7536](https://github.com/MetaMask/core/pull/7536))
1313

1414
## [2.0.0]
1515

packages/ramps-controller/src/RampsController.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,57 @@ describe('RampsController', () => {
324324
await expect(requestPromise).rejects.toThrow('Request was aborted');
325325
});
326326
});
327+
328+
it('does not delete newer pending request when aborted request settles', async () => {
329+
await withController(async ({ controller }) => {
330+
let requestASettled = false;
331+
let requestBCallCount = 0;
332+
333+
// Request A: will be aborted but takes time to settle
334+
const fetcherA = async (signal: AbortSignal): Promise<string> => {
335+
return new Promise<string>((resolve, reject) => {
336+
signal.addEventListener('abort', () => {
337+
// Simulate async cleanup delay before rejecting
338+
setTimeout(() => {
339+
requestASettled = true;
340+
reject(new Error('Request A aborted'));
341+
}, 50);
342+
});
343+
});
344+
};
345+
346+
// Request B: normal request that should deduplicate correctly
347+
const fetcherB = async (): Promise<string> => {
348+
requestBCallCount += 1;
349+
await new Promise((resolve) => setTimeout(resolve, 10));
350+
return 'result-b';
351+
};
352+
353+
// Start request A
354+
const promiseA = controller.executeRequest('race-key', fetcherA);
355+
356+
// Abort request A (removes from pendingRequests, triggers abort)
357+
controller.abortRequest('race-key');
358+
359+
// Start request B with the same key before request A settles
360+
expect(requestASettled).toBe(false);
361+
const promiseB = controller.executeRequest('race-key', fetcherB);
362+
363+
// Start request C with same key - should deduplicate with B
364+
const promiseC = controller.executeRequest('race-key', fetcherB);
365+
366+
// Wait for request A to finish settling (its finally block runs)
367+
await expect(promiseA).rejects.toThrow('Request A aborted');
368+
expect(requestASettled).toBe(true);
369+
370+
// Requests B and C should still work correctly (deduplication intact)
371+
const [resultB, resultC] = await Promise.all([promiseB, promiseC]);
372+
373+
expect(resultB).toBe('result-b');
374+
expect(resultC).toBe('result-b');
375+
expect(requestBCallCount).toBe(1);
376+
});
377+
});
327378
});
328379

329380
describe('cache eviction', () => {

packages/ramps-controller/src/RampsController.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,11 @@ export class RampsController extends BaseController<
272272
);
273273
throw error;
274274
} finally {
275-
this.#pendingRequests.delete(cacheKey);
275+
// Only delete if this is still our entry (not replaced by a new request)
276+
const currentPending = this.#pendingRequests.get(cacheKey);
277+
if (currentPending?.abortController === abortController) {
278+
this.#pendingRequests.delete(cacheKey);
279+
}
276280
}
277281
})();
278282

packages/ramps-controller/src/RequestCache.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe('RequestCache', () => {
1212
describe('createCacheKey', () => {
1313
it('creates a cache key from method and empty params', () => {
1414
const key = createCacheKey('updateGeolocation', []);
15-
expect(key).toBe('getGeolocation:[]');
15+
expect(key).toBe('updateGeolocation:[]');
1616
});
1717

1818
it('creates a cache key from method and params', () => {

0 commit comments

Comments
 (0)