Improve our compliance to the JSON RPC spec

5 files changed, 37 insertions, 33 deletions

diff --git a/packages/order-watcher/README.md b/packages/order-watcher/README.md
index aad90a59a..385fe4715 100644
--- a/packages/order-watcher/README.md
+++ b/packages/order-watcher/README.md
@@ -40,7 +40,7 @@ Several environmental variables can be set to configure the server:
     and accept connections from. When this is not set, we default to 8080.
 
 **Requests**
-The server accepts three types of requests: `ADD_ORDER`, `REMOVE_ORDER` and `GET_STATS`. These mirror what the underlying OrderWatcher does. You can read more in the [wiki](https://0xproject.com/wiki#0x-OrderWatcher). Unlike the OrderWatcher, it does not expose any subscribe or unsubscribe functionality because the client implicitly subscribes and unsubscribes by connecting to the server.
+The server accepts three types of requests: `ADD_ORDER`, `REMOVE_ORDER` and `GET_STATS`. These mirror what the underlying OrderWatcher does. You can read more in the [wiki](https://0xproject.com/wiki#0x-OrderWatcher). Unlike the OrderWatcher, it does not expose any `subscribe` or `unsubscribe` functionality because the WebSocket server keeps a single subscription open for all clients.
 
 The first step for making a request is establishing a connection with the server. In Javascript:
 
@@ -58,9 +58,9 @@ wsClient = create_connection("ws://127.0.0.1:8080")
 
 With the connection established, you prepare the payload for your request. The payload is a json object with a format established by the [JSON RPC specification](https://www.jsonrpc.org/specification):
 
-*   `id`: All requests require you to specify a string as an id. When the server responds to the request, it provides an id as well to allow you to determine which request it is responding to.
+*   `id`: All requests require you to specify a numerical `id`. When the server responds to the request, the response will have the same `id` as the one supplied with your request.
 *   `jsonrpc`: This is always the string `'2.0'`.
-*   `method`: This specifies the OrderWatcher method you want to call. I.e., `'ADD_ORDER'`, `'REMOVE_ORDER'`, and `'GET_STATS'`.
+*   `method`: This specifies the OrderWatcher method you want to call. I.e., `'ADD_ORDER'`, `'REMOVE_ORDER'` or `'GET_STATS'`.
 *   `params`: These contain the parameters needed by OrderWatcher to execute the method you called. For `ADD_ORDER`, provide `{ signedOrder: <your signedOrder> }`. For `REMOVE_ORDER`, provide `{ orderHash: <your orderHash> }`. For `GET_STATS`, no parameters are needed, so you may leave this empty.
 
 Next, convert the payload to a string and send it through the connection.
@@ -68,7 +68,7 @@ In Javascript:
 
 ```
 const addOrderPayload = {
-    id: 'order32',
+    id: 1,
     jsonrpc: '2.0',
     method: 'ADD_ORDER',
     params: { signedOrder: <your signedOrder> },
@@ -81,7 +81,7 @@ In Python:
 ```
 import json
 remove_order_payload = {
-    'id': 'order33',
+    'id': 1,
     'jsonrpc': '2.0',
     'method': 'REMOVE_ORDER',
     'params': {'orderHash': '0x6edc16bf37fde79f5012088c33784c730e2f103d9ab1caf73060c386ad107b7e'},
@@ -90,13 +90,13 @@ wsClient.send(json.dumps(remove_order_payload));
 ```
 
 **Response**
-The server responds to all requests in a similar format. In the data field, you'll find another json object that has been converted into a string. This json object contains the following fields:
+The server responds to all requests in a similar format. In the data field, you'll find another object containing the following fields:
 
-*   `id`: The id corresponding to the request that the server is responding to. `UPDATE` responses are not based on any requests so the `id` field is `null`.
+*   `id`: The id corresponding to the request that the server is responding to. `UPDATE` responses are not based on any requests so the `id` field is omitted`.
 *   `jsonrpc`: Always `'2.0'`.
 *   `method`: The method the server is responding to. Eg. `ADD_ORDER`. When order states change the server may also initiate a response. In this case, method will be listed as `UPDATE`.
-*   `result`: This field varies based on the method. `UPDATE` responses contained the new order state. `GET_STATS` responses contain the current order count. When there are errors, this field is `null`.
-*   `error`: When there is an error executing a request, the error message is listed here. When the server responds successfully, this field is `null`.
+*   `result`: This field varies based on the method. `UPDATE` responses contain the new order state. `GET_STATS` responses contain the current order count. When there are errors, this field is omitted.
+*   `error`: When there is an error executing a request, the [JSON RPC](https://www.jsonrpc.org/specification) error object is listed here. When the server responds successfully, this field is omitted.
 
 In Javascript, the responses can be parsed using the `onmessage` callback:
 
diff --git a/packages/order-watcher/src/order_watcher/order_watcher_websocket_server.ts b/packages/order-watcher/src/order_watcher/order_watcher_websocket_server.ts
index a1b63128f..eac48f849 100644
--- a/packages/order-watcher/src/order_watcher/order_watcher_websocket_server.ts
+++ b/packages/order-watcher/src/order_watcher/order_watcher_websocket_server.ts
@@ -12,7 +12,7 @@ import { assert } from '../utils/assert';
 import { OrderWatcher } from './order_watcher';
 
 const DEFAULT_HTTP_PORT = 8080;
-const JSONRPC_VERSION = '2.0';
+const JSON_RPC_VERSION = '2.0';
 
 // Wraps the OrderWatcher functionality in a WebSocket server. Motivations:
 // 1) Users can watch orders via non-typescript programs.
@@ -77,16 +77,17 @@ export class OrderWatcherWebSocketServer {
             connection.on('close', this._onCloseCallback.bind(this, connection));
             this._connectionStore.add(connection);
         });
+    }
 
+    /**
+     * Activates the WebSocket server by subscribing to the OrderWatcher and
+     * starting the WebSocket's HTTP server
+     */
+    public start(): void {
         // Have the WebSocket server subscribe to the OrderWatcher to receive updates.
         // These updates are then broadcast to clients in the _connectionStore.
         this._orderWatcher.subscribe(this._broadcastCallback.bind(this));
-    }
 
-    /**
-     * Activates the WebSocket server by having its HTTP server start listening.
-     */
-    public listen(): void {
         const port = process.env.ORDER_WATCHER_HTTP_PORT || DEFAULT_HTTP_PORT;
         this._httpServer.listen(port, () => {
             logUtils.log(`${new Date()} [Server] Listening on port ${port}`);
@@ -95,29 +96,30 @@ export class OrderWatcherWebSocketServer {
 
     /**
      * Deactivates the WebSocket server by stopping the HTTP server from accepting
-     * new connections.
+     * new connections and unsubscribing from the OrderWatcher
      */
-    public close(): void {
+    public stop(): void {
         this._httpServer.close();
+        this._orderWatcher.unsubscribe();
     }
 
     private async _onMessageCallbackAsync(connection: WebSocket.connection, message: any): Promise<void> {
         let response: WebSocketResponse;
+        assert.doesConformToSchema('message', message, webSocketUtf8MessageSchema);
+        const request: WebSocketRequest = JSON.parse(message.utf8Data);
+        assert.doesConformToSchema('request', request, webSocketRequestSchema);
+        assert.isString(request.jsonrpc, JSON_RPC_VERSION);
         try {
-            assert.doesConformToSchema('message', message, webSocketUtf8MessageSchema);
-            const request: WebSocketRequest = JSON.parse(message.utf8Data);
-            assert.doesConformToSchema('request', request, webSocketRequestSchema);
-            assert.isString(request.jsonrpc, JSONRPC_VERSION);
             response = {
                 id: request.id,
-                jsonrpc: JSONRPC_VERSION,
+                jsonrpc: JSON_RPC_VERSION,
                 method: request.method,
                 result: await this._routeRequestAsync(request),
             };
         } catch (err) {
             response = {
-                id: null,
-                jsonrpc: JSONRPC_VERSION,
+                id: request.id,
+                jsonrpc: JSON_RPC_VERSION,
                 method: null,
                 error: err.toString(),
             };
@@ -165,12 +167,12 @@ export class OrderWatcherWebSocketServer {
         const response =
             err === null
                 ? {
-                      jsonrpc: JSONRPC_VERSION,
+                      jsonrpc: JSON_RPC_VERSION,
                       method,
                       result: orderState,
                   }
                 : {
-                      jsonrpc: JSONRPC_VERSION,
+                      jsonrpc: JSON_RPC_VERSION,
                       method,
                       error: {
                           code: -32000,
diff --git a/packages/order-watcher/src/schemas/websocket_schemas.ts b/packages/order-watcher/src/schemas/websocket_schemas.ts
index 5e4e1ab74..263dd45b3 100644
--- a/packages/order-watcher/src/schemas/websocket_schemas.ts
+++ b/packages/order-watcher/src/schemas/websocket_schemas.ts
@@ -1,3 +1,5 @@
+// TODO: Move these schemas to the `json-schemas` package and convert to JSON
+// Rename to `OrderWatcherWebSocketRequestSchema`, etc...
 export const webSocketUtf8MessageSchema = {
     id: '/webSocketUtf8MessageSchema',
     properties: {
diff --git a/packages/order-watcher/src/types.ts b/packages/order-watcher/src/types.ts
index ecbebe305..536363d8a 100644
--- a/packages/order-watcher/src/types.ts
+++ b/packages/order-watcher/src/types.ts
@@ -49,21 +49,21 @@ export enum OrderWatcherMethod {
 export type WebSocketRequest = AddOrderRequest | RemoveOrderRequest | GetStatsRequest;
 
 interface AddOrderRequest {
-    id: string;
+    id: number;
     jsonrpc: string;
     method: OrderWatcherMethod.AddOrder;
     params: { signedOrder: SignedOrder };
 }
 
 interface RemoveOrderRequest {
-    id: string;
+    id: number;
     jsonrpc: string;
     method: OrderWatcherMethod.RemoveOrder;
     params: { orderHash: string };
 }
 
 interface GetStatsRequest {
-    id: string;
+    id: number;
     jsonrpc: string;
     method: OrderWatcherMethod.GetStats;
 }
@@ -73,14 +73,14 @@ interface GetStatsRequest {
 export type WebSocketResponse = SuccessfulWebSocketResponse | ErrorWebSocketResponse;
 
 interface SuccessfulWebSocketResponse {
-    id: string | null; // id is null for UPDATE
+    id: number;
     jsonrpc: string;
     method: OrderWatcherMethod;
     result: OrderState | GetStatsResult | undefined; // result is undefined for ADD_ORDER and REMOVE_ORDER
 }
 
 interface ErrorWebSocketResponse {
-    id: null;
+    id: number;
     jsonrpc: string;
     method: null;
     error: JSONRPCError;
diff --git a/packages/order-watcher/test/order_watcher_websocket_server_test.ts b/packages/order-watcher/test/order_watcher_websocket_server_test.ts
index 9f9db7b1f..8a6deede8 100644
--- a/packages/order-watcher/test/order_watcher_websocket_server_test.ts
+++ b/packages/order-watcher/test/order_watcher_websocket_server_test.ts
@@ -105,11 +105,11 @@ describe.only('OrderWatcherWebSocketServer', async () => {
         // Prepare OrderWatcher WebSocket server
         const orderWatcherConfig = {};
         wsServer = new OrderWatcherWebSocketServer(provider, networkId, contractAddresses, orderWatcherConfig);
-        wsServer.listen();
+        wsServer.start();
     });
     after(async () => {
         await blockchainLifecycle.revertAsync();
-        wsServer.close();
+        wsServer.stop();
     });
     beforeEach(async () => {
         await blockchainLifecycle.startAsync();