Skip to content

Commit 538375c

Browse files
committed
Adds abort signal to requests
Enables request abortion using AbortSignal in all transports. The change ensures requests can be cancelled, prevents memory leaks and improves user experience by rejecting requests that are no longer relevant. Implements AbortError class to differentiate RPC errors from aborts. Ran `npm i baseline-browser-mapping@latest -D` to update browser mappings, and `npm audit fix` to fix 3 vulnerabilities (1 low, 1 moderate, 1 high).
1 parent 747c639 commit 538375c

15 files changed

+2608
-8548
lines changed

package-lock.json

Lines changed: 2393 additions & 8532 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"@types/websocket": "1.0.10",
4444
"@types/ws": "8.18.1",
4545
"@typescript-eslint/eslint-plugin": "8.44.0",
46+
"baseline-browser-mapping": "^2.9.5",
4647
"eslint-config-prettier": "10.0.1",
4748
"eslint-plugin-prettier": "5.2.3",
4849
"jest": "29.7.0",
@@ -58,4 +59,4 @@
5859
"strict-event-emitter-types": "2.0.0",
5960
"ws": "8.18.3"
6061
}
61-
}
62+
}

src/ClientInterface.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { IJSONRPCNotification } from "./Request.js";
33
interface Arguments {
44
readonly method: string;
55
readonly params?: readonly unknown[] | object;
6+
readonly timeout?: number;
7+
readonly signal?: AbortSignal;
68
}
79

810
export type RequestArguments = Arguments;

src/Error.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@ export const ERR_TIMEOUT = 7777;
22
export const ERR_MISSIING_ID = 7878;
33
export const ERR_UNKNOWN = 7979;
44

5+
export class AbortError extends Error {
6+
constructor(message: string = "Request aborted") {
7+
super(message);
8+
this.name = "AbortError";
9+
}
10+
}
11+
512
export class JSONRPCError extends Error {
613
public message: string;
714
public code: number;

src/RequestManager.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,11 @@ class RequestManager {
8989
});
9090
return result;
9191
}
92-
return this.getPrimaryTransport().sendData(payload, timeout);
92+
return this.getPrimaryTransport().sendData(
93+
payload,
94+
requestObject.timeout || timeout,
95+
requestObject.signal,
96+
);
9397
}
9498

