Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: rm notification default timeout #232

Merged
merged 2 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/RequestManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class RequestManager {
return this.transports[0];
}

public async request(requestObject: JSONRPCMessage, notification: boolean = false, timeout?: number): Promise<any> {
public async request(requestObject: JSONRPCMessage, notification: boolean = false, timeout?: number | null): Promise<any> {
const internalID = (++this.lastId).toString();
const id = notification ? null : internalID;
// naively grab first transport and use it
Expand Down
2 changes: 1 addition & 1 deletion src/transports/EventEmitterTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class EventEmitterTransport extends Transport {
return Promise.resolve();
}

public sendData(data: JSONRPCRequestData, timeout?: number): Promise<any> {
public sendData(data: JSONRPCRequestData, timeout: number | null = null): Promise<any> {
const prom = this.transportRequestManager.addRequest(data, timeout);
const notifications = getNotifications(data);
const parsedData = this.parseData(data);
Expand Down
2 changes: 1 addition & 1 deletion src/transports/HTTPTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class HTTPTransport extends Transport {
return Promise.resolve();
}

public async sendData(data: JSONRPCRequestData, timeout?: number): Promise<any> {
public async sendData(data: JSONRPCRequestData, timeout: number | null = null): Promise<any> {
const prom = this.transportRequestManager.addRequest(data, timeout);
const notifications = getNotifications(data);
const batch = getBatchRequests(data);
Expand Down
4 changes: 2 additions & 2 deletions src/transports/PostMessageIframeTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class PostMessageIframeTransport extends Transport {
});
}

public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise<any> {
const prom = this.transportRequestManager.addRequest(data, undefined);
public async sendData(data: JSONRPCRequestData, timeout: number | null = 5000): Promise<any> {
const prom = this.transportRequestManager.addRequest(data, null);
if (this.frame) {
this.frame.postMessage((data as IJSONRPCData).request, "*");
}
Expand Down
2 changes: 1 addition & 1 deletion src/transports/PostMessageWindowTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class PostMessageTransport extends Transport {
}

public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise<any> {
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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/transports/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export abstract class Transport {

public abstract connect(): Promise<any>;
public abstract close(): void;
public abstract async sendData(data: JSONRPCRequestData, timeout?: number): Promise<any>;
public abstract async sendData(data: JSONRPCRequestData, timeout?: number | null): Promise<any>;

public subscribe(event: TransportEventName, handler: ITransportEvents[TransportEventName]) {
this.transportRequestManager.transportEventChannel.addListener(event, handler);
Expand Down
14 changes: 7 additions & 7 deletions src/transports/TransportRequestManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 not error on missing id to resolve", () => {
Expand Down Expand Up @@ -68,7 +68,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");
});
Expand Down Expand Up @@ -97,7 +97,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) => {
Expand Down Expand Up @@ -126,7 +126,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) => {
Expand Down Expand Up @@ -154,7 +154,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;
Expand All @@ -180,7 +180,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");
Expand Down
8 changes: 4 additions & 4 deletions src/transports/TransportRequestManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export class TransportRequestManager {
this.pendingBatchRequest = {};
this.transportEventChannel = new EventEmitter();
}
public addRequest(data: JSONRPCRequestData, timeout: number | undefined): Promise<any> {
public addRequest(data: JSONRPCRequestData, timeout: number | null): Promise<any> {
this.transportEventChannel.emit("pending", data);
if (data instanceof Array) {
this.addBatchReq(data, timeout);
Expand Down Expand Up @@ -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;
Expand All @@ -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 };
Expand Down
2 changes: 1 addition & 1 deletion src/transports/WebSocketTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class WebSocketTransport extends Transport {
});
}

public async sendData(data: JSONRPCRequestData, timeout: number | undefined = 5000): Promise<any> {
public async sendData(data: JSONRPCRequestData, timeout: number | null = 5000): Promise<any> {
let prom = this.transportRequestManager.addRequest(data, timeout);
const notifications = getNotifications(data);
this.connection.send(JSON.stringify(this.parseData(data)), (err?: Error) => {
Expand Down