fix: deterministic id and cleanup

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,30 +64,30 @@ describe("client-js", () => {
         },
         {
           jsonrpc: "2.0",
-          id: 4,
+          id: "1",
           result: "bar",
         },
       ]);
       transport.connection.emit("message", result);
     };
 
     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,33 +96,30 @@ 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",
         },
       ]);
       transport.connection.emit("message", result);
     };
 
     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 () => {

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<any> {
+    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}`);
+      }
+    });
   }
 }