Skip to content

Commit a7f6535

Browse files
committed
feat: added new polling class with CockatielPolicy
1 parent 0bf29e4 commit a7f6535

File tree

5 files changed

+223
-1
lines changed

5 files changed

+223
-1
lines changed

packages/shield-controller/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@
4848
},
4949
"dependencies": {
5050
"@metamask/base-controller": "^8.4.2",
51-
"@metamask/utils": "^11.8.1"
51+
"@metamask/controller-utils": "^11.14.1",
52+
"@metamask/utils": "^11.8.1",
53+
"cockatiel": "^3.1.2"
5254
},
5355
"devDependencies": {
5456
"@babel/runtime": "^7.23.9",
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { PollingWithCockatielPolicy } from './polling-with-policy';
2+
import { delay } from '../tests/utils';
3+
import { HttpError } from '@metamask/controller-utils';
4+
5+
describe('PollingWithCockatielPolicy', () => {
6+
it('should return the success result', async () => {
7+
const policy = new PollingWithCockatielPolicy();
8+
const result = await policy.start('test', async () => {
9+
return 'test';
10+
});
11+
expect(result).toBe('test');
12+
});
13+
14+
it('should retry the request and complete successfully', async () => {
15+
const policy = new PollingWithCockatielPolicy();
16+
let invocationCount = 0;
17+
const mockRequestFn = jest
18+
.fn()
19+
.mockImplementation(async (_abortSignal: AbortSignal) => {
20+
invocationCount += 1;
21+
return new Promise((resolve, reject) => {
22+
setTimeout(() => {
23+
// eslint-disable-next-line jest/no-conditional-in-test
24+
if (invocationCount < 3) {
25+
reject(new HttpError(412));
26+
}
27+
resolve('test');
28+
}, 100);
29+
});
30+
});
31+
const result = await policy.start('test', mockRequestFn);
32+
expect(result).toBe('test');
33+
expect(mockRequestFn).toHaveBeenCalledTimes(3);
34+
});
35+
36+
it('should not retry when the error is not retryable', async () => {
37+
const policy = new PollingWithCockatielPolicy();
38+
const mockRequestFn = jest
39+
.fn()
40+
.mockImplementation(async (_abortSignal: AbortSignal) => {
41+
return new Promise((_resolve, reject) => {
42+
reject(new HttpError(500, 'Internal server error'));
43+
});
44+
});
45+
await expect(policy.start('test', mockRequestFn)).rejects.toThrow(
46+
'Internal server error',
47+
);
48+
expect(mockRequestFn).toHaveBeenCalledTimes(1);
49+
});
50+
51+
it('should throw an error when the retry exceeds the max retries', async () => {
52+
const policy = new PollingWithCockatielPolicy({
53+
maxRetries: 3,
54+
});
55+
56+
const requestFn = jest
57+
.fn()
58+
.mockImplementation(async (_abortSignal: AbortSignal) => {
59+
return new Promise((_resolve, reject) => {
60+
setTimeout(() => {
61+
reject(new HttpError(412, 'Results are not available yet'));
62+
}, 100);
63+
});
64+
});
65+
66+
const result = policy.start('test', requestFn);
67+
await expect(result).rejects.toThrow('Results are not available yet');
68+
expect(requestFn).toHaveBeenCalledTimes(4);
69+
});
70+
71+
it('should throw a `Request Cancelled` error when the request is aborted', async () => {
72+
const policy = new PollingWithCockatielPolicy({
73+
maxRetries: 3,
74+
});
75+
76+
const requestFn = jest
77+
.fn()
78+
.mockImplementation(async (abortSignal: AbortSignal) => {
79+
return new Promise((_resolve, reject) => {
80+
setTimeout(() => {
81+
// eslint-disable-next-line jest/no-conditional-in-test
82+
if (abortSignal.aborted) {
83+
reject(new Error('test error'));
84+
}
85+
reject(new Error('test error'));
86+
}, 100);
87+
});
88+
});
89+
90+
const result = policy.start('test', requestFn);
91+
await delay(10);
92+
policy.abortPendingRequest('test');
93+
await expect(result).rejects.toThrow('Request cancelled');
94+
});
95+
96+
it('should throw a `Request Cancelled` error when a new request is started with the same request id', async () => {
97+
const policy = new PollingWithCockatielPolicy();
98+
99+
const requestFn = jest
100+
.fn()
101+
.mockImplementation(async (abortSignal: AbortSignal) => {
102+
return new Promise((resolve, reject) => {
103+
setTimeout(() => {
104+
// eslint-disable-next-line jest/no-conditional-in-test
105+
if (abortSignal.aborted) {
106+
reject(new Error('test error'));
107+
}
108+
resolve('test');
109+
}, 100);
110+
});
111+
});
112+
113+
const result = policy.start('test', requestFn);
114+
await delay(10);
115+
const secondResult = policy.start('test', requestFn);
116+
await expect(result).rejects.toThrow('Request cancelled');
117+
expect(await secondResult).toBe('test');
118+
});
119+
120+
it('should resolve the result when two requests are started with the different request ids', async () => {
121+
const policy = new PollingWithCockatielPolicy();
122+
123+
const requestFn = (result: string) =>
124+
jest.fn().mockImplementation(async (abortSignal: AbortSignal) => {
125+
return new Promise((resolve, reject) => {
126+
// eslint-disable-next-line jest/no-conditional-in-test
127+
if (abortSignal.aborted) {
128+
reject(new Error('test error'));
129+
}
130+
setTimeout(() => {
131+
resolve(result);
132+
}, 100);
133+
});
134+
});
135+
136+
const result = policy.start('test', requestFn('test'));
137+
const secondResult = policy.start('test2', requestFn('test2'));
138+
expect(await result).toBe('test');
139+
expect(await secondResult).toBe('test2');
140+
});
141+
});
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import {
2+
createServicePolicy,
3+
HttpError,
4+
type CreateServicePolicyOptions,
5+
type ServicePolicy,
6+
} from '@metamask/controller-utils';
7+
import { handleWhen } from 'cockatiel';
8+
9+
export type RequestFn<ReturnType> = (
10+
signal: AbortSignal,
11+
) => Promise<ReturnType>;
12+
13+
export class PollingWithCockatielPolicy {
14+
readonly #policy: ServicePolicy;
15+
16+
readonly #requestEntry = new Map<string, AbortController>();
17+
18+
constructor(policyOptions: CreateServicePolicyOptions = {}) {
19+
const retryFilterPolicy = handleWhen(this.#shouldRetry);
20+
this.#policy = createServicePolicy({
21+
...policyOptions,
22+
retryFilterPolicy,
23+
});
24+
}
25+
26+
async start<ReturnType>(requestId: string, requestFn: RequestFn<ReturnType>) {
27+
this.abortPendingRequest(requestId);
28+
const abortController = this.addNewRequestEntry(requestId);
29+
30+
try {
31+
const result = await this.#policy.execute(
32+
async ({ signal: abortSignal }) => {
33+
return requestFn(abortSignal);
34+
},
35+
abortController.signal,
36+
);
37+
return result;
38+
} catch (error) {
39+
if (abortController.signal.aborted) {
40+
throw new Error('Request cancelled');
41+
}
42+
throw error;
43+
}
44+
}
45+
46+
addNewRequestEntry(requestId: string) {
47+
const abortController = new AbortController();
48+
this.#requestEntry.set(requestId, abortController);
49+
return abortController;
50+
}
51+
52+
abortPendingRequest(requestId: string) {
53+
const abortController = this.#requestEntry.get(requestId);
54+
abortController?.abort();
55+
}
56+
57+
#shouldRetry(error: Error): boolean {
58+
if (error instanceof HttpError) {
59+
// Note: we don't retry on 4xx errors, only on 5xx errors.
60+
// but we won't retry on 400 coz it means that the request body is invalid.
61+
return error.httpStatus > 400 && error.httpStatus < 500;
62+
}
63+
return false;
64+
}
65+
}

