From accbf806fe03d4875dd3161cbcd7ff1662719c90 Mon Sep 17 00:00:00 2001 From: Zane Starr Date: Wed, 16 Dec 2020 11:06:43 -0800 Subject: [PATCH] fix: this corrects default timeout to be disabled by specifing null. This fix was needed because the old default used undefined, which in turn triggered the default timeout when left unspecified, but would not allow the user to disable timeouts. This refactor allows users to be specific about what they want specifying default(undefined),custom timeout, or no timeout (null). fixes #231 --- src/Client.ts | 2 +- src/RequestManager.ts | 2 +- src/transports/EventEmitterTransport.ts | 2 +- src/transports/HTTPTransport.ts | 2 +- src/transports/PostMessageIframeTransport.ts | 4 ++-- src/transports/PostMessageWindowTransport.ts | 2 +- src/transports/Transport.ts | 2 +- src/transports/TransportRequestManager.test.ts | 14 +++++++------- src/transports/TransportRequestManager.ts | 8 ++++---- src/transports/WebSocketTransport.ts | 2 +- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/Client.ts b/src/Client.ts index 57618e2..76ce0f4 100644 --- a/src/Client.ts +++ b/src/Client.ts @@ -78,7 +78,7 @@ class Client implements IClient { if (this.requestManager.connectPromise) { await this.requestManager.connectPromise; } - return this.requestManager.request(requestObject, true); + return this.requestManager.request(requestObject, true, null); } public onNotification(callback: (data: IJSONRPCNotification) => void) { diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 348f699..7b2aa5d 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -44,7 +44,7 @@ class RequestManager { return this.transports[0]; } - public async request(requestObject: JSONRPCMessage, notification: boolean = false, timeout?: number): Promise { + public async request(requestObject: JSONRPCMessage, notification: boolean = false, timeout?: number | null): Promise { const internalID = (++this.lastId).toString(); const id = notification ? null : internalID; // naively grab first transport and use it diff --git a/src/transports/EventEmitterTransport.ts b/src/transports/EventEmitterTransport.ts index 5b5271b..9583581 100644 --- a/src/transports/EventEmitterTransport.ts +++ b/src/transports/EventEmitterTransport.ts @@ -22,7 +22,7 @@ class EventEmitterTransport extends Transport { return Promise.resolve(); } - public sendData(data: JSONRPCRequestData, timeout?: number): Promise { + public sendData(data: JSONRPCRequestData, timeout: number | null = null): Promise { const prom = this.transportRequestManager.addRequest(data, timeout); const notifications = getNotifications(data); const parsedData = this.parseData(data); diff --git a/src/transports/HTTPTransport.ts b/src/transports/HTTPTransport.ts index cf7a4f2..3532bd3 100644 --- a/src/transports/HTTPTransport.ts +++ b/src/transports/HTTPTransport.ts @@ -24,7 +24,7 @@ class HTTPTransport extends Transport { return Promise.resolve(); } - public async sendData(data: JSONRPCRequestData, timeout?: number): Promise { + public async sendData(data: JSONRPCRequestData, timeout: number | null = null): Promise { const prom = this.transportRequestManager.addRequest(data, timeout); const notifications = getNotifications(data); const batch = getBatchRequests(data); diff --git a/src/transports/PostMessageIframeTransport.ts b/src/transports/PostMessageIframeTransport.ts index e1b94c7..47b4f55 100644 --- a/src/transports/PostMessageIframeTransport.ts +++ b/src/transports/PostMessageIframeTransport.ts @@ -45,8 +45,8 @@ class PostMessageIframeTransport extends Transport { }); } - public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise { - const prom = this.transportRequestManager.addRequest(data, undefined); + public async sendData(data: JSONRPCRequestData, timeout: number | null = 5000): Promise { + const prom = this.transportRequestManager.addRequest(data, null); if (this.frame) { this.frame.postMessage((data as IJSONRPCData).request, "*"); } diff --git a/src/transports/PostMessageWindowTransport.ts b/src/transports/PostMessageWindowTransport.ts index cc9be8a..00f1905 100644 --- a/src/transports/PostMessageWindowTransport.ts +++ b/src/transports/PostMessageWindowTransport.ts @@ -55,7 +55,7 @@ class PostMessageTransport extends Transport { } public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise { - const prom = this.transportRequestManager.addRequest(data, undefined); + const prom = this.transportRequestManager.addRequest(data, null); if (this.frame) { this.frame.postMessage((data as IJSONRPCData).request, this.uri); } diff --git a/src/transports/Transport.ts b/src/transports/Transport.ts index afd44b1..d18d3d2 100644 --- a/src/transports/Transport.ts +++ b/src/transports/Transport.ts @@ -30,7 +30,7 @@ export abstract class Transport { public abstract connect(): Promise; public abstract close(): void; - public abstract async sendData(data: JSONRPCRequestData, timeout?: number): Promise; + public abstract async sendData(data: JSONRPCRequestData, timeout?: number | null): Promise; public subscribe(event: TransportEventName, handler: ITransportEvents[TransportEventName]) { this.transportRequestManager.transportEventChannel.addListener(event, handler); diff --git a/src/transports/TransportRequestManager.test.ts b/src/transports/TransportRequestManager.test.ts index ecbcda3..3e4f1e6 100644 --- a/src/transports/TransportRequestManager.test.ts +++ b/src/transports/TransportRequestManager.test.ts @@ -13,7 +13,7 @@ describe("Transport Request Manager", () => { expect(data).toBeDefined(); done(); }); - transportReqMan.addRequest({ request: reqData.generateMockRequest(1, "foo", ["bar"]), internalID: 1 }, undefined); + transportReqMan.addRequest({ request: reqData.generateMockRequest(1, "foo", ["bar"]), internalID: 1 }, null); }); it("should timeout pending request after 1s", async () => { @@ -35,7 +35,7 @@ describe("Transport Request Manager", () => { // tslint:disable-next-line:no-empty const reject = () => { }; const request: IBatchRequest[] = [{ resolve, reject, request: req }]; - transportReqMan.addRequest(request, undefined); + transportReqMan.addRequest(request, null); }); it("should error on missing id to resolve", () => { @@ -72,7 +72,7 @@ describe("Transport Request Manager", () => { it("should add and reject pending requests", async () => { const request = { request: reqData.generateMockRequest(1, "foo", ["bar"]), internalID: 1 }; - const prom = transportReqMan.addRequest(request, undefined); + const prom = transportReqMan.addRequest(request, null); transportReqMan.settlePendingRequest([request], new Error("rejecting")); await expect(prom).rejects.toThrowError("rejecting"); }); @@ -101,7 +101,7 @@ describe("Transport Request Manager", () => { const prom = transportReqMan.addRequest({ request: reqData.generateMockRequest(1, "foo", ["bar"]), internalID: 1, - }, undefined); + }, null); // Verify that the response resolves the pending request and the response event fires transportReqMan.transportEventChannel.on("response", async (responseData) => { @@ -130,7 +130,7 @@ describe("Transport Request Manager", () => { // tslint:disable-next-line:no-empty const reject = () => { }; - const prom = transportReqMan.addRequest([{ request: requestData, resolve, reject }], undefined); + const prom = transportReqMan.addRequest([{ request: requestData, resolve, reject }], null); // Verify that the response resolves the pending request and the response event fires transportReqMan.transportEventChannel.on("response", (responseData) => { @@ -158,7 +158,7 @@ describe("Transport Request Manager", () => { // tslint:disable-next-line:no-empty const reject = () => { }; - transportReqMan.addRequest([{ request: requestData, resolve, reject }], undefined); + transportReqMan.addRequest([{ request: requestData, resolve, reject }], null); // Resolve pending request; const err = transportReqMan.resolveResponse(JSON.stringify([res]), false) as Error; @@ -184,7 +184,7 @@ describe("Transport Request Manager", () => { const prom = transportReqMan.addRequest({ request: reqData.generateMockRequest(1, "foo", ["bar"]), internalID: 1, - }, undefined); + }, null); transportReqMan.transportEventChannel.on("response", async (data) => { if (data.error === undefined) { throw new Error("Missing error"); diff --git a/src/transports/TransportRequestManager.ts b/src/transports/TransportRequestManager.ts index 53e6869..ea66454 100644 --- a/src/transports/TransportRequestManager.ts +++ b/src/transports/TransportRequestManager.ts @@ -23,7 +23,7 @@ export class TransportRequestManager { this.pendingBatchRequest = {}; this.transportEventChannel = new EventEmitter(); } - public addRequest(data: JSONRPCRequestData, timeout: number | undefined): Promise { + public addRequest(data: JSONRPCRequestData, timeout: number | null): Promise { this.transportEventChannel.emit("pending", data); if (data instanceof Array) { this.addBatchReq(data, timeout); @@ -67,7 +67,7 @@ export class TransportRequestManager { } } - private addBatchReq(batches: IBatchRequest[], timeout: number | undefined) { + private addBatchReq(batches: IBatchRequest[], timeout: number | null) { batches.forEach((batch) => { const { resolve, reject } = batch; const { internalID } = batch.request; @@ -76,9 +76,9 @@ export class TransportRequestManager { }); return Promise.resolve(); } - private addReq(id: string | number, timeout?: number) { + private addReq(id: string | number, timeout: number | null) { return new Promise((resolve, reject) => { - if (timeout) { + if (timeout !== null && timeout) { this.setRequestTimeout(id, timeout, reject); } this.pendingRequest[id] = { resolve, reject }; diff --git a/src/transports/WebSocketTransport.ts b/src/transports/WebSocketTransport.ts index 4139b94..9c38d79 100644 --- a/src/transports/WebSocketTransport.ts +++ b/src/transports/WebSocketTransport.ts @@ -26,7 +26,7 @@ class WebSocketTransport extends Transport { }); } - public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise { + public async sendData(data: JSONRPCRequestData, timeout: number | null = 5000): Promise { let prom = this.transportRequestManager.addRequest(data, timeout); const notifications = getNotifications(data); this.connection.send(JSON.stringify(this.parseData(data)), (err?: Error) => {