9599
public close(): void {

src/index.test.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,28 @@
1-
import { Client } from "./index.js";
2-
import RequestManager from "./RequestManager.js";
3-
import EventEmitterTransport from "./transports/EventEmitterTransport.js";
1+
import Client, {
2+
RequestManager,
3+
EventEmitterTransport,
4+
HTTPTransport,
5+
WebSocketTransport,
6+
PostMessageWindowTransport,
7+
PostMessageIframeTransport,
8+
JSONRPCError,
9+
} from "./index.js";
410
import { EventEmitter } from "events";
511
import { addMockServerTransport } from "./__mocks__/eventEmitter.js";
612
import { generateMockNotificationRequest } from "./__mocks__/requestData.js";
713

814
describe("client-js", () => {
15+
it("exposes the package exports", () => {
16+
expect(Client).toBeDefined();
17+
expect(RequestManager).toBeDefined();
18+
expect(EventEmitterTransport).toBeDefined();
19+
expect(HTTPTransport).toBeDefined();
20+
expect(WebSocketTransport).toBeDefined();
21+
expect(PostMessageWindowTransport).toBeDefined();
22+
expect(PostMessageIframeTransport).toBeDefined();
23+
expect(JSONRPCError).toBeDefined();
24+
});
25+
926
it("can be constructed", () => {
1027
const emitter = new EventEmitter();
1128
const c = new Client(

src/transports/EventEmitterTransport.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ class EventEmitterTransport extends Transport {
2727
public sendData(
2828
data: JSONRPCRequestData,
2929
timeout: number | null = null,
30+
signal?: AbortSignal | null,
3031
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3132
): Promise<any> {
32-
const prom = this.transportRequestManager.addRequest(data, timeout);
33+
const prom = this.transportRequestManager.addRequest(data, timeout, signal);
3334
const notifications = getNotifications(data);
3435
const parsedData = this.parseData(data);
3536
try {

src/transports/HTTPTransport.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { HTTPTransport, HTTPTransportOptions } from "./HTTPTransport.js";
22
import * as reqMocks from "../__mocks__/requestData.js";
3+
import { AbortError } from "../Error.js";
34

45
describe("HTTPTransport", () => {
56
let mockFetch: jest.MockedFunction<typeof fetch>;
@@ -179,4 +180,56 @@ describe("HTTPTransport", () => {
179180
expect(injectedFetchMock.mock.calls.length).toEqual(1);
180181
expect(result).toEqual(undefined);
181182
});
183+
184+
it("should use global fetch if no fetcher provided", async () => {
185+
const globalFetch = jest.fn().mockImplementation((url, options) => {
186+
const body = options?.body as string;
187+
const responseText = reqMocks.generateMockResponseData(url.toString(), body);
188+
return Promise.resolve({
189+
text: () => Promise.resolve(responseText),
190+
});
191+
}) as any;
192+
global.fetch = globalFetch;
193+
194+
const httpTransport = new HTTPTransport("http://localhost:8545/rpc-request");
195+
await httpTransport.sendData({
196+
request: reqMocks.generateMockRequest(1, "foo", ["bar"]),
197+
internalID: 1,
198+
});
199+
200+
expect(globalFetch).toHaveBeenCalled();
201+
});
202+
203+
it("should abort the fetch request when signal is aborted", async () => {
204+
const controller = new AbortController();
205+
const httpTransport = new HTTPTransport("http://localhost:8545", {
206+
fetcher: mockFetch,
207+
});
208+
const data = reqMocks.generateMockRequest(1, "foo", ["bar"]);
209+
210+
// Setup mock to fail if aborted (simulating fetch behavior)
211+
mockFetch.mockImplementation(async (_url, options) => {
212+
if (options?.signal?.aborted) {
213+
throw new DOMException("The user aborted a request.", "AbortError");
214+
}
215+
return new Promise((_, reject) => {
216+
options?.signal?.addEventListener("abort", () => {
217+
reject(new DOMException("The user aborted a request.", "AbortError"));
218+
});
219+
});
220+
});
221+
222+
const prom = httpTransport.sendData({
223+
request: data,
224+
internalID: 1,
225+
}, null, controller.signal);
226+
227+
controller.abort();
228+
229+
await expect(prom).rejects.toThrow(AbortError);
230+
231+
const callArgs = mockFetch.mock.calls[0];
232+
expect(callArgs[1]?.signal).toBeDefined();
233+
expect(callArgs[1]?.signal?.aborted).toBe(true);
234+
});
182235
});

src/transports/HTTPTransport.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,10 @@ class HTTPTransport extends Transport {
3434
public async sendData(
3535
data: JSONRPCRequestData,
3636
timeout: number | null = null,
37+
signal?: AbortSignal | null,
3738
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3839
): Promise<any> {
39-
const prom = this.transportRequestManager.addRequest(data, timeout);
40+
const prom = this.transportRequestManager.addRequest(data, timeout, signal);
4041
const notifications = getNotifications(data);
4142
const batch = getBatchRequests(data);
4243
const fetcher = this.injectedFetcher || fetch;
@@ -46,6 +47,7 @@ class HTTPTransport extends Transport {
4647
headers: this.headers,
4748
body: JSON.stringify(this.parseData(data)),
4849
credentials: this.credentials,
50+
signal,
4951
});
5052
// requirements are that notifications are successfully sent
5153
this.transportRequestManager.settlePendingRequest(notifications);
@@ -55,23 +57,28 @@ class HTTPTransport extends Transport {
5557
const body = await result.text();
5658
const responseErr = this.transportRequestManager.resolveResponse(body);
5759
if (responseErr) {
58-
// requirements are that batch requuests are successfully resolved
60+
// requirements are that batch requests are successfully resolved
5961
// this ensures that individual requests within the batch request are settled
6062
this.transportRequestManager.settlePendingRequest(batch, responseErr);
6163
return Promise.reject(responseErr);
6264
}
6365
} catch (e) {
6466
const error = e as Error;
6567
const responseErr = new JSONRPCError(error.message, ERR_UNKNOWN, error);
68+
// requirements are that notifications are successfully resolved
6669
this.transportRequestManager.settlePendingRequest(
6770
notifications,
6871
responseErr,
6972
);
73+
// requirements are that batch requests are successfully resolved
7074
this.transportRequestManager.settlePendingRequest(
7175
getBatchRequests(data),
7276
responseErr,
7377
);
74-
return Promise.reject(responseErr);
78+
// requirements are that individual requests are successfully resolved
79+
if (!Array.isArray(data)) {
80+
this.transportRequestManager.settlePendingRequest([data], responseErr);
81+
}
7582
}
7683
return prom;
7784
}

src/transports/PostMessageIframeTransport.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ class PostMessageIframeTransport extends Transport {
5151
public async sendData(
5252
data: JSONRPCRequestData,
5353
_timeout: number | null = 5000,
54+
signal?: AbortSignal | null,
5455
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5556
): Promise<any> {
56-
const prom = this.transportRequestManager.addRequest(data, null);
57+
const prom = this.transportRequestManager.addRequest(data, null, signal);
5758
const notifications = getNotifications(data);
5859
if (this.frame) {
5960
this.frame.postMessage((data as IJSONRPCData).request, "*");

0 commit comments

Comments
 (0)