packages/shield-controller/tests/utils.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,15 @@ export function setupCoverageResultReceived(
106106
baseMessenger.subscribe('ShieldController:coverageResultReceived', handler);
107107
});
108108
}
109+
110+
/**
111+
* Delay for a specified amount of time.
112+
*
113+
* @param ms - The number of milliseconds to delay.
114+
* @returns A promise that resolves after the specified amount of time.
115+
*/
116+
export function delay(ms: number): Promise<void> {
117+
return new Promise((resolve) => {
118+
setTimeout(resolve, ms);
119+
});
120+
}

yarn.lock

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4727,11 +4727,13 @@ __metadata:
47274727
"@lavamoat/preinstall-always-fail": "npm:^2.1.0"
47284728
"@metamask/auto-changelog": "npm:^3.4.4"
47294729
"@metamask/base-controller": "npm:^8.4.2"
4730+
"@metamask/controller-utils": "npm:^11.14.1"
47304731
"@metamask/signature-controller": "npm:^34.0.2"
47314732
"@metamask/transaction-controller": "npm:^60.10.0"
47324733
"@metamask/utils": "npm:^11.8.1"
47334734
"@ts-bridge/cli": "npm:^0.6.1"
47344735
"@types/jest": "npm:^27.4.1"
4736+
cockatiel: "npm:^3.1.2"
47354737
deepmerge: "npm:^4.2.2"
47364738
jest: "npm:^27.5.1"
47374739
lodash: "npm:^4.17.21"

0 commit comments

Comments
 (0)