From 097c960bd859cb953d0493453af449cde2bceea3 Mon Sep 17 00:00:00 2001 From: Zachary Belford Date: Thu, 25 Jul 2019 12:05:13 -0700 Subject: [PATCH] fix: deterministic id and cleanup --- src/RequestManager.test.ts | 65 ++++++++++++++++++-------------------- src/RequestManager.ts | 61 ++++++++++++++--------------------- 2 files changed, 55 insertions(+), 71 deletions(-) diff --git a/src/RequestManager.test.ts b/src/RequestManager.test.ts index e65cc4e..041c8d0 100644 --- a/src/RequestManager.test.ts +++ b/src/RequestManager.test.ts @@ -47,13 +47,13 @@ describe("client-js", () => { }); }); - it("can return errors on batchng requests", async () => { + it("can return errors on batch requests", async () => { const transport = new EventEmitterTransport("foo://unique-uri"); transport.sendData = (data) => { const result = JSON.stringify([ { jsonrpc: "2.0", - id: 3, + id: "0", error: { code: 509, message: "too much 509", @@ -64,7 +64,7 @@ describe("client-js", () => { }, { jsonrpc: "2.0", - id: 4, + id: "1", result: "bar", }, ]); @@ -72,22 +72,22 @@ describe("client-js", () => { }; const c = new RequestManager([transport]); - return c.connect().then(() => { - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - c.endBatch(); - expect(Promise.all(requests)).rejects.toEqual({ - code: 509, - message: "too much 509", - data: { - test: "data", - }, - }); - c.close(); + await c.connect(); + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + expect(requests[0]).rejects.toEqual({ + code: 509, + message: "too much 509", + data: { + test: "data", + }, }); + expect(requests[1]).resolves.toEqual("bar"); + c.endBatch(); + c.close(); }); it("can batch a request", async () => { @@ -96,12 +96,12 @@ describe("client-js", () => { const result = JSON.stringify([ { jsonrpc: "2.0", - id: 5, + id: "0", result: "foo", }, { jsonrpc: "2.0", - id: 6, + id: "1", result: "bar", }, ]); @@ -109,20 +109,17 @@ describe("client-js", () => { }; const c = new RequestManager([transport]); - return c.connect().then(() => { - c.startBatch(); - const requests = [ - c.request("foo", []), - c.request("foo", []), - ]; - c.endBatch(); - return Promise.all(requests).then((results) => { - expect(results[0]).toEqual("foo"); - expect(results[1]).toEqual("bar"); - c.close(); - }); - - }); + await c.connect(); + c.startBatch(); + const requests = [ + c.request("foo", []), + c.request("foo", []), + ]; + c.endBatch(); + const [a, b] = await Promise.all(requests); + expect(a).toEqual("foo"); + expect(b).toEqual("bar"); + c.close(); }); it("can send a request and error", async () => { diff --git a/src/RequestManager.ts b/src/RequestManager.ts index 73b51f7..433d129 100644 --- a/src/RequestManager.ts +++ b/src/RequestManager.ts @@ -1,10 +1,8 @@ import ITransport from "./transports/Transport"; -let id = 1; - interface IJSONRPCRequest { jsonrpc: "2.0"; - id: number; + id: string; method: string; params: any[] | object; } @@ -16,7 +14,7 @@ interface IJSONRPCError { interface IJSONRPCResponse { jsonrpc: "2.0"; - id: number; + id: string; // can also be null result?: any; error?: IJSONRPCError; } @@ -38,6 +36,7 @@ class RequestManager { private requests: any; private batchStarted: boolean = false; private batch: IJSONRPCRequest[] = []; + private lastId: number = -1; constructor(transports: ITransport[]) { this.transports = transports; @@ -59,14 +58,15 @@ class RequestManager { } public async request(method: string, params: any): Promise { + const i = (++this.lastId).toString(); return new Promise((resolve, reject) => { - const i = id++; // naively grab first transport and use it const transport = this.transports[0]; this.requests[i] = { resolve, reject, }; + const payload: IJSONRPCRequest = { jsonrpc: "2.0", id: i, @@ -78,7 +78,7 @@ class RequestManager { } else { transport.sendData(JSON.stringify(payload)); } - }); + }).finally(() => this.requests[i] = undefined); } public close(): void { @@ -98,9 +98,6 @@ class RequestManager { this.batchStarted = true; } - /** - * - */ public endBatch(): void { if (this.batchStarted === false) { throw new Error("cannot end that which has never started"); @@ -117,34 +114,24 @@ class RequestManager { private onData(data: string): void { const parsedData: IJSONRPCResponse[] | IJSONRPCResponse = JSON.parse(data); - // handle batch requests - if (Array.isArray(parsedData)) { - parsedData.forEach((response) => { - if (!this.requests[response.id]) { - return; - } - if (response.error) { - this.requests[response.id].reject(response.error); - } else { - this.requests[response.id].resolve(response.result); - } - }); - return; - } - if (typeof parsedData.result === "undefined" && typeof parsedData.error === "undefined") { - return; - } - const req = this.requests[parsedData.id]; - if (req === undefined) { - return; - } - // resolve promise for id - if (parsedData.error) { - req.reject(parsedData.error); - } else { - req.resolve(parsedData.result); - } - delete this.requests[parsedData.id]; + const results = parsedData instanceof Array ? parsedData : [parsedData]; + + results.forEach((response) => { + const promiseForResult = this.requests[response.id]; + if (promiseForResult === undefined) { + throw new Error( + `Received an unrecognized response id: ${response.id}. Valid ids are: ${Object.keys(this.requests)}`, + ); + } + + if (response.error) { + promiseForResult.reject(response.error); + } else if (response.result) { + promiseForResult.resolve(response.result); + } else { + throw new Error(`Malformed JSON-RPC response object: ${response}`); + } + }); } }