From dec147bc12c9131928330e4c0b9370d9c010c9ef Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Fri, 9 Feb 2024 14:54:19 -0500 Subject: [PATCH 01/40] initial pass at server supporting long running subscriptions via sockets --- src/dwn-server.ts | 3 +- src/storage.ts | 4 +- src/ws-api.ts | 334 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 238 insertions(+), 103 deletions(-) diff --git a/src/dwn-server.ts b/src/dwn-server.ts index 15878ef..6ba27e4 100644 --- a/src/dwn-server.ts +++ b/src/dwn-server.ts @@ -76,7 +76,8 @@ export class DwnServer { if (this.config.webSocketServerEnabled) { this.#wsApi = new WsApi(this.#httpApi.server, this.dwn); - this.#wsApi.start(() => log.info(`WebSocketServer ready...`)); + this.#wsApi.start(); + log.info('WebSocketServer ready...'); } } diff --git a/src/storage.ts b/src/storage.ts index 1988349..23b8e05 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -2,6 +2,7 @@ import * as fs from 'fs'; import { DataStoreLevel, + EventEmitterStream, EventLogLevel, MessageStoreLevel, } from '@tbd54566975/dwn-sdk-js'; @@ -55,7 +56,8 @@ export function getDWNConfig( EStoreType.MessageStore, ); - return { eventLog, dataStore, messageStore, tenantGate }; + const eventStream = config.webSocketServerEnabled ? new EventEmitterStream() : undefined; + return { eventStream, eventLog, dataStore, messageStore, tenantGate }; } function getLevelStore( diff --git a/src/ws-api.ts b/src/ws-api.ts index cac1cea..e8d664d 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -1,25 +1,231 @@ -import { DataStream, type Dwn } from '@tbd54566975/dwn-sdk-js'; - +import type { AddressInfo, WebSocket } from 'ws'; import type { IncomingMessage, Server } from 'http'; + import { base64url } from 'multiformats/bases/base64'; -import { v4 as uuidv4 } from 'uuid'; -import { type AddressInfo, type WebSocket, WebSocketServer } from 'ws'; +import log from 'loglevel'; +import { WebSocketServer } from 'ws'; + +import type { + Dwn, + GenericMessage, + MessageSubscription, + MessageSubscriptionHandler +} from '@tbd54566975/dwn-sdk-js' + +import { DataStream, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; + +import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './lib/json-rpc.js'; -import type { RequestContext } from './lib/json-rpc-router.js'; import { createJsonRpcErrorResponse, - JsonRpcErrorCodes, - type JsonRpcResponse, + createJsonRpcSuccessResponse, + JsonRpcErrorCodes } from './lib/json-rpc.js'; -import { jsonRpcApi } from './json-rpc-api.js'; -import { requestCounter } from './metrics.js'; - const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; +export interface SubscriptionManager { + subscribe: (target: string, subscription: MessageSubscription) => Promise; + close: (target: string, id: string) => Promise; + closeAll: () => Promise; +} + +class Manager { + constructor(private subscriptions: Map> = new Map()){}; + async subscribe(target: string, subscription: MessageSubscription): Promise { + let targetSubscriptions = this.subscriptions.get(target); + if (targetSubscriptions === undefined) { + targetSubscriptions = new Map(); + this.subscriptions.set(target, targetSubscriptions); + } + targetSubscriptions.set(subscription.id, subscription); + } + + async close(target: string, id: string): Promise { + const targetSubscriptions = this.subscriptions.get(target); + if (targetSubscriptions !== undefined) { + const subscription = targetSubscriptions.get(id); + if (subscription !== undefined) { + targetSubscriptions.delete(id); + await subscription.close(); + return; + } + } + + // if it reached here no subscription to close + throw new Error('could not find subscription to close'); + } + + async closeAll(): Promise { + const closePromises = []; + for (const [target, subscriptions] of this.subscriptions) { + this.subscriptions.delete(target); + for (const [id, subscription] of subscriptions) { + subscriptions.delete(id); + closePromises.push(subscription.close()); + } + } + + await Promise.all(closePromises); + } +} + +export class SocketConnection { + private id: string; + private subscriptions: SubscriptionManager; + + constructor(private socket: WebSocket, private dwn: Dwn){ + this.id = crypto.randomUUID(); + this.subscriptions = new Manager(); + socket.on('close', this.close.bind(this)); + socket.on('pong', this.pong.bind(this)); + socket.on('error', this.error.bind(this)); + socket.on('message', this.message.bind(this)); + socket[SOCKET_ISALIVE_SYMBOL] = true; + } + + get isAlive(): boolean { + return this.socket[SOCKET_ISALIVE_SYMBOL]; + } + + /** + * Closes the existing connection and cleans up any listeners or subscriptions. + */ + async close(): Promise { + // clean up all socket event listeners + this.socket.removeAllListeners(); + + // close all of the associated subscriptions + await this.subscriptions.closeAll(); + } + + ping(): void { + this.socket[SOCKET_ISALIVE_SYMBOL] = false; + this.socket.ping(); + } + + /** + * Pong messages are automatically sent in response to ping messages as required by + * the websocket spec. So, no need to send explicit pongs from browser + */ + private pong(): void { + this.socket[SOCKET_ISALIVE_SYMBOL] = true; + } + + private async error(error?:Error): Promise{ + if (error !== undefined) { + log.error('WebSocket', this.id, error); + this.socket.terminate(); + await this.close() + } + } + + private async message(dataBuffer: Buffer): Promise { + + const requestData = dataBuffer.toString(); + if (!requestData) { + return this.send(createJsonRpcErrorResponse( + crypto.randomUUID(), + JsonRpcErrorCodes.BadRequest, + 'request payload required.' + )) + } + + let jsonRequest: JsonRpcRequest; + try { + jsonRequest = JSON.parse(requestData); + } catch(error) { + const errorResponse = createJsonRpcErrorResponse( + crypto.randomUUID(), + JsonRpcErrorCodes.BadRequest, + (error as Error).message + ); + + return this.send(errorResponse); + }; + + const { id, params, method } = jsonRequest; + const { target, message, subscriptionId, encodedData } = params; + + // DISCUSSION: Should this be a DWN message or is this rpc specific to the server? + // Having it as a DWN message feels like the incorrect approach as both `Subscribe` and a potential `Unsubscribe` are ephemeral. + // They do not propagate between DWNs in a way that the current DWN holding the Subscription would eventually get an Unsubscribe. + // The `Unsubscribe` is specifically targeted to the live subscription transport. Am open to other ideas on how to handle this. + + if (method === 'dwn.closeSubscription' && subscriptionId !== undefined) { + return await this.closeSubscription(id, target, subscriptionId); + } else if (method === 'dwn.processMessage' && target && message) { + return await this.processMessage({ id, target, message, encodedData }); + } else { + const errorResponse = createJsonRpcErrorResponse( + id, + JsonRpcErrorCodes.InvalidRequest, + `${method} is not supported.`, + ); + this.send(errorResponse); + } + } + + private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { + this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); + } + + private async closeSubscription(id: JsonRpcId, target: string, subscriptionId: string ): Promise { + try { + await this.subscriptions.close(target, subscriptionId); + const response = createJsonRpcSuccessResponse(id, { reply: { status: 200, detail: 'Accepted' } }); + this.send(response); + } catch(error) { + const errorResponse = createJsonRpcErrorResponse(id, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); + this.send(errorResponse); + } + } + + /** + * Handles a DWN Server RPC Request via WebSockets. Currently only Subscription Messages are supported. + */ + private async processMessage(options: { + id: JsonRpcId, + target: string, + message: GenericMessage, + encodedData?: string; + }):Promise { + + const { id, target, message, encodedData } = options; + + // a subscription message requires a subscription handler + if (message.descriptor.method === DwnMethodName.Subscribe) { + const subscriptionHandler: MessageSubscriptionHandler = (message) => { + const response = createJsonRpcSuccessResponse(id, { reply: { + // status : { code: 200, detail: 'Accepted' }, + record : message + } }); + this.send(response); + } + + const { status, subscription } = await this.dwn.processMessage(target, message, { subscriptionHandler }); + if (status.code !== 200) { + const response = createJsonRpcSuccessResponse(id, { reply: { status }}) + return this.send(response); + } + + await this.subscriptions.subscribe(target, subscription); + const response = createJsonRpcSuccessResponse(id, { reply: { status, subscription: { id: subscription.id } } }); + return this.send(response); + } + + // Check whether data was provided in the request + const dataStream = encodedData ? DataStream.fromBytes(base64url.baseDecode(encodedData)) : undefined; + const dwnResponse = await this.dwn.processMessage(target, message, { dataStream }); + const response = createJsonRpcSuccessResponse(id, { reply: dwnResponse }); + this.send(response); + } +} + export class WsApi { #wsServer: WebSocketServer; + #connectionManager: Set = new Set(); dwn: Dwn; constructor(server: Server, dwn: Dwn) { @@ -40,84 +246,12 @@ export class WsApi { * Sets listeners for `message`, `pong`, `close`, and `error` events. */ #handleConnection(socket: WebSocket, _request: IncomingMessage): void { - const dwn = this.dwn; - - socket[SOCKET_ISALIVE_SYMBOL] = true; - - // Pong messages are automatically sent in response to ping messages as required by - // the websocket spec. So, no need to send explicit pongs from browser - socket.on('pong', function () { - this[SOCKET_ISALIVE_SYMBOL] = true; - }); - - socket.on('close', function () { - // Clean up event listeners - socket.removeAllListeners(); - }); - - socket.on('error', function (error) { - console.error('WebSocket error:', error); - // Close the socket and remove all event listeners - socket.terminate(); - socket.removeAllListeners(); - }); - - socket.on('message', async function (dataBuffer) { - let dwnRequest; - - try { - // deserialize bytes into JSON object - dwnRequest = dataBuffer.toString(); - if (!dwnRequest) { - const jsonRpcResponse = createJsonRpcErrorResponse( - uuidv4(), - JsonRpcErrorCodes.BadRequest, - 'request payload required.', - ); - - const responseBuffer = WsApi.jsonRpcResponseToBuffer(jsonRpcResponse); - return socket.send(responseBuffer); - } - - dwnRequest = JSON.parse(dwnRequest); - } catch (e) { - const jsonRpcResponse = createJsonRpcErrorResponse( - uuidv4(), - JsonRpcErrorCodes.BadRequest, - e.message, - ); - - const responseBuffer = WsApi.jsonRpcResponseToBuffer(jsonRpcResponse); - return socket.send(responseBuffer); - } - - // Check whether data was provided in the request - const { encodedData } = dwnRequest.params; - const requestDataStream = encodedData - ? DataStream.fromBytes(base64url.baseDecode(encodedData)) - : undefined; - - const requestContext: RequestContext = { - dwn, - transport: 'ws', - dataStream: requestDataStream, - }; - const { jsonRpcResponse } = await jsonRpcApi.handle( - dwnRequest, - requestContext, - ); - - if (jsonRpcResponse.error) { - requestCounter.inc({ method: dwnRequest.method, error: 1 }); - } else { - requestCounter.inc({ - method: dwnRequest.method, - status: jsonRpcResponse?.result?.reply?.status?.code || 0, - }); - } - - const responseBuffer = WsApi.jsonRpcResponseToBuffer(jsonRpcResponse); - return socket.send(responseBuffer); + const connection = new SocketConnection(socket, this.dwn); + this.#connectionManager.add(connection); + // attach to the socket's close handler to clean up this connection. + socket.on('close', () => { + // the connection internally already cleans itself up upon a socket close event, we just ned to remove it from our set. + this.#connectionManager.delete(connection); }); } @@ -132,13 +266,12 @@ export class WsApi { // if a pong hasn't received from a socket by the next ping, the server will terminate // the socket connection return setInterval(() => { - this.#wsServer.clients.forEach(function (socket) { - if (socket[SOCKET_ISALIVE_SYMBOL] === false) { - return socket.terminate(); + this.#connectionManager.forEach(async (connection) => { + if (connection.isAlive === false) { + return await connection.close(); } - socket[SOCKET_ISALIVE_SYMBOL] = false; - socket.ping(); + connection.ping(); }); }, HEARTBEAT_INTERVAL); } @@ -158,18 +291,17 @@ export class WsApi { }); } - start(callback?: () => void): WebSocketServer { + start(): WebSocketServer { this.#setupWebSocket(); - callback?.(); return this.#wsServer; } - close(): void { + async close(): Promise { this.#wsServer.close(); - } - - static jsonRpcResponseToBuffer(jsonRpcResponse: JsonRpcResponse): Buffer { - const str = JSON.stringify(jsonRpcResponse); - return Buffer.from(str); + const closeSubscriptions: Promise[] = []; + for (const subscription of this.#connectionManager) { + closeSubscriptions.push(subscription.close()); + } + await Promise.all(closeSubscriptions); } } From aaa6aa2304b810bff3f47de318b037508f9a634f Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Fri, 9 Feb 2024 20:31:31 -0500 Subject: [PATCH 02/40] json rpc cleanup --- src/json-rpc-api.ts | 2 + src/json-rpc-handlers/dwn/process-message.ts | 57 ++++++++++--- src/json-rpc-handlers/subscriptions/close.ts | 29 +++++++ src/json-rpc-handlers/subscriptions/index.ts | 1 + src/lib/json-rpc-router.ts | 7 +- src/ws-api.ts | 90 ++++++-------------- tests/ws-api.spec.ts | 10 +-- 7 files changed, 111 insertions(+), 85 deletions(-) create mode 100644 src/json-rpc-handlers/subscriptions/close.ts create mode 100644 src/json-rpc-handlers/subscriptions/index.ts diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index 41661e5..4bcedff 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -1,7 +1,9 @@ import { JsonRpcRouter } from './lib/json-rpc-router.js'; import { handleDwnProcessMessage } from './json-rpc-handlers/dwn/index.js'; +import { handleSubscriptionsClose } from './json-rpc-handlers/subscriptions/index.js'; export const jsonRpcApi = new JsonRpcRouter(); jsonRpcApi.on('dwn.processMessage', handleDwnProcessMessage); +jsonRpcApi.on('subscriptions.close', handleSubscriptionsClose); diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 09be116..3255a0b 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -1,12 +1,12 @@ -import type { RecordsReadReply } from '@tbd54566975/dwn-sdk-js'; +import { DwnInterfaceName, DwnMethodName, type GenericMessage } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; -import { v4 as uuidv4 } from 'uuid'; import type { HandlerResponse, JsonRpcHandler, } from '../../lib/json-rpc-router.js'; + import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, @@ -17,22 +17,55 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( dwnRequest, context, ) => { - const { dwn, dataStream } = context; - const { target, message } = dwnRequest.params; - const requestId = dwnRequest.id ?? uuidv4(); + const { dwn, dataStream, subscriptionHandler, subscriptionManager, transport } = context; + const { target, message } = dwnRequest.params as { target: string, message: GenericMessage }; + const requestId = dwnRequest.id ?? crypto.randomUUID(); try { - const reply = (await dwn.processMessage( - target, - message, - { dataStream: dataStream as IsomorphicReadable }, - )) as RecordsReadReply; + + // RecordsWrite is only supported on 'http' + if ( + transport !== 'http' && + message.descriptor.interface === DwnInterfaceName.Records && + message.descriptor.method === DwnMethodName.Write + ) { + const jsonRpcResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.MethodNotFound, + `RecordsWrite is not supported via ${context.transport}` + ) + return { jsonRpcResponse }; + } + + // Subscribe methods are only supported on WebSockets + if (transport !== 'ws' && message.descriptor.method === DwnMethodName.Subscribe) { + const jsonRpcResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.MethodNotFound, + `Subscribe not supported via ${context.transport}` + ) + return { jsonRpcResponse }; + } + + const reply = await dwn.processMessage(target, message, { + dataStream: dataStream as IsomorphicReadable, + subscriptionHandler, + }); + + const { record, subscription } = reply; // RecordsRead messages return record data as a stream to for accommodate large amounts of data let recordDataStream; - if (reply?.record?.data !== undefined) { + if (record !== undefined && record.data !== undefined) { recordDataStream = reply.record.data; - delete reply.record.data; + delete reply.record.data; // not serializable via JSON + } + + // Subscribe messages return a close function to facilitate closing the subscription + if (subscription !== undefined) { + const { id, close } = subscription; + subscriptionManager.subscribe(target, { id, close }); + delete reply.subscription.close // not serializable via JSON } const jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply }); diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts new file mode 100644 index 0000000..59e1755 --- /dev/null +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -0,0 +1,29 @@ +import type { + HandlerResponse, + JsonRpcHandler, +} from '../../lib/json-rpc-router.js'; + +import { + createJsonRpcErrorResponse, + createJsonRpcSuccessResponse, + JsonRpcErrorCodes, +} from '../../lib/json-rpc.js'; + +export const handleSubscriptionsClose: JsonRpcHandler = async ( + dwnRequest, + context, +) => { + const requestId = dwnRequest.id ?? crypto.randomUUID(); + const { subscriptionManager } = context; + const { target, subscriptionId } = dwnRequest.params as { target: string, subscriptionId: string }; + + let jsonRpcResponse; + try { + await subscriptionManager.close(target, subscriptionId); + jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply: { status: 200, detail: 'Accepted' } }); + } catch(error) { + jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); + } + + return { jsonRpcResponse } as HandlerResponse; +} \ No newline at end of file diff --git a/src/json-rpc-handlers/subscriptions/index.ts b/src/json-rpc-handlers/subscriptions/index.ts new file mode 100644 index 0000000..1225a0f --- /dev/null +++ b/src/json-rpc-handlers/subscriptions/index.ts @@ -0,0 +1 @@ +export * from './close.js'; diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 78036c7..3838786 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -1,13 +1,16 @@ -import type { Dwn } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, MessageSubscriptionHandler } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; import type { JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; +import type { SubscriptionManager } from '../ws-api.js'; export type RequestContext = { - dwn: Dwn; transport: 'http' | 'ws'; + dwn: Dwn; + subscriptionManager?: SubscriptionManager; dataStream?: Readable; + subscriptionHandler?: MessageSubscriptionHandler; }; export type HandlerResponse = { diff --git a/src/ws-api.ts b/src/ws-api.ts index e8d664d..6234d3a 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -1,7 +1,6 @@ import type { AddressInfo, WebSocket } from 'ws'; import type { IncomingMessage, Server } from 'http'; -import { base64url } from 'multiformats/bases/base64'; import log from 'loglevel'; import { WebSocketServer } from 'ws'; @@ -9,19 +8,21 @@ import type { Dwn, GenericMessage, MessageSubscription, - MessageSubscriptionHandler } from '@tbd54566975/dwn-sdk-js' -import { DataStream, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; +import { DwnMethodName } from '@tbd54566975/dwn-sdk-js'; +import type { RequestContext } from './lib/json-rpc-router.js'; import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './lib/json-rpc.js'; +import { jsonRpcApi } from './json-rpc-api.js'; import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, JsonRpcErrorCodes } from './lib/json-rpc.js'; + const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; @@ -122,7 +123,6 @@ export class SocketConnection { } private async message(dataBuffer: Buffer): Promise { - const requestData = dataBuffer.toString(); if (!requestData) { return this.send(createJsonRpcErrorResponse( @@ -145,82 +145,42 @@ export class SocketConnection { return this.send(errorResponse); }; - const { id, params, method } = jsonRequest; - const { target, message, subscriptionId, encodedData } = params; - - // DISCUSSION: Should this be a DWN message or is this rpc specific to the server? - // Having it as a DWN message feels like the incorrect approach as both `Subscribe` and a potential `Unsubscribe` are ephemeral. - // They do not propagate between DWNs in a way that the current DWN holding the Subscription would eventually get an Unsubscribe. - // The `Unsubscribe` is specifically targeted to the live subscription transport. Am open to other ideas on how to handle this. - - if (method === 'dwn.closeSubscription' && subscriptionId !== undefined) { - return await this.closeSubscription(id, target, subscriptionId); - } else if (method === 'dwn.processMessage' && target && message) { - return await this.processMessage({ id, target, message, encodedData }); - } else { - const errorResponse = createJsonRpcErrorResponse( - id, - JsonRpcErrorCodes.InvalidRequest, - `${method} is not supported.`, - ); - this.send(errorResponse); - } + const requestContext = await this.buildRequestContext(jsonRequest); + const { jsonRpcResponse } = await jsonRpcApi.handle(jsonRequest, requestContext); + this.send(jsonRpcResponse); } private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); } - private async closeSubscription(id: JsonRpcId, target: string, subscriptionId: string ): Promise { - try { - await this.subscriptions.close(target, subscriptionId); - const response = createJsonRpcSuccessResponse(id, { reply: { status: 200, detail: 'Accepted' } }); + private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { + return (message) => { + const response = createJsonRpcSuccessResponse(id, { reply: { + record : message + } }); this.send(response); - } catch(error) { - const errorResponse = createJsonRpcErrorResponse(id, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); - this.send(errorResponse); } } - /** - * Handles a DWN Server RPC Request via WebSockets. Currently only Subscription Messages are supported. - */ - private async processMessage(options: { - id: JsonRpcId, - target: string, - message: GenericMessage, - encodedData?: string; - }):Promise { - - const { id, target, message, encodedData } = options; - - // a subscription message requires a subscription handler - if (message.descriptor.method === DwnMethodName.Subscribe) { - const subscriptionHandler: MessageSubscriptionHandler = (message) => { - const response = createJsonRpcSuccessResponse(id, { reply: { - // status : { code: 200, detail: 'Accepted' }, - record : message - } }); - this.send(response); - } + private async buildRequestContext(request: JsonRpcRequest): Promise { + const { id, params, method} = request; + const requestContext: RequestContext = { + transport : 'ws', + dwn : this.dwn, + subscriptionManager : this.subscriptions, + } - const { status, subscription } = await this.dwn.processMessage(target, message, { subscriptionHandler }); - if (status.code !== 200) { - const response = createJsonRpcSuccessResponse(id, { reply: { status }}) - return this.send(response); + if (method === 'dwn.processMessage') { + const { message } = params; + if (message.descriptor.method === DwnMethodName.Subscribe) { + requestContext.subscriptionHandler = this.subscriptionHandler(id).bind(this); } - - await this.subscriptions.subscribe(target, subscription); - const response = createJsonRpcSuccessResponse(id, { reply: { status, subscription: { id: subscription.id } } }); - return this.send(response); } - // Check whether data was provided in the request - const dataStream = encodedData ? DataStream.fromBytes(base64url.baseDecode(encodedData)) : undefined; - const dwnResponse = await this.dwn.processMessage(target, message, { dataStream }); - const response = createJsonRpcSuccessResponse(id, { reply: dwnResponse }); - this.send(response); + return requestContext; } + } export class WsApi { diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index e2b338f..aa95810 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -52,7 +52,7 @@ describe('websocket api', function () { expect(resp.error.message).to.include('JSON'); }); - it('handles RecordsWrite messages', async function () { + it('RecordsWrite messages are not supported', async function () { const alice = await TestDataGenerator.generateDidKeyPersona(); const { recordsWrite, dataStream } = await createRecordsWriteMessage(alice); @@ -72,10 +72,8 @@ describe('websocket api', function () { ); const resp = JSON.parse(data.toString()); expect(resp.id).to.equal(requestId); - console.log(resp.error); - expect(resp.error).to.not.exist; - - const { reply } = resp.result; - expect(reply.status.code).to.equal(202); + expect(resp.error).to.not.be.undefined; + expect(resp.error.code).to.equal(JsonRpcErrorCodes.MethodNotFound); + expect(resp.error.message).to.include('RecordsWrite is not supported via ws'); }); }); From 6f46c0126903f0f2bb25f5918358c7d7af0011b5 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Sat, 10 Feb 2024 22:03:30 -0500 Subject: [PATCH 03/40] brought back requestCounter functionality for jsonrpc response/error --- src/ws-api.ts | 49 +++++++++++++++++++++++++++----------------- tests/ws-api.spec.ts | 11 +++++++--- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/src/ws-api.ts b/src/ws-api.ts index 6234d3a..f080600 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -21,6 +21,7 @@ import { createJsonRpcSuccessResponse, JsonRpcErrorCodes } from './lib/json-rpc.js'; +import { requestCounter } from './metrics.js'; const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); @@ -73,12 +74,11 @@ class Manager { } export class SocketConnection { - private id: string; - private subscriptions: SubscriptionManager; - - constructor(private socket: WebSocket, private dwn: Dwn){ - this.id = crypto.randomUUID(); - this.subscriptions = new Manager(); + constructor( + private socket: WebSocket, + private dwn: Dwn, + private subscriptions: SubscriptionManager = new Manager(), + ){ socket.on('close', this.close.bind(this)); socket.on('pong', this.pong.bind(this)); socket.on('error', this.error.bind(this)); @@ -116,7 +116,7 @@ export class SocketConnection { private async error(error?:Error): Promise{ if (error !== undefined) { - log.error('WebSocket', this.id, error); + log.error('WebSocket', this.socket.url, error); this.socket.terminate(); await this.close() } @@ -147,6 +147,15 @@ export class SocketConnection { const requestContext = await this.buildRequestContext(jsonRequest); const { jsonRpcResponse } = await jsonRpcApi.handle(jsonRequest, requestContext); + if (jsonRpcResponse.error) { + requestCounter.inc({ method: jsonRequest.method, error: 1 }); + } else { + requestCounter.inc({ + method: jsonRequest.method, + status: jsonRpcResponse?.result?.reply?.status?.code || 0, + }); + } + this.send(jsonRpcResponse); } @@ -172,7 +181,7 @@ export class SocketConnection { } if (method === 'dwn.processMessage') { - const { message } = params; + const { message } = params as { message: GenericMessage }; if (message.descriptor.method === DwnMethodName.Subscribe) { requestContext.subscriptionHandler = this.subscriptionHandler(id).bind(this); } @@ -180,14 +189,15 @@ export class SocketConnection { return requestContext; } - } export class WsApi { #wsServer: WebSocketServer; - #connectionManager: Set = new Set(); dwn: Dwn; + #heartbeatInterval: NodeJS.Timer | undefined; + #connections: Map = new Map(); + constructor(server: Server, dwn: Dwn) { this.dwn = dwn; this.#wsServer = new WebSocketServer({ server }); @@ -207,26 +217,28 @@ export class WsApi { */ #handleConnection(socket: WebSocket, _request: IncomingMessage): void { const connection = new SocketConnection(socket, this.dwn); - this.#connectionManager.add(connection); + this.#connections.set(socket, connection); // attach to the socket's close handler to clean up this connection. socket.on('close', () => { // the connection internally already cleans itself up upon a socket close event, we just ned to remove it from our set. - this.#connectionManager.delete(connection); + this.#connections.delete(socket); }); } - /** * This handler returns an interval to ping clients' socket every 30s * if a pong hasn't received from a socket by the next ping, the server will terminate the socket connection. */ #setupHeartbeat(): NodeJS.Timer { + if (this.#heartbeatInterval) { + return this.#heartbeatInterval; + } // Sometimes connections between client <-> server can get borked in such a way that // leaves both unaware of the borkage. ping messages can be used as a means to verify // that the remote endpoint is still responsive. Server will ping each socket every 30s // if a pong hasn't received from a socket by the next ping, the server will terminate // the socket connection - return setInterval(() => { - this.#connectionManager.forEach(async (connection) => { + this.#heartbeatInterval = setInterval(() => { + this.#connections.forEach(async (connection) => { if (connection.isAlive === false) { return await connection.close(); } @@ -258,10 +270,9 @@ export class WsApi { async close(): Promise { this.#wsServer.close(); - const closeSubscriptions: Promise[] = []; - for (const subscription of this.#connectionManager) { - closeSubscriptions.push(subscription.close()); + for (const [socket, connection] of this.#connections) { + this.#connections.delete(socket); + await connection.close() } - await Promise.all(closeSubscriptions); } } diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index aa95810..8977b70 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,3 +1,4 @@ +import type { Dwn} from '@tbd54566975/dwn-sdk-js'; import { DataStream, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; import { expect } from 'chai'; @@ -16,14 +17,15 @@ import { createRecordsWriteMessage, sendWsMessage } from './utils.js'; let server: http.Server; let wsServer: WebSocketServer; +let dwn: Dwn; -describe('websocket api', function () { +describe.only('websocket api', function () { before(async function () { server = http.createServer(); server.listen(9002, '127.0.0.1'); - const testDwn = await getTestDwn(); - const wsApi = new WsApi(server, testDwn); + dwn = await getTestDwn(); + const wsApi = new WsApi(server, dwn); wsServer = wsApi.start(); }); @@ -76,4 +78,7 @@ describe('websocket api', function () { expect(resp.error.code).to.equal(JsonRpcErrorCodes.MethodNotFound); expect(resp.error.message).to.include('RecordsWrite is not supported via ws'); }); + + xit('RecordsSubscribe keeps a subscription alive', async function() { + }); }); From 523558d8c795e609e5d7de9f0e0b4f5db2960823 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 10:48:47 -0500 Subject: [PATCH 04/40] add JSONRPCSocket class to handle requests and long-running subscriptions over sockets, added test client helpers for unit testing, added some initial unit tests --- src/http-api.ts | 4 +- src/json-rpc-socket.ts | 89 +++++++++++++++++++++++ tests/http-api.spec.ts | 2 +- tests/test-dwn.ts | 21 +++--- tests/utils.ts | 131 +++++++++++++++++++++++++++++++++- tests/ws-api.spec.ts | 158 ++++++++++++++++++++++++++++++++++++++--- 6 files changed, 381 insertions(+), 24 deletions(-) create mode 100644 src/json-rpc-socket.ts diff --git a/src/http-api.ts b/src/http-api.ts index 30972ae..93128be 100644 --- a/src/http-api.ts +++ b/src/http-api.ts @@ -30,9 +30,7 @@ export class HttpApi { registrationManager: RegistrationManager; dwn: Dwn; - constructor(config: DwnServerConfig, dwn: Dwn, registrationManager: RegistrationManager) { - console.log(config); - + constructor(config: DwnServerConfig, dwn: Dwn, registrationManager?: RegistrationManager) { this.#config = config; this.#api = express(); this.#server = http.createServer(this.#api); diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts new file mode 100644 index 0000000..40e2fb9 --- /dev/null +++ b/src/json-rpc-socket.ts @@ -0,0 +1,89 @@ +import { v4 as uuidv4 } from 'uuid'; +import WebSocket from 'ws'; + +import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; + +const CONNECT_TIMEOUT = 3_000; + +export class JSONRPCSocket { + private isOpen = false; + constructor(private socket: WebSocket) { + socket.onopen = this.open; + } + + static async connect(url: string): Promise { + + const onclose = ():void => { + console.log('json rpc close'); + }; + + const onerror = (event: any):void => { + console.log('json rpc error', event); + }; + + const socket = new WebSocket(url); + socket.onclose = onclose; + socket.onerror = onerror; + + return new Promise((resolve, reject) => { + socket.on('open', () => { + resolve(new JSONRPCSocket(socket)); + }); + + setTimeout(() => reject, CONNECT_TIMEOUT); + }); + } + + open(): void { + this.isOpen = true; + } + + close(): void { + this.isOpen = false; + this.socket.close(); + } + + /** + * Sends a JSON-RPC request through the socket. You must subscribe to a message listener separately to capture the response. + */ + send(request: JsonRpcRequest):void { + return this.socket.send(Buffer.from(JSON.stringify(request))); + } + + subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): { close: () => void } { + request.id ??= uuidv4(); + + const messageHandler = (event: { data: any }):void => { + const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; + if (jsonRpcResponse.id === request.id) { + return listener(jsonRpcResponse); + } + }; + + this.socket.addEventListener('message', messageHandler); + this.send(request); + + return { + close: ():void => { + this.socket.removeEventListener('message', messageHandler); + } + }; + } + + async request(request: JsonRpcRequest): Promise { + return new Promise((resolve) => { + request.id ??= uuidv4(); + + const handleResponse = (event: { data: any }):void => { + const jsonRpsResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; + if (jsonRpsResponse.id === request.id) { + this.socket.removeEventListener('message', handleResponse); + return resolve(jsonRpsResponse); + } + }; + + this.socket.addEventListener('message', handleResponse); + this.send(request); + }); + } +} \ No newline at end of file diff --git a/tests/http-api.spec.ts b/tests/http-api.spec.ts index bb9a22b..a64121e 100644 --- a/tests/http-api.spec.ts +++ b/tests/http-api.spec.ts @@ -62,7 +62,7 @@ describe('http api', function () { const proofOfWorkInitialMaximumAllowedHash = config.registrationProofOfWorkInitialMaxHash; registrationManager = await RegistrationManager.create({ registrationStoreUrl, termsOfServiceFilePath, proofOfWorkInitialMaximumAllowedHash }); - dwn = await getTestDwn(registrationManager); + dwn = await getTestDwn({ tenantGate: registrationManager }); httpApi = new HttpApi(config, dwn, registrationManager); diff --git a/tests/test-dwn.ts b/tests/test-dwn.ts index b286c68..713fa55 100644 --- a/tests/test-dwn.ts +++ b/tests/test-dwn.ts @@ -1,5 +1,5 @@ import type { TenantGate } from '@tbd54566975/dwn-sdk-js'; -import { Dwn } from '@tbd54566975/dwn-sdk-js'; +import { Dwn, EventEmitterStream } from '@tbd54566975/dwn-sdk-js'; import { DataStoreSql, EventLogSql, @@ -9,26 +9,29 @@ import { import { getDialectFromURI } from '../src/storage.js'; import { DidDht, DidIon, DidKey, DidResolver } from '@web5/dids'; -export async function getTestDwn( - tenantGate?: TenantGate -): Promise { +export async function getTestDwn(options: { + tenantGate?: TenantGate, + withEvents?: boolean, +} = {}): Promise { + const { tenantGate, withEvents = false } = options; + const db = getDialectFromURI(new URL('sqlite://')); + const dataStore = new DataStoreSql(db); + const eventLog = new EventLogSql(db); + const messageStore = new MessageStoreSql(db); + const eventStream = withEvents ? new EventEmitterStream() : undefined; // NOTE: no resolver cache used here to avoid locking LevelDB const didResolver = new DidResolver({ didResolvers : [DidDht, DidIon, DidKey], }); - const db = getDialectFromURI(new URL('sqlite://')); - const dataStore = new DataStoreSql(db); - const eventLog = new EventLogSql(db); - const messageStore = new MessageStoreSql(db); - let dwn: Dwn; try { dwn = await Dwn.create({ eventLog, dataStore, messageStore, + eventStream, tenantGate, didResolver }); diff --git a/tests/utils.ts b/tests/utils.ts index 3ded3c4..8e322e2 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -1,14 +1,20 @@ -import type { Persona } from '@tbd54566975/dwn-sdk-js'; +import type { GenericMessage, MessageSubscriptionHandler, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; import { Cid, DataStream, RecordsWrite } from '@tbd54566975/dwn-sdk-js'; import type { ReadStream } from 'node:fs'; import fs from 'node:fs'; import http from 'node:http'; import path from 'path'; +import { v4 as uuidv4 } from 'uuid'; +import fetch from 'node-fetch'; import type { Readable } from 'readable-stream'; import { fileURLToPath } from 'url'; import { WebSocket } from 'ws'; +import type { JsonRpcResponse, JsonRpcRequest } from '../src/lib/json-rpc.js'; +import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; +import { JSONRPCSocket } from '../src/json-rpc-socket.js'; + // __filename and __dirname are not defined in ES module scope const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -139,6 +145,65 @@ export function streamHttpRequest( }); } +export async function sendHttpMessage(options: { + url: string, + target: string, + message: GenericMessage, + data?: any, +}): Promise { + const { url, target, message, data } = options; + // First RecordsWrite that creates the record. + const requestId = uuidv4(); + const jsonRpcRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + target, + message, + }); + + const fetchOpts = { + method : 'POST', + headers : { + 'dwn-request': JSON.stringify(jsonRpcRequest) + } + }; + + if (data !== undefined) { + fetchOpts.headers['content-type'] = 'application/octet-stream'; + fetchOpts['body'] = data; + } + + const resp = await fetch(url, fetchOpts); + let dwnRpcResponse: JsonRpcResponse; + + // check to see if response is in header first. if it is, that means the response is a ReadableStream + let dataStream; + const { headers } = resp; + if (headers.has('dwn-response')) { + const jsonRpcResponse = JSON.parse(headers.get('dwn-response')) as JsonRpcResponse; + + if (jsonRpcResponse == null) { + throw new Error(`failed to parse json rpc response. dwn url: ${url}`); + } + + dataStream = resp.body; + dwnRpcResponse = jsonRpcResponse; + } else { + const responseBody = await resp.text(); + dwnRpcResponse = JSON.parse(responseBody); + } + + if (dwnRpcResponse.error) { + const { code, message } = dwnRpcResponse.error; + throw new Error(`(${code}) - ${message}`); + } + + const { reply } = dwnRpcResponse.result; + if (dataStream) { + reply['record']['data'] = dataStream; + } + + return reply as UnionMessageReply; +} + export async function sendWsMessage( address: string, message: any, @@ -156,3 +221,67 @@ export async function sendWsMessage( }; }); } + +const MAX_RESPONSE_TIMEOUT = 3_000; + +export async function subscriptionRequest( + url: string, + request: JsonRpcRequest, + messageHandler: MessageSubscriptionHandler +): Promise<{ status: any, subscription?: { id: string, close: () => Promise } }> { + let resolved: boolean = false; + const { params: { target } } = request; + const connection = await JSONRPCSocket.connect(url); + + const closeSubscription = async (id: string, target: string, connection: JSONRPCSocket): Promise => { + const requestId = crypto.randomUUID(); + const request = createJsonRpcRequest(requestId, 'subscriptions.close', { subscriptionId: id, target }); + return await connection.request(request); + } + + return new Promise<{ status: any, subscription?: { id: string, close: () => Promise } }>((resolve, reject) => { + const { close: subscriptionClose } = connection.subscribe(request, (response) => { + const { result, error } = response; + + // this is an error specific to the `JsonRpcRequest` requesting the subscription + if (error) { + reject(error); + return; + } + + // at this point the reply should be DwnRpcResponse + const { status, record, subscription } = result.reply; + if (record) { + messageHandler(record); + return; + } + + if (subscription) { + resolved = true; + + resolve({ + status, + subscription: { + ...subscription, + close: async (): Promise => { + subscriptionClose(); + const closeResponse = await closeSubscription(subscription.id, target, connection); + if (closeResponse.error?.message !== undefined) { + throw new Error(`unable to close subscription: ${closeResponse.error.message}`); + } + } + } + }) + } else { + resolve({ status }); + } + }); + + setTimeout(() => { + if (resolved) { + return; + }; + return reject('subscription request timeout'); + }, MAX_RESPONSE_TIMEOUT); + }); +} diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 8977b70..8b3761e 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,9 +1,9 @@ -import type { Dwn} from '@tbd54566975/dwn-sdk-js'; -import { DataStream, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; +import { DataStream, Message, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; import { expect } from 'chai'; import { base64url } from 'multiformats/bases/base64'; -import http from 'node:http'; +import type { Server } from 'http'; import { v4 as uuidv4 } from 'uuid'; import { type WebSocketServer } from 'ws'; @@ -11,20 +11,24 @@ import { createJsonRpcRequest, JsonRpcErrorCodes, } from '../src/lib/json-rpc.js'; +import { config } from '../src/config.js'; import { WsApi } from '../src/ws-api.js'; import { getTestDwn } from './test-dwn.js'; -import { createRecordsWriteMessage, sendWsMessage } from './utils.js'; +import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest } from './utils.js'; +import { HttpApi } from '../src/http-api.js'; -let server: http.Server; +let server: Server; let wsServer: WebSocketServer; let dwn: Dwn; -describe.only('websocket api', function () { +describe('websocket api', function () { before(async function () { - server = http.createServer(); - server.listen(9002, '127.0.0.1'); + dwn = await getTestDwn({ withEvents: true }); + + // set up http api for issuing writes within the tests + const httpApi = new HttpApi(config, dwn); + server = await httpApi.start(9002); - dwn = await getTestDwn(); const wsApi = new WsApi(server, dwn); wsServer = wsApi.start(); }); @@ -79,6 +83,140 @@ describe.only('websocket api', function () { expect(resp.error.message).to.include('RecordsWrite is not supported via ws'); }); - xit('RecordsSubscribe keeps a subscription alive', async function() { + it('subscribes to records and receives updates', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const { message } = await TestDataGenerator.generateRecordsSubscribe({ + author: alice, + filter: { + schema: 'foo/bar' + } + }); + + const records: string[] = []; + const subscriptionHandler = async (message: GenericMessage): Promise => { + records.push(await Message.getCid(message)); + }; + + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + message: message, + target: alice.did, + }); + + const response = await subscriptionRequest('ws://127.0.0.1:9002', dwnRequest, subscriptionHandler); + expect(response.status.code).to.equal(200); + expect(response.subscription).to.not.be.undefined; + + const write1Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult1 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write1Message.message, + data : write1Message.dataBytes, + }); + expect(writeResult1.status.code).to.equal(202); + + const write2Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult2 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write2Message.message, + data : write2Message.dataBytes, + }) + expect(writeResult2.status.code).to.equal(202); + + // close the subscription + await response.subscription.close(); + + await new Promise(resolve => setTimeout(resolve, 500)); // wait for records to be processed + expect(records).to.have.members([ + await Message.getCid(write1Message.message), + await Message.getCid(write2Message.message) + ]); + }); + + it('stops receiving updates when subscription is closed', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const { message } = await TestDataGenerator.generateRecordsSubscribe({ + author: alice, + filter: { + schema: 'foo/bar' + } + }); + + const records: string[] = []; + const subscriptionHandler = async (message: GenericMessage): Promise => { + records.push(await Message.getCid(message)); + }; + + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + message: message, + target: alice.did, + }); + const response = await subscriptionRequest('ws://127.0.0.1:9002', dwnRequest, subscriptionHandler); + expect(response.status.code).to.equal(200); + expect(response.subscription).to.not.be.undefined; + + const write1Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult1 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write1Message.message, + data : write1Message.dataBytes, + }); + expect(writeResult1.status.code).to.equal(202); + + // close the subscription after only 1 message + await response.subscription.close(); + + // write more messages that won't show up in the subscription + const write2Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult2 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write2Message.message, + data : write2Message.dataBytes, + }) + expect(writeResult2.status.code).to.equal(202); + + const write3Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult3 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write3Message.message, + data : write3Message.dataBytes, + }) + expect(writeResult3.status.code).to.equal(202); + + await new Promise(resolve => setTimeout(resolve, 500)); // wait for records to be processed + expect(records).to.have.members([ await Message.getCid(write1Message.message) ]); }); }); From 5b791764d23bc4f3edc67c6ac838aaa10e36faa4 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 14:05:16 -0500 Subject: [PATCH 05/40] handle connection close not found error explicitly, general clean up --- src/dwn-error.ts | 1 + src/json-rpc-handlers/dwn/process-message.ts | 11 ++++++----- src/json-rpc-handlers/subscriptions/close.ts | 14 ++++++++++++-- src/lib/json-rpc.ts | 10 ++++++---- src/ws-api.ts | 6 +++++- tests/ws-api.spec.ts | 2 +- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/dwn-error.ts b/src/dwn-error.ts index fa19159..0163a28 100644 --- a/src/dwn-error.ts +++ b/src/dwn-error.ts @@ -32,5 +32,6 @@ export enum DwnServerErrorCode { ProofOfWorkManagerInvalidResponseNonceFormat = 'ProofOfWorkManagerInvalidResponseNonceFormat', ProofOfWorkManagerResponseNonceReused = 'ProofOfWorkManagerResponseNonceReused', RegistrationManagerInvalidOrOutdatedTermsOfServiceHash = 'RegistrationManagerInvalidOrOutdatedTermsOfServiceHash', + SubscriptionManagerSubscriptionNotFound = 'SubscriptionManagerSubscriptionNotFound', TenantRegistrationOutdatedTermsOfService = 'TenantRegistrationOutdatedTermsOfService', } diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 3255a0b..dd01584 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -1,4 +1,5 @@ -import { DwnInterfaceName, DwnMethodName, type GenericMessage } from '@tbd54566975/dwn-sdk-js'; +import type { GenericMessage } from '@tbd54566975/dwn-sdk-js'; +import { DwnInterfaceName, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; @@ -31,17 +32,17 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( ) { const jsonRpcResponse = createJsonRpcErrorResponse( requestId, - JsonRpcErrorCodes.MethodNotFound, + JsonRpcErrorCodes.InvalidParams, `RecordsWrite is not supported via ${context.transport}` ) return { jsonRpcResponse }; } - // Subscribe methods are only supported on WebSockets + // Subscribe methods are only supported on 'ws' (WebSockets) if (transport !== 'ws' && message.descriptor.method === DwnMethodName.Subscribe) { const jsonRpcResponse = createJsonRpcErrorResponse( requestId, - JsonRpcErrorCodes.MethodNotFound, + JsonRpcErrorCodes.InvalidParams, `Subscribe not supported via ${context.transport}` ) return { jsonRpcResponse }; @@ -55,7 +56,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( const { record, subscription } = reply; // RecordsRead messages return record data as a stream to for accommodate large amounts of data - let recordDataStream; + let recordDataStream: IsomorphicReadable; if (record !== undefined && record.data !== undefined) { recordDataStream = reply.record.data; delete reply.record.data; // not serializable via JSON diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts index 59e1755..62b4f40 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -1,8 +1,10 @@ +import { DwnServerErrorCode } from '../../dwn-error.js'; import type { HandlerResponse, JsonRpcHandler, } from '../../lib/json-rpc-router.js'; +import type { JsonRpcResponse } from '../../lib/json-rpc.js'; import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, @@ -17,12 +19,20 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( const { subscriptionManager } = context; const { target, subscriptionId } = dwnRequest.params as { target: string, subscriptionId: string }; - let jsonRpcResponse; + let jsonRpcResponse:JsonRpcResponse; try { await subscriptionManager.close(target, subscriptionId); jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply: { status: 200, detail: 'Accepted' } }); } catch(error) { - jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); + if (error.code === DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound) { + jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); + } else { + jsonRpcResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.InternalError, + `unknown subscription close error for ${subscriptionId}: ${error.message}` + ); + } } return { jsonRpcResponse } as HandlerResponse; diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index dfd638a..fbffd99 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -23,10 +23,12 @@ export enum JsonRpcErrorCodes { InternalError = -32603, ParseError = -32700, - // App defined errors - BadRequest = -50400, // equivalent to HTTP Status 400 - Unauthorized = -50401, // equivalent to HTTP Status 401 - Forbidden = -50403, // equivalent to HTTP Status 403 + /** App defined error equivalent to HTTP Status 400 */ + BadRequest = -50400, + /** App defined error equivalent to HTTP Status 401 */ + Unauthorized = -50401, + /** App defined error equivalent to HTTP Status 403 */ + Forbidden = -50403, } export type JsonRpcResponse = JsonRpcSuccessResponse | JsonRpcErrorResponse; diff --git a/src/ws-api.ts b/src/ws-api.ts index f080600..d638da4 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -22,6 +22,7 @@ import { JsonRpcErrorCodes } from './lib/json-rpc.js'; import { requestCounter } from './metrics.js'; +import { DwnServerError, DwnServerErrorCode } from './dwn-error.js'; const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); @@ -56,7 +57,10 @@ class Manager { } // if it reached here no subscription to close - throw new Error('could not find subscription to close'); + throw new DwnServerError( + DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound, + `subscription ${id} was not found` + ) } async closeAll(): Promise { diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 8b3761e..b905c4b 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -79,7 +79,7 @@ describe('websocket api', function () { const resp = JSON.parse(data.toString()); expect(resp.id).to.equal(requestId); expect(resp.error).to.not.be.undefined; - expect(resp.error.code).to.equal(JsonRpcErrorCodes.MethodNotFound); + expect(resp.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); expect(resp.error.message).to.include('RecordsWrite is not supported via ws'); }); From 81f3ccd5c51b3870998d00c506e3aca1b78e96e4 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 14:09:58 -0500 Subject: [PATCH 06/40] replace crypto.randomUUID() --- src/json-rpc-handlers/dwn/process-message.ts | 3 ++- src/json-rpc-handlers/subscriptions/close.ts | 4 +++- src/ws-api.ts | 5 +++-- tests/utils.ts | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index dd01584..d46cbb4 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -2,6 +2,7 @@ import type { GenericMessage } from '@tbd54566975/dwn-sdk-js'; import { DwnInterfaceName, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; +import { v4 as uuidv4 } from 'uuid'; import type { HandlerResponse, @@ -20,7 +21,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( ) => { const { dwn, dataStream, subscriptionHandler, subscriptionManager, transport } = context; const { target, message } = dwnRequest.params as { target: string, message: GenericMessage }; - const requestId = dwnRequest.id ?? crypto.randomUUID(); + const requestId = dwnRequest.id ?? uuidv4(); try { diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts index 62b4f40..76862f8 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -1,3 +1,5 @@ +import { v4 as uuidv4 } from 'uuid'; + import { DwnServerErrorCode } from '../../dwn-error.js'; import type { HandlerResponse, @@ -15,7 +17,7 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( dwnRequest, context, ) => { - const requestId = dwnRequest.id ?? crypto.randomUUID(); + const requestId = dwnRequest.id ?? uuidv4(); const { subscriptionManager } = context; const { target, subscriptionId } = dwnRequest.params as { target: string, subscriptionId: string }; diff --git a/src/ws-api.ts b/src/ws-api.ts index d638da4..c41e41e 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -3,6 +3,7 @@ import type { IncomingMessage, Server } from 'http'; import log from 'loglevel'; import { WebSocketServer } from 'ws'; +import { v4 as uuidv4 } from 'uuid'; import type { Dwn, @@ -130,7 +131,7 @@ export class SocketConnection { const requestData = dataBuffer.toString(); if (!requestData) { return this.send(createJsonRpcErrorResponse( - crypto.randomUUID(), + uuidv4(), JsonRpcErrorCodes.BadRequest, 'request payload required.' )) @@ -141,7 +142,7 @@ export class SocketConnection { jsonRequest = JSON.parse(requestData); } catch(error) { const errorResponse = createJsonRpcErrorResponse( - crypto.randomUUID(), + uuidv4(), JsonRpcErrorCodes.BadRequest, (error as Error).message ); diff --git a/tests/utils.ts b/tests/utils.ts index 8e322e2..9bd625a 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -234,7 +234,7 @@ export async function subscriptionRequest( const connection = await JSONRPCSocket.connect(url); const closeSubscription = async (id: string, target: string, connection: JSONRPCSocket): Promise => { - const requestId = crypto.randomUUID(); + const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'subscriptions.close', { subscriptionId: id, target }); return await connection.request(request); } From 1dd2943b5b67e5c55be5478fa321f3b422ad7231 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 14:26:37 -0500 Subject: [PATCH 07/40] clean up JSON RPC Socket client --- src/json-rpc-socket.ts | 78 ++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 40e2fb9..c96f40d 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -3,15 +3,23 @@ import WebSocket from 'ws'; import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; +// These were arbitrarily chosen, but can be modified via connect options const CONNECT_TIMEOUT = 3_000; +const RESPONSE_TIMEOUT = 30_000; +export type JSONRPCSocketOptions = { + connectTimeout?: number; + responseTimeout?: number; +} + +/** + * JSONRPC Socket Client for WebSocket request/response and long-running subscriptions + */ export class JSONRPCSocket { - private isOpen = false; - constructor(private socket: WebSocket) { - socket.onopen = this.open; - } + private constructor(private socket: WebSocket, private responseTimeout: number) {} - static async connect(url: string): Promise { + static async connect(url: string, options: JSONRPCSocketOptions = {}): Promise { + const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT } = options; const onclose = ():void => { console.log('json rpc close'); @@ -27,63 +35,73 @@ export class JSONRPCSocket { return new Promise((resolve, reject) => { socket.on('open', () => { - resolve(new JSONRPCSocket(socket)); + resolve(new JSONRPCSocket(socket, responseTimeout)); }); - setTimeout(() => reject, CONNECT_TIMEOUT); + setTimeout(() => reject, connectTimeout); }); } - open(): void { - this.isOpen = true; - } - close(): void { - this.isOpen = false; this.socket.close(); } /** - * Sends a JSON-RPC request through the socket. You must subscribe to a message listener separately to capture the response. + * Sends a JSON-RPC request through the socket and waits for a single response. */ - send(request: JsonRpcRequest):void { - return this.socket.send(Buffer.from(JSON.stringify(request))); + async request(request: JsonRpcRequest): Promise { + return new Promise((resolve, reject) => { + request.id ??= uuidv4(); + + const handleResponse = (event: { data: any }):void => { + const jsonRpsResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; + if (jsonRpsResponse.id === request.id) { + // if the incoming response id matches the request id, we will remove the listener and resolve the response + this.socket.removeEventListener('message', handleResponse); + return resolve(jsonRpsResponse); + } + }; + + // subscribe to the listener before sending the request + this.socket.addEventListener('message', handleResponse); + this.send(request); + + // reject this promise if we don't receive any response back within the timeout period + setTimeout(reject, this.responseTimeout); + }); } + /** + * Sends a JSON-RPC request through the socket and keeps a listener open to read associated responses as they arrive. + * Returns a close method to clean up the listener. + */ subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): { close: () => void } { request.id ??= uuidv4(); const messageHandler = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; if (jsonRpcResponse.id === request.id) { + // if the incoming response id matches the request id, trigger the listener return listener(jsonRpcResponse); } }; + // subscribe to the listener before sending the request this.socket.addEventListener('message', messageHandler); this.send(request); return { close: ():void => { + // removes the listener for this particular request this.socket.removeEventListener('message', messageHandler); } }; } - async request(request: JsonRpcRequest): Promise { - return new Promise((resolve) => { - request.id ??= uuidv4(); - - const handleResponse = (event: { data: any }):void => { - const jsonRpsResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; - if (jsonRpsResponse.id === request.id) { - this.socket.removeEventListener('message', handleResponse); - return resolve(jsonRpsResponse); - } - }; - - this.socket.addEventListener('message', handleResponse); - this.send(request); - }); + /** + * Sends a JSON-RPC request through the socket. You must subscribe to a message listener separately to capture the response. + */ + send(request: JsonRpcRequest):void { + return this.socket.send(Buffer.from(JSON.stringify(request))); } } \ No newline at end of file From 919df3b55ac55db5ca4bebaf2f69bb1ca8ef91ab Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 14:52:12 -0500 Subject: [PATCH 08/40] optional tenant gate and event stream in options --- src/dwn-server.ts | 18 ++++++++++++++---- src/storage.ts | 11 +++++++---- src/ws-api.ts | 3 ++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/dwn-server.ts b/src/dwn-server.ts index 6ba27e4..b2502e9 100644 --- a/src/dwn-server.ts +++ b/src/dwn-server.ts @@ -1,4 +1,5 @@ -import { Dwn } from '@tbd54566975/dwn-sdk-js'; +import type { EventStream } from '@tbd54566975/dwn-sdk-js'; +import { Dwn, EventEmitterStream } from '@tbd54566975/dwn-sdk-js'; import type { Server } from 'http'; import log from 'loglevel'; @@ -61,7 +62,17 @@ export class DwnServer { proofOfWorkInitialMaximumAllowedHash: this.config.registrationProofOfWorkInitialMaxHash, }); - this.dwn = await Dwn.create(getDWNConfig(this.config, registrationManager)); + let eventStream: EventStream | undefined; + if (this.config.webSocketServerEnabled) { + // setting `EventEmitterStream` as default the default `EventStream + // if an alternate implementation is needed instantiate a `Dwn` with a custom `EventStream` and add it to server options. + eventStream = new EventEmitterStream(); + } + + this.dwn = await Dwn.create(getDWNConfig(this.config, { + tenantGate: registrationManager, + eventStream, + })); } this.#httpApi = new HttpApi(this.config, this.dwn, registrationManager); @@ -76,8 +87,7 @@ export class DwnServer { if (this.config.webSocketServerEnabled) { this.#wsApi = new WsApi(this.#httpApi.server, this.dwn); - this.#wsApi.start(); - log.info('WebSocketServer ready...'); + this.#wsApi.start(() => log.info('WebSocketServer ready...')); } } diff --git a/src/storage.ts b/src/storage.ts index 23b8e05..a4eea8c 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -2,7 +2,6 @@ import * as fs from 'fs'; import { DataStoreLevel, - EventEmitterStream, EventLogLevel, MessageStoreLevel, } from '@tbd54566975/dwn-sdk-js'; @@ -10,6 +9,7 @@ import type { DataStore, DwnConfig, EventLog, + EventStream, MessageStore, TenantGate, } from '@tbd54566975/dwn-sdk-js'; @@ -46,9 +46,13 @@ export enum BackendTypes { export type StoreType = DataStore | EventLog | MessageStore; export function getDWNConfig( - config: DwnServerConfig, - tenantGate: TenantGate, + config : DwnServerConfig, + options : { + tenantGate? : TenantGate, + eventStream? : EventStream, + } ): DwnConfig { + const { tenantGate, eventStream } = options; const dataStore: DataStore = getStore(config.dataStore, EStoreType.DataStore); const eventLog: EventLog = getStore(config.eventLog, EStoreType.EventLog); const messageStore: MessageStore = getStore( @@ -56,7 +60,6 @@ export function getDWNConfig( EStoreType.MessageStore, ); - const eventStream = config.webSocketServerEnabled ? new EventEmitterStream() : undefined; return { eventStream, eventLog, dataStore, messageStore, tenantGate }; } diff --git a/src/ws-api.ts b/src/ws-api.ts index c41e41e..ab0cef1 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -268,8 +268,9 @@ export class WsApi { }); } - start(): WebSocketServer { + start(callback?: () => void): WebSocketServer { this.#setupWebSocket(); + callback?.(); return this.#wsServer; } From eac8d3b78e55a1ead99eb68478b8d45b17f0e99b Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 17:58:20 -0500 Subject: [PATCH 09/40] refactor into separate files, account for null errors, use loglevel --- src/config.ts | 2 +- src/connection/connection-manager.ts | 35 ++++ src/connection/socket-connection.ts | 143 +++++++++++++++ src/json-rpc-socket.ts | 5 +- src/lib/json-rpc-router.ts | 2 +- src/subscription-manager.ts | 55 ++++++ src/ws-api.ts | 255 ++------------------------- tests/utils.ts | 4 +- tests/ws-api.spec.ts | 4 +- 9 files changed, 253 insertions(+), 252 deletions(-) create mode 100644 src/connection/connection-manager.ts create mode 100644 src/connection/socket-connection.ts create mode 100644 src/subscription-manager.ts diff --git a/src/config.ts b/src/config.ts index adc5de3..be83277 100644 --- a/src/config.ts +++ b/src/config.ts @@ -8,7 +8,7 @@ export const config = { // port that server listens on port: parseInt(process.env.DS_PORT || '3000'), // whether to enable 'ws:' - webSocketServerEnabled: { on: true, off: false }[process.env.DS_WEBSOCKET_SERVER] ?? true, + webSocketServerEnabled: { on: true, off: false }[process.env.DWN_WEBSOCKET_SERVER] ?? true, // where to store persistent data messageStore: process.env.DWN_STORAGE_MESSAGES || process.env.DWN_STORAGE || 'level://data', dataStore: process.env.DWN_STORAGE_DATA || process.env.DWN_STORAGE || 'level://data', diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts new file mode 100644 index 0000000..6f3388a --- /dev/null +++ b/src/connection/connection-manager.ts @@ -0,0 +1,35 @@ +import type { Dwn } from "@tbd54566975/dwn-sdk-js"; + +import type { WebSocket } from 'ws'; + +import { SocketConnection } from "./socket-connection.js"; +import { InMemorySubscriptionManager } from "../subscription-manager.js"; + +export interface ConnectionManager { + connect(socket: WebSocket): Promise; + close(): Promise +} + +export class InMemoryConnectionManager implements ConnectionManager { + constructor(private dwn: Dwn, private connections: Map = new Map()) {} + + /** + * Handler for opening websocket event - `connection`. + * Sets listeners for `message`, `pong`, `close`, and `error` events. + */ + async connect(socket: WebSocket): Promise { + const connection = new SocketConnection(socket, this.dwn, new InMemorySubscriptionManager()); + this.connections.set(socket, connection); + // attach to the socket's close handler to clean up this connection. + socket.on('close', () => { + // the connection internally already cleans itself up upon a socket close event, we just ned to remove it from our set. + this.connections.delete(socket); + }); + } + + async close(): Promise { + const closePromises = []; + this.connections.forEach(connection => closePromises.push(connection.close())); + await Promise.all(closePromises); + } +} \ No newline at end of file diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts new file mode 100644 index 0000000..fabcab1 --- /dev/null +++ b/src/connection/socket-connection.ts @@ -0,0 +1,143 @@ +import type { Dwn, GenericMessage } from "@tbd54566975/dwn-sdk-js"; +import { DwnMethodName } from "@tbd54566975/dwn-sdk-js"; + +import log from 'loglevel'; +import { v4 as uuidv4 } from 'uuid'; +import type { WebSocket } from "ws"; + +import type { RequestContext } from "../lib/json-rpc-router.js"; +import type { SubscriptionManager } from "../subscription-manager.js"; +import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from "../lib/json-rpc.js"; + +import { requestCounter } from "../metrics.js"; +import { jsonRpcApi } from "../json-rpc-api.js"; +import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcSuccessResponse } from "../lib/json-rpc.js"; + +const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); +const HEARTBEAT_INTERVAL = 30_000; + +export class SocketConnection { + private heartbeatInterval: NodeJS.Timer; + constructor( + private socket: WebSocket, + private dwn: Dwn, + private subscriptions: SubscriptionManager + ){ + socket.on('close', this.close.bind(this)); + socket.on('pong', this.pong.bind(this)); + socket.on('error', this.error.bind(this)); + socket.on('message', this.message.bind(this)); + + // Sometimes connections between client <-> server can get borked in such a way that + // leaves both unaware of the borkage. ping messages can be used as a means to verify + // that the remote endpoint is still responsive. Server will ping each socket every 30s + // if a pong hasn't received from a socket by the next ping, the server will terminate + // the socket connection + socket[SOCKET_ISALIVE_SYMBOL] = true; + this.heartbeatInterval = setInterval(() => { + if (this.socket[SOCKET_ISALIVE_SYMBOL] === false) { + this.close(); + } + this.socket[SOCKET_ISALIVE_SYMBOL] = false; + this.socket.ping(); + }, HEARTBEAT_INTERVAL); + } + + /** + * Closes the existing connection and cleans up any listeners or subscriptions. + */ + async close(): Promise { + clearInterval(this.heartbeatInterval); + // clean up all socket event listeners + this.socket.removeAllListeners(); + + // close all of the associated subscriptions + await this.subscriptions.closeAll(); + + // close the socket. + this.socket.close(); + } + + /** + * Pong messages are automatically sent in response to ping messages as required by + * the websocket spec. So, no need to send explicit pongs. + */ + private pong(): void { + this.socket[SOCKET_ISALIVE_SYMBOL] = true; + } + + private async error(error?:Error): Promise{ + if (error) { + log.error(`SocketConnection error, terminating connection`, error); + this.socket.terminate(); + await this.close() + } + } + + private async message(dataBuffer: Buffer): Promise { + const requestData = dataBuffer.toString(); + if (!requestData) { + return this.send(createJsonRpcErrorResponse( + uuidv4(), + JsonRpcErrorCodes.BadRequest, + 'request payload required.' + )) + } + + let jsonRequest: JsonRpcRequest; + try { + jsonRequest = JSON.parse(requestData); + } catch(error) { + const errorResponse = createJsonRpcErrorResponse( + uuidv4(), + JsonRpcErrorCodes.BadRequest, + (error as Error).message + ); + + return this.send(errorResponse); + }; + + const requestContext = await this.buildRequestContext(jsonRequest); + const { jsonRpcResponse } = await jsonRpcApi.handle(jsonRequest, requestContext); + if (jsonRpcResponse.error) { + requestCounter.inc({ method: jsonRequest.method, error: 1 }); + } else { + requestCounter.inc({ + method: jsonRequest.method, + status: jsonRpcResponse?.result?.reply?.status?.code || 0, + }); + } + this.send(jsonRpcResponse); + } + + private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { + this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); + } + + private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { + return (message) => { + const response = createJsonRpcSuccessResponse(id, { reply: { + record : message + } }); + this.send(response); + } + } + + private async buildRequestContext(request: JsonRpcRequest): Promise { + const { id, params, method} = request; + const requestContext: RequestContext = { + transport : 'ws', + dwn : this.dwn, + subscriptionManager : this.subscriptions, + } + + if (method === 'dwn.processMessage') { + const { message } = params as { message: GenericMessage }; + if (message.descriptor.method === DwnMethodName.Subscribe) { + requestContext.subscriptionHandler = this.subscriptionHandler(id).bind(this); + } + } + + return requestContext; + } +} diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index c96f40d..0622b37 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -1,3 +1,4 @@ +import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; import WebSocket from 'ws'; @@ -22,11 +23,11 @@ export class JSONRPCSocket { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT } = options; const onclose = ():void => { - console.log('json rpc close'); + log.info(`JSON RPC Socket close ${url}`); }; const onerror = (event: any):void => { - console.log('json rpc error', event); + log.error(`JSON RPC Socket error ${url}`, event); }; const socket = new WebSocket(url); diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 3838786..f92368c 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -2,8 +2,8 @@ import type { Dwn, MessageSubscriptionHandler } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; +import type { SubscriptionManager } from '../subscription-manager.js'; import type { JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; -import type { SubscriptionManager } from '../ws-api.js'; export type RequestContext = { transport: 'http' | 'ws'; diff --git a/src/subscription-manager.ts b/src/subscription-manager.ts new file mode 100644 index 0000000..de87dec --- /dev/null +++ b/src/subscription-manager.ts @@ -0,0 +1,55 @@ +import type { MessageSubscription } from "@tbd54566975/dwn-sdk-js"; + +import { DwnServerError, DwnServerErrorCode } from "./dwn-error.js"; + +/** + * SubscriptionManager manages the subscriptions related to a `SocketConnection` + */ +export interface SubscriptionManager { + subscribe: (target: string, subscription: MessageSubscription) => Promise; + close: (target: string, id: string) => Promise; + closeAll: () => Promise; +} + +export class InMemorySubscriptionManager implements SubscriptionManager { + constructor(private subscriptions: Map> = new Map()){}; + async subscribe(target: string, subscription: MessageSubscription): Promise { + let targetSubscriptions = this.subscriptions.get(target); + if (targetSubscriptions === undefined) { + targetSubscriptions = new Map(); + this.subscriptions.set(target, targetSubscriptions); + } + targetSubscriptions.set(subscription.id, subscription); + } + + async close(target: string, id: string): Promise { + const targetSubscriptions = this.subscriptions.get(target); + if (targetSubscriptions !== undefined) { + const subscription = targetSubscriptions.get(id); + if (subscription !== undefined) { + targetSubscriptions.delete(id); + await subscription.close(); + return; + } + } + + // if it reached here no subscription to close + throw new DwnServerError( + DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound, + `subscription ${id} was not found` + ) + } + + async closeAll(): Promise { + const closePromises = []; + for (const [target, subscriptions] of this.subscriptions) { + this.subscriptions.delete(target); + for (const [id, subscription] of subscriptions) { + subscriptions.delete(id); + closePromises.push(subscription.close()); + } + } + + await Promise.all(closePromises); + } +} \ No newline at end of file diff --git a/src/ws-api.ts b/src/ws-api.ts index ab0cef1..25bcc8b 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -1,210 +1,24 @@ -import type { AddressInfo, WebSocket } from 'ws'; -import type { IncomingMessage, Server } from 'http'; - -import log from 'loglevel'; -import { WebSocketServer } from 'ws'; -import { v4 as uuidv4 } from 'uuid'; import type { Dwn, - GenericMessage, - MessageSubscription, -} from '@tbd54566975/dwn-sdk-js' - -import { DwnMethodName } from '@tbd54566975/dwn-sdk-js'; - -import type { RequestContext } from './lib/json-rpc-router.js'; -import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './lib/json-rpc.js'; - -import { jsonRpcApi } from './json-rpc-api.js'; -import { - createJsonRpcErrorResponse, - createJsonRpcSuccessResponse, - JsonRpcErrorCodes -} from './lib/json-rpc.js'; -import { requestCounter } from './metrics.js'; -import { DwnServerError, DwnServerErrorCode } from './dwn-error.js'; - - -const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); -const HEARTBEAT_INTERVAL = 30_000; - -export interface SubscriptionManager { - subscribe: (target: string, subscription: MessageSubscription) => Promise; - close: (target: string, id: string) => Promise; - closeAll: () => Promise; -} - -class Manager { - constructor(private subscriptions: Map> = new Map()){}; - async subscribe(target: string, subscription: MessageSubscription): Promise { - let targetSubscriptions = this.subscriptions.get(target); - if (targetSubscriptions === undefined) { - targetSubscriptions = new Map(); - this.subscriptions.set(target, targetSubscriptions); - } - targetSubscriptions.set(subscription.id, subscription); - } - - async close(target: string, id: string): Promise { - const targetSubscriptions = this.subscriptions.get(target); - if (targetSubscriptions !== undefined) { - const subscription = targetSubscriptions.get(id); - if (subscription !== undefined) { - targetSubscriptions.delete(id); - await subscription.close(); - return; - } - } - - // if it reached here no subscription to close - throw new DwnServerError( - DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound, - `subscription ${id} was not found` - ) - } - - async closeAll(): Promise { - const closePromises = []; - for (const [target, subscriptions] of this.subscriptions) { - this.subscriptions.delete(target); - for (const [id, subscription] of subscriptions) { - subscriptions.delete(id); - closePromises.push(subscription.close()); - } - } - - await Promise.all(closePromises); - } -} - -export class SocketConnection { - constructor( - private socket: WebSocket, - private dwn: Dwn, - private subscriptions: SubscriptionManager = new Manager(), - ){ - socket.on('close', this.close.bind(this)); - socket.on('pong', this.pong.bind(this)); - socket.on('error', this.error.bind(this)); - socket.on('message', this.message.bind(this)); - socket[SOCKET_ISALIVE_SYMBOL] = true; - } - - get isAlive(): boolean { - return this.socket[SOCKET_ISALIVE_SYMBOL]; - } - - /** - * Closes the existing connection and cleans up any listeners or subscriptions. - */ - async close(): Promise { - // clean up all socket event listeners - this.socket.removeAllListeners(); - - // close all of the associated subscriptions - await this.subscriptions.closeAll(); - } - - ping(): void { - this.socket[SOCKET_ISALIVE_SYMBOL] = false; - this.socket.ping(); - } +} from '@tbd54566975/dwn-sdk-js'; - /** - * Pong messages are automatically sent in response to ping messages as required by - * the websocket spec. So, no need to send explicit pongs from browser - */ - private pong(): void { - this.socket[SOCKET_ISALIVE_SYMBOL] = true; - } - - private async error(error?:Error): Promise{ - if (error !== undefined) { - log.error('WebSocket', this.socket.url, error); - this.socket.terminate(); - await this.close() - } - } - - private async message(dataBuffer: Buffer): Promise { - const requestData = dataBuffer.toString(); - if (!requestData) { - return this.send(createJsonRpcErrorResponse( - uuidv4(), - JsonRpcErrorCodes.BadRequest, - 'request payload required.' - )) - } - - let jsonRequest: JsonRpcRequest; - try { - jsonRequest = JSON.parse(requestData); - } catch(error) { - const errorResponse = createJsonRpcErrorResponse( - uuidv4(), - JsonRpcErrorCodes.BadRequest, - (error as Error).message - ); - - return this.send(errorResponse); - }; - - const requestContext = await this.buildRequestContext(jsonRequest); - const { jsonRpcResponse } = await jsonRpcApi.handle(jsonRequest, requestContext); - if (jsonRpcResponse.error) { - requestCounter.inc({ method: jsonRequest.method, error: 1 }); - } else { - requestCounter.inc({ - method: jsonRequest.method, - status: jsonRpcResponse?.result?.reply?.status?.code || 0, - }); - } - - this.send(jsonRpcResponse); - } - - private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { - this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); - } - - private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { - return (message) => { - const response = createJsonRpcSuccessResponse(id, { reply: { - record : message - } }); - this.send(response); - } - } +import type { AddressInfo } from 'ws'; +import type { Server } from 'http'; - private async buildRequestContext(request: JsonRpcRequest): Promise { - const { id, params, method} = request; - const requestContext: RequestContext = { - transport : 'ws', - dwn : this.dwn, - subscriptionManager : this.subscriptions, - } - - if (method === 'dwn.processMessage') { - const { message } = params as { message: GenericMessage }; - if (message.descriptor.method === DwnMethodName.Subscribe) { - requestContext.subscriptionHandler = this.subscriptionHandler(id).bind(this); - } - } +import { WebSocketServer } from 'ws'; - return requestContext; - } -} +import type { ConnectionManager } from './connection/connection-manager.js'; +import { InMemoryConnectionManager } from './connection/connection-manager.js'; export class WsApi { #wsServer: WebSocketServer; dwn: Dwn; + #connections: ConnectionManager - #heartbeatInterval: NodeJS.Timer | undefined; - #connections: Map = new Map(); - - constructor(server: Server, dwn: Dwn) { + constructor(server: Server, dwn: Dwn, connectionManager?: ConnectionManager) { this.dwn = dwn; + this.#connections = connectionManager || new InMemoryConnectionManager(dwn); this.#wsServer = new WebSocketServer({ server }); } @@ -216,56 +30,14 @@ export class WsApi { return this.#wsServer; } - /** - * Handler for opening websocket event - `connection`. - * Sets listeners for `message`, `pong`, `close`, and `error` events. - */ - #handleConnection(socket: WebSocket, _request: IncomingMessage): void { - const connection = new SocketConnection(socket, this.dwn); - this.#connections.set(socket, connection); - // attach to the socket's close handler to clean up this connection. - socket.on('close', () => { - // the connection internally already cleans itself up upon a socket close event, we just ned to remove it from our set. - this.#connections.delete(socket); - }); - } - /** - * This handler returns an interval to ping clients' socket every 30s - * if a pong hasn't received from a socket by the next ping, the server will terminate the socket connection. - */ - #setupHeartbeat(): NodeJS.Timer { - if (this.#heartbeatInterval) { - return this.#heartbeatInterval; - } - // Sometimes connections between client <-> server can get borked in such a way that - // leaves both unaware of the borkage. ping messages can be used as a means to verify - // that the remote endpoint is still responsive. Server will ping each socket every 30s - // if a pong hasn't received from a socket by the next ping, the server will terminate - // the socket connection - this.#heartbeatInterval = setInterval(() => { - this.#connections.forEach(async (connection) => { - if (connection.isAlive === false) { - return await connection.close(); - } - - connection.ping(); - }); - }, HEARTBEAT_INTERVAL); - } - /** * Handler for starting a WebSocket. * Sets listeners for `connection`, `close` events. * It clears `heartbeatInterval` when a `close` event is made. */ #setupWebSocket(): void { - this.#wsServer.on('connection', this.#handleConnection.bind(this)); - - const heartbeatInterval = this.#setupHeartbeat(); - - this.#wsServer.on('close', function close() { - clearInterval(heartbeatInterval); - }); + this.#wsServer.on('connection', this.#connections.connect.bind(this)); + this.#wsServer.on('close', this.#connections.close.bind(this)); } start(callback?: () => void): WebSocketServer { @@ -276,9 +48,6 @@ export class WsApi { async close(): Promise { this.#wsServer.close(); - for (const [socket, connection] of this.#connections) { - this.#connections.delete(socket); - await connection.close() - } + await this.#connections.close(); } } diff --git a/tests/utils.ts b/tests/utils.ts index 9bd625a..c05021d 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -222,7 +222,7 @@ export async function sendWsMessage( }); } -const MAX_RESPONSE_TIMEOUT = 3_000; +const MAX_RESPONSE_TIMEOUT = 1_500; export async function subscriptionRequest( url: string, @@ -255,10 +255,8 @@ export async function subscriptionRequest( messageHandler(record); return; } - if (subscription) { resolved = true; - resolve({ status, subscription: { diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index b905c4b..74661f2 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -139,7 +139,7 @@ describe('websocket api', function () { // close the subscription await response.subscription.close(); - await new Promise(resolve => setTimeout(resolve, 500)); // wait for records to be processed + await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed expect(records).to.have.members([ await Message.getCid(write1Message.message), await Message.getCid(write2Message.message) @@ -216,7 +216,7 @@ describe('websocket api', function () { }) expect(writeResult3.status.code).to.equal(202); - await new Promise(resolve => setTimeout(resolve, 500)); // wait for records to be processed + await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed expect(records).to.have.members([ await Message.getCid(write1Message.message) ]); }); }); From 0669f04518d0f8301d6736edf37e9920b65e3db6 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 12 Feb 2024 18:04:37 -0500 Subject: [PATCH 10/40] uncesseary chagnge --- src/config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.ts b/src/config.ts index be83277..adc5de3 100644 --- a/src/config.ts +++ b/src/config.ts @@ -8,7 +8,7 @@ export const config = { // port that server listens on port: parseInt(process.env.DS_PORT || '3000'), // whether to enable 'ws:' - webSocketServerEnabled: { on: true, off: false }[process.env.DWN_WEBSOCKET_SERVER] ?? true, + webSocketServerEnabled: { on: true, off: false }[process.env.DS_WEBSOCKET_SERVER] ?? true, // where to store persistent data messageStore: process.env.DWN_STORAGE_MESSAGES || process.env.DWN_STORAGE || 'level://data', dataStore: process.env.DWN_STORAGE_DATA || process.env.DWN_STORAGE || 'level://data', From f2b71d30dd40b273723555163cfe69831b8ed3ca Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 11:12:18 -0500 Subject: [PATCH 11/40] update comments --- src/connection/connection-manager.ts | 31 ++++++++++++++++++-- src/connection/socket-connection.ts | 20 ++++++++++++- src/http-api.ts | 2 ++ src/json-rpc-handlers/subscriptions/close.ts | 6 ++++ src/lib/json-rpc-router.ts | 5 +++- src/subscription-manager.ts | 5 ++++ src/ws-api.ts | 2 +- 7 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts index 6f3388a..931315d 100644 --- a/src/connection/connection-manager.ts +++ b/src/connection/connection-manager.ts @@ -1,15 +1,28 @@ import type { Dwn } from "@tbd54566975/dwn-sdk-js"; +import type { IncomingMessage } from "http"; import type { WebSocket } from 'ws'; import { SocketConnection } from "./socket-connection.js"; import { InMemorySubscriptionManager } from "../subscription-manager.js"; +/** + * Interface for managing `WebSocket` connections as they arrive. + */ export interface ConnectionManager { - connect(socket: WebSocket): Promise; - close(): Promise + /** connect handler used for the `WebSockets` `'connection'` event. */ + connect(socket: WebSocket, request?: IncomingMessage): Promise; + /** cleans up handlers associated with the `WebSocket` connection */ + close(socket:WebSocket): Promise; + /** closes all of the connections */ + closeAll(): Promise } +/** + * A Simple In Memory ConnectionManager implementation. + * It uses a `Map` to manage connections. + * It uses am `InMemorySubscriptionManager` for individual subscription management within the connection. + */ export class InMemoryConnectionManager implements ConnectionManager { constructor(private dwn: Dwn, private connections: Map = new Map()) {} @@ -27,7 +40,19 @@ export class InMemoryConnectionManager implements ConnectionManager { }); } - async close(): Promise { + /** + * Handler for closing websocket event - `close`. + */ + async close(socket: WebSocket): Promise { + const connection = this.connections.get(socket); + this.connections.delete(socket); + await connection.close(); + } + + /** + * Closes all associated connections. + */ + async closeAll(): Promise { const closePromises = []; this.connections.forEach(connection => closePromises.push(connection.close())); await Promise.all(closePromises); diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index fabcab1..d45d554 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -1,9 +1,9 @@ import type { Dwn, GenericMessage } from "@tbd54566975/dwn-sdk-js"; import { DwnMethodName } from "@tbd54566975/dwn-sdk-js"; +import type { WebSocket } from "ws"; import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; -import type { WebSocket } from "ws"; import type { RequestContext } from "../lib/json-rpc-router.js"; import type { SubscriptionManager } from "../subscription-manager.js"; @@ -16,6 +16,9 @@ import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcSuccessResp const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; +/** + * SocketConnection class sets up a socket connection along with a `ping/pong` heartbeat. + */ export class SocketConnection { private heartbeatInterval: NodeJS.Timer; constructor( @@ -74,6 +77,9 @@ export class SocketConnection { } } + /** + * Handles a `JSON RPC 2.0` encoded message. + */ private async message(dataBuffer: Buffer): Promise { const requestData = dataBuffer.toString(); if (!requestData) { @@ -110,10 +116,17 @@ export class SocketConnection { this.send(jsonRpcResponse); } + /** + * Sends a JSON encoded Buffer through the Websocket. + */ private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); } + /** + * Subscription Handler used to build the context for a `JSON RPC` API call. + * Wraps the incoming `message` in a `JSON RPC Success Response` using the origin subscription`JSON RPC Id` to send through the WebSocket. + */ private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { return (message) => { const response = createJsonRpcSuccessResponse(id, { reply: { @@ -123,6 +136,11 @@ export class SocketConnection { } } + /** + * Builds a `RequestContext` object to use with the `JSON RPC API`. + * + * Adds a `subscriptionHandler` for `Subscribe` messages. + */ private async buildRequestContext(request: JsonRpcRequest): Promise { const { id, params, method} = request; const requestContext: RequestContext = { diff --git a/src/http-api.ts b/src/http-api.ts index 93128be..ff0c1a2 100644 --- a/src/http-api.ts +++ b/src/http-api.ts @@ -31,6 +31,8 @@ export class HttpApi { dwn: Dwn; constructor(config: DwnServerConfig, dwn: Dwn, registrationManager?: RegistrationManager) { + console.log(config); + this.#config = config; this.#api = express(); this.#server = http.createServer(this.#api); diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts index 76862f8..c38c386 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -13,6 +13,12 @@ import { JsonRpcErrorCodes, } from '../../lib/json-rpc.js'; +/** + * Closes a subscription for a given `target` and `subscriptionId` within a given connection's `SubscriptionManager`. + * @param dwnRequest must include the `target` and `subscriptionId` within the `params`. + * @param context must include the `subscriptionManager` for the associated connection. + * + */ export const handleSubscriptionsClose: JsonRpcHandler = async ( dwnRequest, context, diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index f92368c..8065a15 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -8,9 +8,12 @@ import type { JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; export type RequestContext = { transport: 'http' | 'ws'; dwn: Dwn; + /** The `SubscriptionManager` associated with a subscription request, only used in `ws` requests */ subscriptionManager?: SubscriptionManager; - dataStream?: Readable; + /** The `MessageSubscriptionHandler` associated with a subscription request, only used in `ws` requests */ subscriptionHandler?: MessageSubscriptionHandler; + /** The `Readable` stream associated with a `RecordsWrite` request only used in `ws` requests */ + dataStream?: Readable; }; export type HandlerResponse = { diff --git a/src/subscription-manager.ts b/src/subscription-manager.ts index de87dec..ac18d2b 100644 --- a/src/subscription-manager.ts +++ b/src/subscription-manager.ts @@ -11,8 +11,13 @@ export interface SubscriptionManager { closeAll: () => Promise; } +/** + * Simple InMemory implementation of a `SubscriptionManager`. + * Uses `Map` to manage internal state. + */ export class InMemorySubscriptionManager implements SubscriptionManager { constructor(private subscriptions: Map> = new Map()){}; + async subscribe(target: string, subscription: MessageSubscription): Promise { let targetSubscriptions = this.subscriptions.get(target); if (targetSubscriptions === undefined) { diff --git a/src/ws-api.ts b/src/ws-api.ts index 25bcc8b..57e807c 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -48,6 +48,6 @@ export class WsApi { async close(): Promise { this.#wsServer.close(); - await this.#connections.close(); + await this.#connections.closeAll(); } } From 47279cd1c84c27ac26655d493dc091331fde7797 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 14:00:40 -0500 Subject: [PATCH 12/40] refactor SocketConnection --- src/connection/connection-manager.ts | 4 +- src/connection/socket-connection.ts | 47 ++++++++++++--- src/dwn-error.ts | 3 +- src/json-rpc-handlers/dwn/process-message.ts | 33 ++++++++++- src/json-rpc-handlers/subscriptions/close.ts | 21 +++---- src/lib/json-rpc-router.ts | 5 +- src/subscription-manager.ts | 60 -------------------- tests/utils.ts | 10 ++-- 8 files changed, 91 insertions(+), 92 deletions(-) delete mode 100644 src/subscription-manager.ts diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts index 931315d..d8a1947 100644 --- a/src/connection/connection-manager.ts +++ b/src/connection/connection-manager.ts @@ -4,7 +4,6 @@ import type { IncomingMessage } from "http"; import type { WebSocket } from 'ws'; import { SocketConnection } from "./socket-connection.js"; -import { InMemorySubscriptionManager } from "../subscription-manager.js"; /** * Interface for managing `WebSocket` connections as they arrive. @@ -21,7 +20,6 @@ export interface ConnectionManager { /** * A Simple In Memory ConnectionManager implementation. * It uses a `Map` to manage connections. - * It uses am `InMemorySubscriptionManager` for individual subscription management within the connection. */ export class InMemoryConnectionManager implements ConnectionManager { constructor(private dwn: Dwn, private connections: Map = new Map()) {} @@ -31,7 +29,7 @@ export class InMemoryConnectionManager implements ConnectionManager { * Sets listeners for `message`, `pong`, `close`, and `error` events. */ async connect(socket: WebSocket): Promise { - const connection = new SocketConnection(socket, this.dwn, new InMemorySubscriptionManager()); + const connection = new SocketConnection(socket, this.dwn); this.connections.set(socket, connection); // attach to the socket's close handler to clean up this connection. socket.on('close', () => { diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index d45d554..6613ff1 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -6,25 +6,31 @@ import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; import type { RequestContext } from "../lib/json-rpc-router.js"; -import type { SubscriptionManager } from "../subscription-manager.js"; import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from "../lib/json-rpc.js"; import { requestCounter } from "../metrics.js"; import { jsonRpcApi } from "../json-rpc-api.js"; import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcSuccessResponse } from "../lib/json-rpc.js"; +import { DwnServerError, DwnServerErrorCode } from "../dwn-error.js"; const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; +export interface Subscription { + id: JsonRpcId; + close: () => Promise; +} + /** * SocketConnection class sets up a socket connection along with a `ping/pong` heartbeat. */ export class SocketConnection { private heartbeatInterval: NodeJS.Timer; + private subscriptions: Map = new Map(); + constructor( private socket: WebSocket, - private dwn: Dwn, - private subscriptions: SubscriptionManager + private dwn: Dwn ){ socket.on('close', this.close.bind(this)); socket.on('pong', this.pong.bind(this)); @@ -46,6 +52,28 @@ export class SocketConnection { }, HEARTBEAT_INTERVAL); } + async subscribe(subscription: Subscription): Promise { + if (this.subscriptions.has(subscription.id)) { + throw new DwnServerError( + DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists, + `the subscription with id ${subscription.id} already exists` + ) + } + + this.subscriptions.set(subscription.id, subscription); + } + + async closeSubscription(id: JsonRpcId): Promise { + if (!this.subscriptions.has(id)) { + throw new DwnServerError( + DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdNotFound, + `the subscription with id ${id} was not found` + ) + } + + this.subscriptions.delete(id); + } + /** * Closes the existing connection and cleans up any listeners or subscriptions. */ @@ -54,8 +82,13 @@ export class SocketConnection { // clean up all socket event listeners this.socket.removeAllListeners(); + const closePromises = []; + for (const [_target, subscription] of this.subscriptions) { + closePromises.push(subscription.close()); + } + // close all of the associated subscriptions - await this.subscriptions.closeAll(); + await Promise.all(closePromises); // close the socket. this.socket.close(); @@ -144,9 +177,9 @@ export class SocketConnection { private async buildRequestContext(request: JsonRpcRequest): Promise { const { id, params, method} = request; const requestContext: RequestContext = { - transport : 'ws', - dwn : this.dwn, - subscriptionManager : this.subscriptions, + transport : 'ws', + dwn : this.dwn, + socketConnection : this, } if (method === 'dwn.processMessage') { diff --git a/src/dwn-error.ts b/src/dwn-error.ts index 0163a28..196a5e2 100644 --- a/src/dwn-error.ts +++ b/src/dwn-error.ts @@ -26,12 +26,13 @@ export class DwnServerError extends Error { * DWN Server error codes. */ export enum DwnServerErrorCode { + ConnectionSubscriptionJsonRPCIdExists = 'ConnectionSubscriptionJsonRPCIdExists', + ConnectionSubscriptionJsonRPCIdNotFound = 'ConnectionSubscriptionJsonRPCIdNotFound', ProofOfWorkInsufficientSolutionNonce = 'ProofOfWorkInsufficientSolutionNonce', ProofOfWorkInvalidOrExpiredChallenge = 'ProofOfWorkInvalidOrExpiredChallenge', ProofOfWorkManagerInvalidChallengeNonce = 'ProofOfWorkManagerInvalidChallengeNonce', ProofOfWorkManagerInvalidResponseNonceFormat = 'ProofOfWorkManagerInvalidResponseNonceFormat', ProofOfWorkManagerResponseNonceReused = 'ProofOfWorkManagerResponseNonceReused', RegistrationManagerInvalidOrOutdatedTermsOfServiceHash = 'RegistrationManagerInvalidOrOutdatedTermsOfServiceHash', - SubscriptionManagerSubscriptionNotFound = 'SubscriptionManagerSubscriptionNotFound', TenantRegistrationOutdatedTermsOfService = 'TenantRegistrationOutdatedTermsOfService', } diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index d46cbb4..7ea296e 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -4,11 +4,15 @@ import { DwnInterfaceName, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; import { v4 as uuidv4 } from 'uuid'; +import type { + JsonRpcErrorResponse, +} from '../../lib/json-rpc.js'; import type { HandlerResponse, JsonRpcHandler, } from '../../lib/json-rpc-router.js'; +import { DwnServerErrorCode } from '../../dwn-error.js'; import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, @@ -19,7 +23,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( dwnRequest, context, ) => { - const { dwn, dataStream, subscriptionHandler, subscriptionManager, transport } = context; + const { dwn, dataStream, subscriptionHandler, socketConnection, transport } = context; const { target, message } = dwnRequest.params as { target: string, message: GenericMessage }; const requestId = dwnRequest.id ?? uuidv4(); @@ -65,8 +69,31 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( // Subscribe messages return a close function to facilitate closing the subscription if (subscription !== undefined) { - const { id, close } = subscription; - subscriptionManager.subscribe(target, { id, close }); + const { close } = subscription; + try { + await socketConnection.subscribe({ + id: requestId, + close, + }) + } catch(error) { + let errorResponse: JsonRpcErrorResponse; + if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists) { + // a subscription with this request id already exists + errorResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.BadRequest, + `the request id ${requestId} already has an active subscription` + ); + } else { + // will catch as an unknown error below + throw new Error('unknown error adding subscription'); + } + + // close the subscription that was just opened and return an error + await close(); + return { jsonRpcResponse: errorResponse }; + } + delete reply.subscription.close // not serializable via JSON } diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts index c38c386..b926447 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -6,7 +6,7 @@ import type { JsonRpcHandler, } from '../../lib/json-rpc-router.js'; -import type { JsonRpcResponse } from '../../lib/json-rpc.js'; +import type { JsonRpcId, JsonRpcResponse } from '../../lib/json-rpc.js'; import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, @@ -14,9 +14,10 @@ import { } from '../../lib/json-rpc.js'; /** - * Closes a subscription for a given `target` and `subscriptionId` within a given connection's `SubscriptionManager`. - * @param dwnRequest must include the `target` and `subscriptionId` within the `params`. - * @param context must include the `subscriptionManager` for the associated connection. + * Closes a subscription for a given `id` for a given `SocketConnection` + * + * @param dwnRequest must include the `id` of the subscription to close within the `params`. + * @param context must include the associated `SocketConnection`. * */ export const handleSubscriptionsClose: JsonRpcHandler = async ( @@ -24,21 +25,21 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( context, ) => { const requestId = dwnRequest.id ?? uuidv4(); - const { subscriptionManager } = context; - const { target, subscriptionId } = dwnRequest.params as { target: string, subscriptionId: string }; + const { socketConnection } = context; + const { id } = dwnRequest.params as { id: JsonRpcId}; let jsonRpcResponse:JsonRpcResponse; try { - await subscriptionManager.close(target, subscriptionId); + await socketConnection.closeSubscription(id); jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply: { status: 200, detail: 'Accepted' } }); } catch(error) { - if (error.code === DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound) { - jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${subscriptionId} does not exist.`); + if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdNotFound) { + jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${id} does not exist.`); } else { jsonRpcResponse = createJsonRpcErrorResponse( requestId, JsonRpcErrorCodes.InternalError, - `unknown subscription close error for ${subscriptionId}: ${error.message}` + `unknown subscription close error for ${id}: ${error.message}` ); } } diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 8065a15..18ba03e 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -2,14 +2,13 @@ import type { Dwn, MessageSubscriptionHandler } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; -import type { SubscriptionManager } from '../subscription-manager.js'; import type { JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; +import type { SocketConnection } from '../connection/socket-connection.js'; export type RequestContext = { transport: 'http' | 'ws'; dwn: Dwn; - /** The `SubscriptionManager` associated with a subscription request, only used in `ws` requests */ - subscriptionManager?: SubscriptionManager; + socketConnection?: SocketConnection; /** The `MessageSubscriptionHandler` associated with a subscription request, only used in `ws` requests */ subscriptionHandler?: MessageSubscriptionHandler; /** The `Readable` stream associated with a `RecordsWrite` request only used in `ws` requests */ diff --git a/src/subscription-manager.ts b/src/subscription-manager.ts deleted file mode 100644 index ac18d2b..0000000 --- a/src/subscription-manager.ts +++ /dev/null @@ -1,60 +0,0 @@ -import type { MessageSubscription } from "@tbd54566975/dwn-sdk-js"; - -import { DwnServerError, DwnServerErrorCode } from "./dwn-error.js"; - -/** - * SubscriptionManager manages the subscriptions related to a `SocketConnection` - */ -export interface SubscriptionManager { - subscribe: (target: string, subscription: MessageSubscription) => Promise; - close: (target: string, id: string) => Promise; - closeAll: () => Promise; -} - -/** - * Simple InMemory implementation of a `SubscriptionManager`. - * Uses `Map` to manage internal state. - */ -export class InMemorySubscriptionManager implements SubscriptionManager { - constructor(private subscriptions: Map> = new Map()){}; - - async subscribe(target: string, subscription: MessageSubscription): Promise { - let targetSubscriptions = this.subscriptions.get(target); - if (targetSubscriptions === undefined) { - targetSubscriptions = new Map(); - this.subscriptions.set(target, targetSubscriptions); - } - targetSubscriptions.set(subscription.id, subscription); - } - - async close(target: string, id: string): Promise { - const targetSubscriptions = this.subscriptions.get(target); - if (targetSubscriptions !== undefined) { - const subscription = targetSubscriptions.get(id); - if (subscription !== undefined) { - targetSubscriptions.delete(id); - await subscription.close(); - return; - } - } - - // if it reached here no subscription to close - throw new DwnServerError( - DwnServerErrorCode.SubscriptionManagerSubscriptionNotFound, - `subscription ${id} was not found` - ) - } - - async closeAll(): Promise { - const closePromises = []; - for (const [target, subscriptions] of this.subscriptions) { - this.subscriptions.delete(target); - for (const [id, subscription] of subscriptions) { - subscriptions.delete(id); - closePromises.push(subscription.close()); - } - } - - await Promise.all(closePromises); - } -} \ No newline at end of file diff --git a/tests/utils.ts b/tests/utils.ts index c05021d..0df40ff 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -11,7 +11,7 @@ import type { Readable } from 'readable-stream'; import { fileURLToPath } from 'url'; import { WebSocket } from 'ws'; -import type { JsonRpcResponse, JsonRpcRequest } from '../src/lib/json-rpc.js'; +import type { JsonRpcResponse, JsonRpcRequest, JsonRpcId } from '../src/lib/json-rpc.js'; import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; import { JSONRPCSocket } from '../src/json-rpc-socket.js'; @@ -230,12 +230,12 @@ export async function subscriptionRequest( messageHandler: MessageSubscriptionHandler ): Promise<{ status: any, subscription?: { id: string, close: () => Promise } }> { let resolved: boolean = false; - const { params: { target } } = request; + const { id: requestId } = request; const connection = await JSONRPCSocket.connect(url); - const closeSubscription = async (id: string, target: string, connection: JSONRPCSocket): Promise => { + const closeSubscription = async (id: JsonRpcId, connection: JSONRPCSocket): Promise => { const requestId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'subscriptions.close', { subscriptionId: id, target }); + const request = createJsonRpcRequest(requestId, 'subscriptions.close', { id }); return await connection.request(request); } @@ -263,7 +263,7 @@ export async function subscriptionRequest( ...subscription, close: async (): Promise => { subscriptionClose(); - const closeResponse = await closeSubscription(subscription.id, target, connection); + const closeResponse = await closeSubscription(requestId, connection); if (closeResponse.error?.message !== undefined) { throw new Error(`unable to close subscription: ${closeResponse.error.message}`); } From d9b731a1351c7fc0c4522831a22e0e3962954005 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 15:58:09 -0500 Subject: [PATCH 13/40] clearer comments related to connetion reference handling and close --- src/connection/socket-connection.ts | 30 ++++++++++++++------ src/dwn-server.ts | 2 +- src/json-rpc-api.ts | 2 +- src/json-rpc-handlers/dwn/process-message.ts | 27 +++++++----------- src/json-rpc-handlers/subscriptions/close.ts | 5 ++-- src/json-rpc-socket.ts | 24 ++++++++++------ tests/utils.ts | 2 +- 7 files changed, 53 insertions(+), 39 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 6613ff1..903c70a 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -16,26 +16,28 @@ import { DwnServerError, DwnServerErrorCode } from "../dwn-error.js"; const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; -export interface Subscription { +export interface JsonRPCSubscription { + /** JSON RPC Id of the Subscription Request */ id: JsonRpcId; close: () => Promise; } /** - * SocketConnection class sets up a socket connection along with a `ping/pong` heartbeat. + * SocketConnection handles a WebSocket connection to a DWN using JSON RPC. + * It also manages references to the long running RPC subscriptions for the connection. */ export class SocketConnection { private heartbeatInterval: NodeJS.Timer; - private subscriptions: Map = new Map(); + private subscriptions: Map = new Map(); constructor( private socket: WebSocket, private dwn: Dwn ){ + socket.on('message', this.message.bind(this)); socket.on('close', this.close.bind(this)); - socket.on('pong', this.pong.bind(this)); socket.on('error', this.error.bind(this)); - socket.on('message', this.message.bind(this)); + socket.on('pong', this.pong.bind(this)); // Sometimes connections between client <-> server can get borked in such a way that // leaves both unaware of the borkage. ping messages can be used as a means to verify @@ -52,7 +54,11 @@ export class SocketConnection { }, HEARTBEAT_INTERVAL); } - async subscribe(subscription: Subscription): Promise { + /** + * Adds a reference for the JSON RPC Subscription to this connection. + * Used for cleanup if the connection is closed. + */ + async subscribe(subscription: JsonRPCSubscription): Promise { if (this.subscriptions.has(subscription.id)) { throw new DwnServerError( DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists, @@ -63,6 +69,11 @@ export class SocketConnection { this.subscriptions.set(subscription.id, subscription); } + /** + * Closes and removes the reference for a given subscription from this connection. + * + * @param id the `JsonRpcId` of the JSON RPC subscription request. + */ async closeSubscription(id: JsonRpcId): Promise { if (!this.subscriptions.has(id)) { throw new DwnServerError( @@ -71,6 +82,8 @@ export class SocketConnection { ) } + const connection = this.subscriptions.get(id); + await connection.close(); this.subscriptions.delete(id); } @@ -83,8 +96,9 @@ export class SocketConnection { this.socket.removeAllListeners(); const closePromises = []; - for (const [_target, subscription] of this.subscriptions) { + for (const [id, subscription] of this.subscriptions) { closePromises.push(subscription.close()); + this.subscriptions.delete(id); } // close all of the associated subscriptions @@ -158,7 +172,7 @@ export class SocketConnection { /** * Subscription Handler used to build the context for a `JSON RPC` API call. - * Wraps the incoming `message` in a `JSON RPC Success Response` using the origin subscription`JSON RPC Id` to send through the WebSocket. + * Wraps the incoming `message` in a `JSON RPC Success Response` using the original subscription`JSON RPC Id` to send through the WebSocket. */ private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { return (message) => { diff --git a/src/dwn-server.ts b/src/dwn-server.ts index b2502e9..90de633 100644 --- a/src/dwn-server.ts +++ b/src/dwn-server.ts @@ -65,7 +65,7 @@ export class DwnServer { let eventStream: EventStream | undefined; if (this.config.webSocketServerEnabled) { // setting `EventEmitterStream` as default the default `EventStream - // if an alternate implementation is needed instantiate a `Dwn` with a custom `EventStream` and add it to server options. + // if an alternate implementation is needed, instantiate a `Dwn` with a custom `EventStream` and add it to server options. eventStream = new EventEmitterStream(); } diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index 4bcedff..76e971e 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -6,4 +6,4 @@ import { handleSubscriptionsClose } from './json-rpc-handlers/subscriptions/inde export const jsonRpcApi = new JsonRpcRouter(); jsonRpcApi.on('dwn.processMessage', handleDwnProcessMessage); -jsonRpcApi.on('subscriptions.close', handleSubscriptionsClose); +jsonRpcApi.on('subscription.close', handleSubscriptionsClose); diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 7ea296e..8380800 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -4,9 +4,6 @@ import { DwnInterfaceName, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; import { v4 as uuidv4 } from 'uuid'; -import type { - JsonRpcErrorResponse, -} from '../../lib/json-rpc.js'; import type { HandlerResponse, JsonRpcHandler, @@ -28,8 +25,8 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( const requestId = dwnRequest.id ?? uuidv4(); try { - - // RecordsWrite is only supported on 'http' + // RecordsWrite is only supported on 'http' to support data stream for large data + // TODO: https://github.com/TBD54566975/dwn-server/issues/108 if ( transport !== 'http' && message.descriptor.interface === DwnInterfaceName.Records && @@ -71,30 +68,26 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( if (subscription !== undefined) { const { close } = subscription; try { - await socketConnection.subscribe({ - id: requestId, - close, - }) + // adding a reference to the close function for this subscription request to the connection. + // this will facilitate closing the subscription later. + await socketConnection.subscribe({ id: requestId, close }); + delete reply.subscription.close // not serializable via JSON } catch(error) { - let errorResponse: JsonRpcErrorResponse; + // close the subscription upon receiving an error here + await close(); if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists) { // a subscription with this request id already exists - errorResponse = createJsonRpcErrorResponse( + const errorResponse = createJsonRpcErrorResponse( requestId, JsonRpcErrorCodes.BadRequest, `the request id ${requestId} already has an active subscription` ); + return { jsonRpcResponse: errorResponse }; } else { // will catch as an unknown error below throw new Error('unknown error adding subscription'); } - - // close the subscription that was just opened and return an error - await close(); - return { jsonRpcResponse: errorResponse }; } - - delete reply.subscription.close // not serializable via JSON } const jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply }); diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscriptions/close.ts index b926447..0f8f889 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscriptions/close.ts @@ -14,9 +14,9 @@ import { } from '../../lib/json-rpc.js'; /** - * Closes a subscription for a given `id` for a given `SocketConnection` + * Closes a subscription tied to a specific `SocketConnection`. * - * @param dwnRequest must include the `id` of the subscription to close within the `params`. + * @param dwnRequest must include JsonRpcId of the subscription request within the `params`. * @param context must include the associated `SocketConnection`. * */ @@ -30,6 +30,7 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( let jsonRpcResponse:JsonRpcResponse; try { + // closing the subscription and cleaning up the reference within the given connection. await socketConnection.closeSubscription(id); jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply: { status: 200, detail: 'Accepted' } }); } catch(error) { diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 0622b37..df4cebe 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -8,9 +8,11 @@ import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; const CONNECT_TIMEOUT = 3_000; const RESPONSE_TIMEOUT = 30_000; -export type JSONRPCSocketOptions = { +export interface JSONRPCSocketOptions { connectTimeout?: number; responseTimeout?: number; + onclose?: () => void; + onerror?: (error?: any) => void; } /** @@ -20,17 +22,21 @@ export class JSONRPCSocket { private constructor(private socket: WebSocket, private responseTimeout: number) {} static async connect(url: string, options: JSONRPCSocketOptions = {}): Promise { - const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT } = options; + const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; - const onclose = ():void => { - log.info(`JSON RPC Socket close ${url}`); - }; + const socket = new WebSocket(url); + if (onclose === undefined) { + socket.onclose = ():void => { + log.info(`JSON RPC Socket close ${url}`); + } + } - const onerror = (event: any):void => { - log.error(`JSON RPC Socket error ${url}`, event); - }; + if (onerror === undefined) { + socket.onerror = (error?: any):void => { + log.error(`JSON RPC Socket error ${url}`, error); + } + } - const socket = new WebSocket(url); socket.onclose = onclose; socket.onerror = onerror; diff --git a/tests/utils.ts b/tests/utils.ts index 0df40ff..60b586f 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -235,7 +235,7 @@ export async function subscriptionRequest( const closeSubscription = async (id: JsonRpcId, connection: JSONRPCSocket): Promise => { const requestId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'subscriptions.close', { id }); + const request = createJsonRpcRequest(requestId, 'subscription.close', { id }); return await connection.request(request); } From c14ab4faab0dd1854fb7e35ec37426a67dec7d18 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 17:58:45 -0500 Subject: [PATCH 14/40] increase coverage in ws-api and dwn-server --- src/dwn-server.ts | 4 ++-- tests/dwn-server.spec.ts | 31 +++++++++++++++++++++++++++++++ tests/ws-api.spec.ts | 26 ++++++++++++++++---------- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/dwn-server.ts b/src/dwn-server.ts index 90de633..7069653 100644 --- a/src/dwn-server.ts +++ b/src/dwn-server.ts @@ -99,8 +99,8 @@ export class DwnServer { return this.#httpApi.server; } - get wsServer(): WebSocketServer { - return this.#wsApi.server; + get wsServer(): WebSocketServer | undefined { + return this.#wsApi?.server; } /** diff --git a/tests/dwn-server.spec.ts b/tests/dwn-server.spec.ts index 0cc2888..56ba884 100644 --- a/tests/dwn-server.spec.ts +++ b/tests/dwn-server.spec.ts @@ -17,4 +17,35 @@ describe('DwnServer', function () { dwnServer.stop(() => console.log('server Stop')); expect(dwnServer.httpServer.listening).to.be.false; }); + + describe('webSocketServerEnabled config', function() { + it('should not return a websocket server if disabled', async function() { + const withoutSocketServer = new DwnServer({ + config: { + ...dwnServerConfig, + webSocketServerEnabled: false, + } + }); + + await withoutSocketServer.start(() => console.log('Started without sockets.')); + expect(withoutSocketServer.httpServer.listening).to.be.true; + expect(withoutSocketServer.wsServer).to.be.undefined; + withoutSocketServer.stop(() => console.log('server Stop')); + expect(withoutSocketServer.httpServer.listening).to.be.false; + }); + + it('should return a websocket server if enabled', async function() { + const withSocketServer = new DwnServer({ + config: { + ...dwnServerConfig, + webSocketServerEnabled: true, + } + }); + + await withSocketServer.start(() => console.log('Started with sockets')); + expect(withSocketServer.wsServer).to.not.be.undefined; + withSocketServer.stop(() => console.log('server Stop')); + expect(withSocketServer.httpServer.listening).to.be.false; + }); + }); }); diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 74661f2..a6d4d31 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,11 +1,11 @@ import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; import { DataStream, Message, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; +import type { AddressInfo } from 'ws'; + import { expect } from 'chai'; import { base64url } from 'multiformats/bases/base64'; -import type { Server } from 'http'; import { v4 as uuidv4 } from 'uuid'; -import { type WebSocketServer } from 'ws'; import { createJsonRpcRequest, @@ -17,8 +17,7 @@ import { getTestDwn } from './test-dwn.js'; import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest } from './utils.js'; import { HttpApi } from '../src/http-api.js'; -let server: Server; -let wsServer: WebSocketServer; +let wsApi: WsApi; let dwn: Dwn; describe('websocket api', function () { @@ -27,16 +26,23 @@ describe('websocket api', function () { // set up http api for issuing writes within the tests const httpApi = new HttpApi(config, dwn); - server = await httpApi.start(9002); + await httpApi.start(9002); - const wsApi = new WsApi(server, dwn); - wsServer = wsApi.start(); + wsApi = new WsApi(httpApi.server, dwn); + wsApi.start(); }); after(function () { - wsServer.close(); - server.close(); - server.closeAllConnections(); + wsApi.close(); + }); + + it('returns the server address', async function() { + if (typeof wsApi.address === 'string') { + expect(wsApi.address).to.include('9002'); + } else { + const port = (wsApi.address as AddressInfo).port; + expect(port).to.equal(9002); + } }); it('returns an error response if no request payload is provided', async function () { From 9451a26d7ddfe06d6f32b1f8bb51dd910655e2d1 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 19:05:48 -0500 Subject: [PATCH 15/40] clean up listener on reject --- src/json-rpc-socket.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index df4cebe..74f6d3b 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -74,7 +74,10 @@ export class JSONRPCSocket { this.send(request); // reject this promise if we don't receive any response back within the timeout period - setTimeout(reject, this.responseTimeout); + setTimeout(() => { + this.socket.removeEventListener('message', handleResponse); + reject('request timed out'); + }, this.responseTimeout); }); } From 5ed80e05f138eb81b6601ad2d216ff24ca130a94 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 21:06:13 -0500 Subject: [PATCH 16/40] additional JSON RPC Socket client coverage --- src/json-rpc-socket.ts | 6 +- tests/json-rpc-socket.spec.ts | 118 ++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 tests/json-rpc-socket.spec.ts diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 74f6d3b..b3c0e19 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -9,9 +9,13 @@ const CONNECT_TIMEOUT = 3_000; const RESPONSE_TIMEOUT = 30_000; export interface JSONRPCSocketOptions { + /** socket connection timeout in milliseconds */ connectTimeout?: number; + /** response timeout for rpc requests in milliseconds */ responseTimeout?: number; + /** optional connection close handler */ onclose?: () => void; + /** optional socket error handler */ onerror?: (error?: any) => void; } @@ -76,7 +80,7 @@ export class JSONRPCSocket { // reject this promise if we don't receive any response back within the timeout period setTimeout(() => { this.socket.removeEventListener('message', handleResponse); - reject('request timed out'); + reject(new Error('request timed out')); }, this.responseTimeout); }); } diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts new file mode 100644 index 0000000..abf796f --- /dev/null +++ b/tests/json-rpc-socket.spec.ts @@ -0,0 +1,118 @@ +import { expect } from 'chai'; +import { v4 as uuidv4 } from 'uuid'; +import { WebSocketServer } from 'ws'; + +import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json-rpc.js'; + +import { JSONRPCSocket } from '../src/json-rpc-socket.js'; +import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; + +describe.only('JSONRPCSocket', () => { + let wsServer: WebSocketServer; + + before(async () => { + wsServer = new WebSocketServer({ + port: 9003, + }); + }); + + beforeEach(() => { + wsServer.removeAllListeners(); + }); + + it('connects to a url', async () => { + const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + expect(wsServer.clients.size).to.equal(1); + client.close(); + + // give time for the connection to close on the server. + await new Promise((resolve) => setTimeout(resolve, 5)); + expect(wsServer.clients.size).to.equal(0); + }); + + it('resolves a request with given params', async () => { + wsServer.addListener('connection', (socket) => { + socket.on('message', (dataBuffer: Buffer) => { + const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; + const { param1, param2 } = request.params; + expect(param1).to.equal('test-param1'); + expect(param2).to.equal('test-param2'); + + // send response passed tests + const response = createJsonRpcSuccessResponse(request.id, {}); + socket.send(Buffer.from(JSON.stringify(response))); + }); + }); + + const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + const response = await client.request(request); + expect(response.id).to.equal(request.id); + }); + + it('request times out', async () => { + // time out after 1 ms + const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + + try { + await client.request(request); + } catch (error) { + expect(error).to.not.be.undefined; + expect((error as Error).message).to.include('timed out'); + } + }); + + it('opens a subscription', async () => { + wsServer.addListener('connection', (socket) => { + socket.on('message', (dataBuffer: Buffer) => { + const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; + // send 3 messages + for (let i = 0; i < 3; i++) { + const response = createJsonRpcSuccessResponse(request.id, { count: i }); + socket.send(Buffer.from(JSON.stringify(response))); + } + }); + }); + const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + + let responseCounter = 0; + const responseListener = (response: JsonRpcResponse): void => { + expect(response.id).to.equal(request.id); + const { count } = response.result; + expect(count).to.equal(responseCounter); + responseCounter++; + } + + const subscription = client.subscribe(request, responseListener); + // wait for the messages to arrive + await new Promise((resolve) => setTimeout(resolve, 5)); + // the original response + expect(responseCounter).to.equal(3); + subscription.close(); + }); + + it('sends message', async () => { + const received = new Promise<{ id?: JsonRpcId }>((resolve) => { + wsServer.addListener('connection', (socket) => { + socket.on('message', (dataBuffer: Buffer) => { + const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; + const { param1, param2 } = request.params; + expect(param1).to.equal('test-param1'); + expect(param2).to.equal('test-param2'); + resolve(request); + }); + }); + }); + const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + client.send(request); + const resolved = await received; + expect(resolved.id).to.equal(request.id); + }); +}); From 0f0a22380097eb0292a2b85d0f599fbf3d0b4669 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 13 Feb 2024 21:09:52 -0500 Subject: [PATCH 17/40] remove only test decorator --- tests/json-rpc-socket.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index abf796f..65f64dd 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -7,7 +7,7 @@ import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json import { JSONRPCSocket } from '../src/json-rpc-socket.js'; import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; -describe.only('JSONRPCSocket', () => { +describe('JSONRPCSocket', () => { let wsServer: WebSocketServer; before(async () => { From be4a981792129d69381b814d2d2084d409779f33 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 17:21:56 -0500 Subject: [PATCH 18/40] addressed PR suggestions --- src/connection/connection-manager.ts | 10 --------- src/connection/socket-connection.ts | 33 ++++++++++++++-------------- src/dwn-server.ts | 6 ++--- src/ws-api.ts | 19 +++++----------- tests/dwn-server.spec.ts | 4 ++-- tests/ws-api.spec.ts | 11 ---------- 6 files changed, 28 insertions(+), 55 deletions(-) diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts index d8a1947..342048c 100644 --- a/src/connection/connection-manager.ts +++ b/src/connection/connection-manager.ts @@ -24,10 +24,6 @@ export interface ConnectionManager { export class InMemoryConnectionManager implements ConnectionManager { constructor(private dwn: Dwn, private connections: Map = new Map()) {} - /** - * Handler for opening websocket event - `connection`. - * Sets listeners for `message`, `pong`, `close`, and `error` events. - */ async connect(socket: WebSocket): Promise { const connection = new SocketConnection(socket, this.dwn); this.connections.set(socket, connection); @@ -38,18 +34,12 @@ export class InMemoryConnectionManager implements ConnectionManager { }); } - /** - * Handler for closing websocket event - `close`. - */ async close(socket: WebSocket): Promise { const connection = this.connections.get(socket); this.connections.delete(socket); await connection.close(); } - /** - * Closes all associated connections. - */ async closeAll(): Promise { const closePromises = []; this.connections.forEach(connection => closePromises.push(connection.close())); diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 903c70a..3b05c30 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -13,10 +13,9 @@ import { jsonRpcApi } from "../json-rpc-api.js"; import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcSuccessResponse } from "../lib/json-rpc.js"; import { DwnServerError, DwnServerErrorCode } from "../dwn-error.js"; -const SOCKET_ISALIVE_SYMBOL = Symbol('isAlive'); const HEARTBEAT_INTERVAL = 30_000; -export interface JsonRPCSubscription { +export interface JsonRpcSubscription { /** JSON RPC Id of the Subscription Request */ id: JsonRpcId; close: () => Promise; @@ -28,7 +27,8 @@ export interface JsonRPCSubscription { */ export class SocketConnection { private heartbeatInterval: NodeJS.Timer; - private subscriptions: Map = new Map(); + private subscriptions: Map = new Map(); + private isAlive: boolean; constructor( private socket: WebSocket, @@ -44,12 +44,12 @@ export class SocketConnection { // that the remote endpoint is still responsive. Server will ping each socket every 30s // if a pong hasn't received from a socket by the next ping, the server will terminate // the socket connection - socket[SOCKET_ISALIVE_SYMBOL] = true; + this.isAlive = true; this.heartbeatInterval = setInterval(() => { - if (this.socket[SOCKET_ISALIVE_SYMBOL] === false) { + if (this.isAlive === false) { this.close(); } - this.socket[SOCKET_ISALIVE_SYMBOL] = false; + this.isAlive = false; this.socket.ping(); }, HEARTBEAT_INTERVAL); } @@ -58,7 +58,7 @@ export class SocketConnection { * Adds a reference for the JSON RPC Subscription to this connection. * Used for cleanup if the connection is closed. */ - async subscribe(subscription: JsonRPCSubscription): Promise { + async subscribe(subscription: JsonRpcSubscription): Promise { if (this.subscriptions.has(subscription.id)) { throw new DwnServerError( DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists, @@ -113,15 +113,16 @@ export class SocketConnection { * the websocket spec. So, no need to send explicit pongs. */ private pong(): void { - this.socket[SOCKET_ISALIVE_SYMBOL] = true; + this.isAlive = true; } - private async error(error?:Error): Promise{ - if (error) { - log.error(`SocketConnection error, terminating connection`, error); - this.socket.terminate(); - await this.close() - } + /** + * Log the error and close the connection. + */ + private async error(error:Error): Promise{ + log.error(`SocketConnection error, terminating connection`, error); + this.socket.terminate(); + await this.close(); } /** @@ -166,8 +167,8 @@ export class SocketConnection { /** * Sends a JSON encoded Buffer through the Websocket. */ - private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { - this.socket.send(Buffer.from(JSON.stringify(response)), this.error.bind(this)); + private send(response: JsonRpcResponse | JsonRpcErrorResponse, onError?: (error?: Error) => void): void { + this.socket.send(Buffer.from(JSON.stringify(response)), onError); } /** diff --git a/src/dwn-server.ts b/src/dwn-server.ts index 7069653..7a9e23f 100644 --- a/src/dwn-server.ts +++ b/src/dwn-server.ts @@ -40,10 +40,9 @@ export class DwnServer { prefix.apply(log); } - async start(callback?: () => void): Promise { + async start(): Promise { await this.#setupServer(); setProcessHandlers(this); - callback?.(); } /** @@ -87,7 +86,8 @@ export class DwnServer { if (this.config.webSocketServerEnabled) { this.#wsApi = new WsApi(this.#httpApi.server, this.dwn); - this.#wsApi.start(() => log.info('WebSocketServer ready...')); + this.#wsApi.start(); + log.info('WebSocketServer ready...'); } } diff --git a/src/ws-api.ts b/src/ws-api.ts index 57e807c..426aadc 100644 --- a/src/ws-api.ts +++ b/src/ws-api.ts @@ -3,7 +3,6 @@ import type { Dwn, } from '@tbd54566975/dwn-sdk-js'; -import type { AddressInfo } from 'ws'; import type { Server } from 'http'; import { WebSocketServer } from 'ws'; @@ -14,18 +13,14 @@ import { InMemoryConnectionManager } from './connection/connection-manager.js'; export class WsApi { #wsServer: WebSocketServer; dwn: Dwn; - #connections: ConnectionManager + #connectionManager: ConnectionManager constructor(server: Server, dwn: Dwn, connectionManager?: ConnectionManager) { this.dwn = dwn; - this.#connections = connectionManager || new InMemoryConnectionManager(dwn); + this.#connectionManager = connectionManager || new InMemoryConnectionManager(dwn); this.#wsServer = new WebSocketServer({ server }); } - get address(): AddressInfo | string { - return this.#wsServer.address(); - } - get server(): WebSocketServer { return this.#wsServer; } @@ -33,21 +28,19 @@ export class WsApi { /** * Handler for starting a WebSocket. * Sets listeners for `connection`, `close` events. - * It clears `heartbeatInterval` when a `close` event is made. */ #setupWebSocket(): void { - this.#wsServer.on('connection', this.#connections.connect.bind(this)); - this.#wsServer.on('close', this.#connections.close.bind(this)); + this.#wsServer.on('connection', (socket, request) => this.#connectionManager.connect(socket, request)); + this.#wsServer.on('close', () => this.#connectionManager.closeAll()); } - start(callback?: () => void): WebSocketServer { + start(): WebSocketServer { this.#setupWebSocket(); - callback?.(); return this.#wsServer; } async close(): Promise { this.#wsServer.close(); - await this.#connections.closeAll(); + await this.#connectionManager.closeAll(); } } diff --git a/tests/dwn-server.spec.ts b/tests/dwn-server.spec.ts index 56ba884..0107e51 100644 --- a/tests/dwn-server.spec.ts +++ b/tests/dwn-server.spec.ts @@ -27,7 +27,7 @@ describe('DwnServer', function () { } }); - await withoutSocketServer.start(() => console.log('Started without sockets.')); + await withoutSocketServer.start(); expect(withoutSocketServer.httpServer.listening).to.be.true; expect(withoutSocketServer.wsServer).to.be.undefined; withoutSocketServer.stop(() => console.log('server Stop')); @@ -42,7 +42,7 @@ describe('DwnServer', function () { } }); - await withSocketServer.start(() => console.log('Started with sockets')); + await withSocketServer.start(); expect(withSocketServer.wsServer).to.not.be.undefined; withSocketServer.stop(() => console.log('server Stop')); expect(withSocketServer.httpServer.listening).to.be.false; diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index a6d4d31..0eb33d0 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,8 +1,6 @@ import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; import { DataStream, Message, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; -import type { AddressInfo } from 'ws'; - import { expect } from 'chai'; import { base64url } from 'multiformats/bases/base64'; import { v4 as uuidv4 } from 'uuid'; @@ -36,15 +34,6 @@ describe('websocket api', function () { wsApi.close(); }); - it('returns the server address', async function() { - if (typeof wsApi.address === 'string') { - expect(wsApi.address).to.include('9002'); - } else { - const port = (wsApi.address as AddressInfo).port; - expect(port).to.equal(9002); - } - }); - it('returns an error response if no request payload is provided', async function () { const data = await sendWsMessage('ws://127.0.0.1:9002', Buffer.from('')); From 50dbb8972a88f185ed299fe0050ff022916e3216 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 17:25:45 -0500 Subject: [PATCH 19/40] remove close connection --- src/connection/connection-manager.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts index 342048c..250c97e 100644 --- a/src/connection/connection-manager.ts +++ b/src/connection/connection-manager.ts @@ -11,8 +11,6 @@ import { SocketConnection } from "./socket-connection.js"; export interface ConnectionManager { /** connect handler used for the `WebSockets` `'connection'` event. */ connect(socket: WebSocket, request?: IncomingMessage): Promise; - /** cleans up handlers associated with the `WebSocket` connection */ - close(socket:WebSocket): Promise; /** closes all of the connections */ closeAll(): Promise } @@ -34,12 +32,6 @@ export class InMemoryConnectionManager implements ConnectionManager { }); } - async close(socket: WebSocket): Promise { - const connection = this.connections.get(socket); - this.connections.delete(socket); - await connection.close(); - } - async closeAll(): Promise { const closePromises = []; this.connections.forEach(connection => closePromises.push(connection.close())); From 18e4a5ce5e4a215d754ebf9d52ea163dd4b9a2e9 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 17:32:25 -0500 Subject: [PATCH 20/40] optional socket send logger for SocketConnection.send() --- src/connection/socket-connection.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 3b05c30..2a3dee3 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -165,9 +165,15 @@ export class SocketConnection { } /** - * Sends a JSON encoded Buffer through the Websocket. + * Sends a JSON encoded Buffer through the Websocket. Accepts an error handler, if none is provided the error is logged. */ private send(response: JsonRpcResponse | JsonRpcErrorResponse, onError?: (error?: Error) => void): void { + if (!onError) { + onError = (error):void => { + if(error) { log.error('socket send error', error, response); } + } + } + this.socket.send(Buffer.from(JSON.stringify(response)), onError); } From 7bb207a1c9ba8670901ffd10943dc57a29815613 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 17:34:14 -0500 Subject: [PATCH 21/40] rename onError to callback --- src/connection/socket-connection.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 2a3dee3..e76e015 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -165,16 +165,16 @@ export class SocketConnection { } /** - * Sends a JSON encoded Buffer through the Websocket. Accepts an error handler, if none is provided the error is logged. + * Sends a JSON encoded Buffer through the Websocket. Accepts a callback, if none is provided an error logger is used. */ - private send(response: JsonRpcResponse | JsonRpcErrorResponse, onError?: (error?: Error) => void): void { - if (!onError) { - onError = (error):void => { + private send(response: JsonRpcResponse | JsonRpcErrorResponse, cb?: (error?: Error) => void): void { + if (!cb) { + cb = (error):void => { if(error) { log.error('socket send error', error, response); } } } - this.socket.send(Buffer.from(JSON.stringify(response)), onError); + this.socket.send(Buffer.from(JSON.stringify(response)), cb); } /** From c837cd7c97285c6ff492ca4ad87372a6bd1bd070 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 20:18:15 -0500 Subject: [PATCH 22/40] rename some classes for consistency, update coments as per PR review --- src/connection/socket-connection.ts | 4 ++-- src/dwn-error.ts | 4 ++-- src/json-rpc-api.ts | 2 +- src/json-rpc-handlers/dwn/process-message.ts | 2 +- .../{subscriptions => subscription}/close.ts | 2 +- .../{subscriptions => subscription}/index.ts | 0 src/json-rpc-socket.ts | 9 ++++----- src/lib/json-rpc-router.ts | 2 +- tests/json-rpc-socket.spec.ts | 18 ++++++++++++------ tests/utils.ts | 6 +++--- 10 files changed, 27 insertions(+), 22 deletions(-) rename src/json-rpc-handlers/{subscriptions => subscription}/close.ts (98%) rename src/json-rpc-handlers/{subscriptions => subscription}/index.ts (100%) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index e76e015..51eff9e 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -61,7 +61,7 @@ export class SocketConnection { async subscribe(subscription: JsonRpcSubscription): Promise { if (this.subscriptions.has(subscription.id)) { throw new DwnServerError( - DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists, + DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdExists, `the subscription with id ${subscription.id} already exists` ) } @@ -77,7 +77,7 @@ export class SocketConnection { async closeSubscription(id: JsonRpcId): Promise { if (!this.subscriptions.has(id)) { throw new DwnServerError( - DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdNotFound, + DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdNotFound, `the subscription with id ${id} was not found` ) } diff --git a/src/dwn-error.ts b/src/dwn-error.ts index 196a5e2..e872524 100644 --- a/src/dwn-error.ts +++ b/src/dwn-error.ts @@ -26,8 +26,8 @@ export class DwnServerError extends Error { * DWN Server error codes. */ export enum DwnServerErrorCode { - ConnectionSubscriptionJsonRPCIdExists = 'ConnectionSubscriptionJsonRPCIdExists', - ConnectionSubscriptionJsonRPCIdNotFound = 'ConnectionSubscriptionJsonRPCIdNotFound', + ConnectionSubscriptionJsonRpcIdExists = 'ConnectionSubscriptionJsonRpcIdExists', + ConnectionSubscriptionJsonRpcIdNotFound = 'ConnectionSubscriptionJsonRpcIdNotFound', ProofOfWorkInsufficientSolutionNonce = 'ProofOfWorkInsufficientSolutionNonce', ProofOfWorkInvalidOrExpiredChallenge = 'ProofOfWorkInvalidOrExpiredChallenge', ProofOfWorkManagerInvalidChallengeNonce = 'ProofOfWorkManagerInvalidChallengeNonce', diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index 76e971e..647ff21 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -1,7 +1,7 @@ import { JsonRpcRouter } from './lib/json-rpc-router.js'; import { handleDwnProcessMessage } from './json-rpc-handlers/dwn/index.js'; -import { handleSubscriptionsClose } from './json-rpc-handlers/subscriptions/index.js'; +import { handleSubscriptionsClose } from './json-rpc-handlers/subscription/index.js'; export const jsonRpcApi = new JsonRpcRouter(); diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 8380800..950e6fe 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -75,7 +75,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( } catch(error) { // close the subscription upon receiving an error here await close(); - if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdExists) { + if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdExists) { // a subscription with this request id already exists const errorResponse = createJsonRpcErrorResponse( requestId, diff --git a/src/json-rpc-handlers/subscriptions/close.ts b/src/json-rpc-handlers/subscription/close.ts similarity index 98% rename from src/json-rpc-handlers/subscriptions/close.ts rename to src/json-rpc-handlers/subscription/close.ts index 0f8f889..3c47fe1 100644 --- a/src/json-rpc-handlers/subscriptions/close.ts +++ b/src/json-rpc-handlers/subscription/close.ts @@ -34,7 +34,7 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( await socketConnection.closeSubscription(id); jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply: { status: 200, detail: 'Accepted' } }); } catch(error) { - if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRPCIdNotFound) { + if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdNotFound) { jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidParams, `subscription ${id} does not exist.`); } else { jsonRpcResponse = createJsonRpcErrorResponse( diff --git a/src/json-rpc-handlers/subscriptions/index.ts b/src/json-rpc-handlers/subscription/index.ts similarity index 100% rename from src/json-rpc-handlers/subscriptions/index.ts rename to src/json-rpc-handlers/subscription/index.ts diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index b3c0e19..480d58c 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -22,10 +22,10 @@ export interface JSONRPCSocketOptions { /** * JSONRPC Socket Client for WebSocket request/response and long-running subscriptions */ -export class JSONRPCSocket { +export class JsonRpcClient { private constructor(private socket: WebSocket, private responseTimeout: number) {} - static async connect(url: string, options: JSONRPCSocketOptions = {}): Promise { + static async connect(url: string, options: JSONRPCSocketOptions = {}): Promise { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; const socket = new WebSocket(url); @@ -44,9 +44,9 @@ export class JSONRPCSocket { socket.onclose = onclose; socket.onerror = onerror; - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { socket.on('open', () => { - resolve(new JSONRPCSocket(socket, responseTimeout)); + resolve(new JsonRpcClient(socket, responseTimeout)); }); setTimeout(() => reject, connectTimeout); @@ -91,7 +91,6 @@ export class JSONRPCSocket { */ subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): { close: () => void } { request.id ??= uuidv4(); - const messageHandler = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; if (jsonRpcResponse.id === request.id) { diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 18ba03e..5c58e5d 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -11,7 +11,7 @@ export type RequestContext = { socketConnection?: SocketConnection; /** The `MessageSubscriptionHandler` associated with a subscription request, only used in `ws` requests */ subscriptionHandler?: MessageSubscriptionHandler; - /** The `Readable` stream associated with a `RecordsWrite` request only used in `ws` requests */ + /** The `Readable` stream associated with a `RecordsWrite` request only used in `http` requests */ dataStream?: Readable; }; diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 65f64dd..db49741 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -4,7 +4,7 @@ import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json-rpc.js'; -import { JSONRPCSocket } from '../src/json-rpc-socket.js'; +import { JsonRpcClient } from '../src/json-rpc-socket.js'; import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; describe('JSONRPCSocket', () => { @@ -21,7 +21,7 @@ describe('JSONRPCSocket', () => { }); it('connects to a url', async () => { - const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); expect(wsServer.clients.size).to.equal(1); client.close(); @@ -44,7 +44,7 @@ describe('JSONRPCSocket', () => { }); }); - const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); const response = await client.request(request); @@ -53,7 +53,7 @@ describe('JSONRPCSocket', () => { it('request times out', async () => { // time out after 1 ms - const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); + const client = await JsonRpcClient.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); @@ -76,7 +76,7 @@ describe('JSONRPCSocket', () => { } }); }); - const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); @@ -108,11 +108,17 @@ describe('JSONRPCSocket', () => { }); }); }); - const client = await JSONRPCSocket.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); client.send(request); const resolved = await received; expect(resolved.id).to.equal(request.id); }); + + xit('calls onerror handler', async () => { + }); + + xit('calls onclose handler', async () => { + }); }); diff --git a/tests/utils.ts b/tests/utils.ts index 60b586f..714976f 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -13,7 +13,7 @@ import { WebSocket } from 'ws'; import type { JsonRpcResponse, JsonRpcRequest, JsonRpcId } from '../src/lib/json-rpc.js'; import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; -import { JSONRPCSocket } from '../src/json-rpc-socket.js'; +import { JsonRpcClient } from '../src/json-rpc-socket.js'; // __filename and __dirname are not defined in ES module scope const __filename = fileURLToPath(import.meta.url); @@ -231,9 +231,9 @@ export async function subscriptionRequest( ): Promise<{ status: any, subscription?: { id: string, close: () => Promise } }> { let resolved: boolean = false; const { id: requestId } = request; - const connection = await JSONRPCSocket.connect(url); + const connection = await JsonRpcClient.connect(url); - const closeSubscription = async (id: JsonRpcId, connection: JSONRPCSocket): Promise => { + const closeSubscription = async (id: JsonRpcId, connection: JsonRpcClient): Promise => { const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'subscription.close', { id }); return await connection.request(request); From b2eacecadf8d99834f8ca0999ad4efc56836cffe Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 14 Feb 2024 20:52:24 -0500 Subject: [PATCH 23/40] update chai, added types for chai and sinon --- package-lock.json | 48 ++++++++++++++++--- package.json | 4 +- src/json-rpc-handlers/dwn/process-message.ts | 4 ++ tests/json-rpc-socket.spec.ts | 21 ++++---- .../proof-of-work-manager.spec.ts | 4 +- 5 files changed, 60 insertions(+), 21 deletions(-) diff --git a/package-lock.json b/package-lock.json index 45359d5..f111f15 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35,10 +35,12 @@ "devDependencies": { "@types/bytes": "3.1.1", "@types/chai": "4.3.4", + "@types/chai-as-promised": "7.1.5", "@types/express": "4.17.17", "@types/mocha": "10.0.1", "@types/node": "18.11.18", "@types/readable-stream": "4.0.6", + "@types/sinon": "17.0.3", "@types/supertest": "2.0.12", "@types/ws": "8.5.4", "@typescript-eslint/eslint-plugin": "5.59.0", @@ -62,7 +64,7 @@ "lint-staged": "^14.0.1", "mocha": "^10.2.0", "puppeteer": "^21.4.0", - "sinon": "16.1.0", + "sinon": "17.0.1", "stream-browserify": "^3.0.0", "supertest": "6.3.3", "typescript": "^5.1.6" @@ -743,6 +745,15 @@ "integrity": "sha512-KnRanxnpfpjUTqTCXslZSEdLfXExwgNxYPdiO2WGUj8+HDjFi8R3k5RVKPeSCzLjCcshCAtVO2QBbVuAV4kTnw==", "dev": true }, + "node_modules/@types/chai-as-promised": { + "version": "7.1.5", + "resolved": "https://registry.npmjs.org/@types/chai-as-promised/-/chai-as-promised-7.1.5.tgz", + "integrity": "sha512-jStwss93SITGBwt/niYrkf2C+/1KTeZCZl1LaeezTlqppAKeoQC7jxyqYuP72sxBGKCIbw7oHgbYssIRzT5FCQ==", + "dev": true, + "dependencies": { + "@types/chai": "*" + } + }, "node_modules/@types/connect": { "version": "3.4.38", "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.38.tgz", @@ -888,6 +899,21 @@ "@types/node": "*" } }, + "node_modules/@types/sinon": { + "version": "17.0.3", + "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-17.0.3.tgz", + "integrity": "sha512-j3uovdn8ewky9kRBG19bOwaZbexJu/XjtkHyjvUgt4xfPFz18dcORIMqnYh66Fx3Powhcr85NT5+er3+oViapw==", + "dev": true, + "dependencies": { + "@types/sinonjs__fake-timers": "*" + } + }, + "node_modules/@types/sinonjs__fake-timers": { + "version": "8.1.5", + "resolved": "https://registry.npmjs.org/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-8.1.5.tgz", + "integrity": "sha512-mQkU2jY8jJEF7YHjHvsQO8+3ughTL1mcnn96igfhONmR+fUPSKIkefQYpSe8bsly2Ep7oQbn/6VG5/9/0qcArQ==", + "dev": true + }, "node_modules/@types/superagent": { "version": "4.1.21", "resolved": "https://registry.npmjs.org/@types/superagent/-/superagent-4.1.21.tgz", @@ -8222,17 +8248,16 @@ } }, "node_modules/sinon": { - "version": "16.1.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-16.1.0.tgz", - "integrity": "sha512-ZSgzF0vwmoa8pq0GEynqfdnpEDyP1PkYmEChnkjW0Vyh8IDlyFEJ+fkMhCP0il6d5cJjPl2PUsnUSAuP5sttOQ==", - "deprecated": "16.1.1", + "version": "17.0.1", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-17.0.1.tgz", + "integrity": "sha512-wmwE19Lie0MLT+ZYNpDymasPHUKTaZHUH/pKEubRXIzySv9Atnlw+BUMGCzWgV7b7wO+Hw6f1TEOr0IUnmU8/g==", "dev": true, "dependencies": { "@sinonjs/commons": "^3.0.0", - "@sinonjs/fake-timers": "^10.3.0", + "@sinonjs/fake-timers": "^11.2.2", "@sinonjs/samsam": "^8.0.0", "diff": "^5.1.0", - "nise": "^5.1.4", + "nise": "^5.1.5", "supports-color": "^7.2.0" }, "funding": { @@ -8240,6 +8265,15 @@ "url": "https://opencollective.com/sinon" } }, + "node_modules/sinon/node_modules/@sinonjs/fake-timers": { + "version": "11.2.2", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-11.2.2.tgz", + "integrity": "sha512-G2piCSxQ7oWOxwGSAyFHfPIsyeJGXYtc6mFbnFA+kRXkiEnTl8c/8jul2S329iFBnDI9HGoeWWAZvuvOkZccgw==", + "dev": true, + "dependencies": { + "@sinonjs/commons": "^3.0.0" + } + }, "node_modules/sinon/node_modules/diff": { "version": "5.1.0", "resolved": "https://registry.npmjs.org/diff/-/diff-5.1.0.tgz", diff --git a/package.json b/package.json index 9c5365d..fd60e4c 100644 --- a/package.json +++ b/package.json @@ -50,10 +50,12 @@ "devDependencies": { "@types/bytes": "3.1.1", "@types/chai": "4.3.4", + "@types/chai-as-promised": "7.1.5", "@types/express": "4.17.17", "@types/mocha": "10.0.1", "@types/node": "18.11.18", "@types/readable-stream": "4.0.6", + "@types/sinon": "17.0.3", "@types/supertest": "2.0.12", "@types/ws": "8.5.4", "@typescript-eslint/eslint-plugin": "5.59.0", @@ -77,7 +79,7 @@ "lint-staged": "^14.0.1", "mocha": "^10.2.0", "puppeteer": "^21.4.0", - "sinon": "16.1.0", + "sinon": "17.0.1", "stream-browserify": "^3.0.0", "supertest": "6.3.3", "typescript": "^5.1.6" diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 950e6fe..21158ac 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -15,6 +15,7 @@ import { createJsonRpcSuccessResponse, JsonRpcErrorCodes, } from '../../lib/json-rpc.js'; +import log from 'loglevel'; export const handleDwnProcessMessage: JsonRpcHandler = async ( dwnRequest, @@ -104,6 +105,9 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( e.message, ); + // log the error response + log.error('handleDwnProcessMessage error', jsonRpcResponse); + return { jsonRpcResponse } as HandlerResponse; } }; diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index db49741..8548de7 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -1,4 +1,6 @@ -import { expect } from 'chai'; +import chaiAsPromised from 'chai-as-promised'; +import chai, { expect } from 'chai'; + import { v4 as uuidv4 } from 'uuid'; import { WebSocketServer } from 'ws'; @@ -7,6 +9,8 @@ import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json import { JsonRpcClient } from '../src/json-rpc-socket.js'; import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; +chai.use(chaiAsPromised); + describe('JSONRPCSocket', () => { let wsServer: WebSocketServer; @@ -56,13 +60,9 @@ describe('JSONRPCSocket', () => { const client = await JsonRpcClient.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + const requestPromise = client.request(request); - try { - await client.request(request); - } catch (error) { - expect(error).to.not.be.undefined; - expect((error as Error).message).to.include('timed out'); - } + await expect(requestPromise).to.eventually.be.rejectedWith('timed out'); }); it('opens a subscription', async () => { @@ -97,14 +97,14 @@ describe('JSONRPCSocket', () => { }); it('sends message', async () => { - const received = new Promise<{ id?: JsonRpcId }>((resolve) => { + const receivedPromise = new Promise<{ reply: { id?: JsonRpcId }}>((resolve) => { wsServer.addListener('connection', (socket) => { socket.on('message', (dataBuffer: Buffer) => { const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; const { param1, param2 } = request.params; expect(param1).to.equal('test-param1'); expect(param2).to.equal('test-param2'); - resolve(request); + resolve({ reply: { id: request.id }}); }); }); }); @@ -112,8 +112,7 @@ describe('JSONRPCSocket', () => { const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); client.send(request); - const resolved = await received; - expect(resolved.id).to.equal(request.id); + await expect(receivedPromise).to.eventually.eql({ reply: { id: request.id }}); }); xit('calls onerror handler', async () => { diff --git a/tests/registration/proof-of-work-manager.spec.ts b/tests/registration/proof-of-work-manager.spec.ts index 06354a2..986b6df 100644 --- a/tests/registration/proof-of-work-manager.spec.ts +++ b/tests/registration/proof-of-work-manager.spec.ts @@ -46,8 +46,8 @@ describe('ProofOfWorkManager', function () { } }; - const challengeNonceRefreshSpy = sinon.stub(proofOfWorkManager, 'refreshChallengeNonce').callsFake(stub); - const maximumAllowedHashValueRefreshSpy = sinon.stub(proofOfWorkManager, 'refreshMaximumAllowedHashValue').callsFake(stub); + const challengeNonceRefreshSpy = sinon.stub(proofOfWorkManager as any, 'refreshChallengeNonce').callsFake(stub); + const maximumAllowedHashValueRefreshSpy = sinon.stub(proofOfWorkManager as any, 'refreshMaximumAllowedHashValue').callsFake(stub); clock.tick(60 * 60 * 1000); From e12028f818d06a6af037c4ca0363b14106e028a1 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 15 Feb 2024 10:04:35 -0500 Subject: [PATCH 24/40] address review suggestions --- src/connection/socket-connection.ts | 11 ++++++----- src/json-rpc-handlers/dwn/process-message.ts | 2 +- src/json-rpc-socket.ts | 7 ++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 51eff9e..f0fc64b 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -58,7 +58,7 @@ export class SocketConnection { * Adds a reference for the JSON RPC Subscription to this connection. * Used for cleanup if the connection is closed. */ - async subscribe(subscription: JsonRpcSubscription): Promise { + async addSubscription(subscription: JsonRpcSubscription): Promise { if (this.subscriptions.has(subscription.id)) { throw new DwnServerError( DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdExists, @@ -178,10 +178,11 @@ export class SocketConnection { } /** - * Subscription Handler used to build the context for a `JSON RPC` API call. + * Creates a subscription handler to send messages matching the subscription requested. + * * Wraps the incoming `message` in a `JSON RPC Success Response` using the original subscription`JSON RPC Id` to send through the WebSocket. */ - private subscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { + private createSubscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { return (message) => { const response = createJsonRpcSuccessResponse(id, { reply: { record : message @@ -196,7 +197,7 @@ export class SocketConnection { * Adds a `subscriptionHandler` for `Subscribe` messages. */ private async buildRequestContext(request: JsonRpcRequest): Promise { - const { id, params, method} = request; + const { id, params, method } = request; const requestContext: RequestContext = { transport : 'ws', dwn : this.dwn, @@ -206,7 +207,7 @@ export class SocketConnection { if (method === 'dwn.processMessage') { const { message } = params as { message: GenericMessage }; if (message.descriptor.method === DwnMethodName.Subscribe) { - requestContext.subscriptionHandler = this.subscriptionHandler(id).bind(this); + requestContext.subscriptionHandler = this.createSubscriptionHandler(id).bind(this); } } diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 21158ac..2f88c6f 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -71,7 +71,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( try { // adding a reference to the close function for this subscription request to the connection. // this will facilitate closing the subscription later. - await socketConnection.subscribe({ id: requestId, close }); + await socketConnection.addSubscription({ id: requestId, close }); delete reply.subscription.close // not serializable via JSON } catch(error) { // close the subscription upon receiving an error here diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 480d58c..78a5b03 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -8,7 +8,7 @@ import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; const CONNECT_TIMEOUT = 3_000; const RESPONSE_TIMEOUT = 30_000; -export interface JSONRPCSocketOptions { +export interface JsonRpcSocketOptions { /** socket connection timeout in milliseconds */ connectTimeout?: number; /** response timeout for rpc requests in milliseconds */ @@ -25,7 +25,7 @@ export interface JSONRPCSocketOptions { export class JsonRpcClient { private constructor(private socket: WebSocket, private responseTimeout: number) {} - static async connect(url: string, options: JSONRPCSocketOptions = {}): Promise { + static async connect(url: string, options: JsonRpcSocketOptions = {}): Promise { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; const socket = new WebSocket(url); @@ -115,6 +115,7 @@ export class JsonRpcClient { * Sends a JSON-RPC request through the socket. You must subscribe to a message listener separately to capture the response. */ send(request: JsonRpcRequest):void { - return this.socket.send(Buffer.from(JSON.stringify(request))); + this.socket.send(Buffer.from(JSON.stringify(request))); + return; } } \ No newline at end of file From 97a1ef9f0d584baccfa37beaaf07233b3a6b27f1 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 15 Feb 2024 18:28:45 -0500 Subject: [PATCH 25/40] review comments --- src/connection/socket-connection.ts | 6 ++---- src/json-rpc-socket.ts | 10 +++++----- src/lib/json-rpc-router.ts | 6 +++--- tests/dwn-process-message.spec.ts | 2 ++ tests/http-api.spec.ts | 1 + tests/json-rpc-socket.spec.ts | 12 ++++++------ tests/utils.ts | 28 ++++++++++++---------------- tests/ws-api.spec.ts | 28 +++++++++++++++++----------- 8 files changed, 48 insertions(+), 45 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index f0fc64b..0567daf 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -183,10 +183,8 @@ export class SocketConnection { * Wraps the incoming `message` in a `JSON RPC Success Response` using the original subscription`JSON RPC Id` to send through the WebSocket. */ private createSubscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { - return (message) => { - const response = createJsonRpcSuccessResponse(id, { reply: { - record : message - } }); + return (event) => { + const response = createJsonRpcSuccessResponse(id, { reply: { event } }); this.send(response); } } diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 78a5b03..826a353 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -20,12 +20,12 @@ export interface JsonRpcSocketOptions { } /** - * JSONRPC Socket Client for WebSocket request/response and long-running subscriptions + * JSON RPC Socket Client for WebSocket request/response and long-running subscriptions. */ -export class JsonRpcClient { +export class JsonRpcSocket { private constructor(private socket: WebSocket, private responseTimeout: number) {} - static async connect(url: string, options: JsonRpcSocketOptions = {}): Promise { + static async connect(url: string, options: JsonRpcSocketOptions = {}): Promise { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; const socket = new WebSocket(url); @@ -44,9 +44,9 @@ export class JsonRpcClient { socket.onclose = onclose; socket.onerror = onerror; - return new Promise((resolve, reject) => { + return new Promise((resolve, reject) => { socket.on('open', () => { - resolve(new JsonRpcClient(socket, responseTimeout)); + resolve(new JsonRpcSocket(socket, responseTimeout)); }); setTimeout(() => reject, connectTimeout); diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 5c58e5d..ec63cee 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -1,4 +1,4 @@ -import type { Dwn, MessageSubscriptionHandler } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; @@ -9,8 +9,8 @@ export type RequestContext = { transport: 'http' | 'ws'; dwn: Dwn; socketConnection?: SocketConnection; - /** The `MessageSubscriptionHandler` associated with a subscription request, only used in `ws` requests */ - subscriptionHandler?: MessageSubscriptionHandler; + /** The `GenericMessage` handler associated with a subscription request, only used in `ws` requests */ + subscriptionHandler?: (message: GenericMessage) => void; /** The `Readable` stream associated with a `RecordsWrite` request only used in `http` requests */ dataStream?: Readable; }; diff --git a/tests/dwn-process-message.spec.ts b/tests/dwn-process-message.spec.ts index 1447eb8..5249da2 100644 --- a/tests/dwn-process-message.spec.ts +++ b/tests/dwn-process-message.spec.ts @@ -32,6 +32,7 @@ describe('handleDwnProcessMessage', function () { const { reply } = jsonRpcResponse.result; expect(reply.status.code).to.equal(202); expect(reply.status.detail).to.equal('Accepted'); + await dwn.close(); }); it('returns a JSON RPC Success Response when DWN returns a 4XX/5XX status code', async function () { @@ -59,5 +60,6 @@ describe('handleDwnProcessMessage', function () { expect(reply.status.detail).to.exist; expect(reply.data).to.be.undefined; expect(reply.entries).to.be.undefined; + await dwn.close(); }); }); diff --git a/tests/http-api.spec.ts b/tests/http-api.spec.ts index a64121e..1b391a7 100644 --- a/tests/http-api.spec.ts +++ b/tests/http-api.spec.ts @@ -228,6 +228,7 @@ describe('http api', function () { expect(body.error).to.not.exist; const { reply } = body.result; + console.log('reply', reply); expect(reply.status.code).to.equal(202); }); diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 8548de7..69c944f 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -6,7 +6,7 @@ import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json-rpc.js'; -import { JsonRpcClient } from '../src/json-rpc-socket.js'; +import { JsonRpcSocket } from '../src/json-rpc-socket.js'; import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; chai.use(chaiAsPromised); @@ -25,7 +25,7 @@ describe('JSONRPCSocket', () => { }); it('connects to a url', async () => { - const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); expect(wsServer.clients.size).to.equal(1); client.close(); @@ -48,7 +48,7 @@ describe('JSONRPCSocket', () => { }); }); - const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); const response = await client.request(request); @@ -57,7 +57,7 @@ describe('JSONRPCSocket', () => { it('request times out', async () => { // time out after 1 ms - const client = await JsonRpcClient.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 1 }); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); const requestPromise = client.request(request); @@ -76,7 +76,7 @@ describe('JSONRPCSocket', () => { } }); }); - const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); @@ -108,7 +108,7 @@ describe('JSONRPCSocket', () => { }); }); }); - const client = await JsonRpcClient.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); client.send(request); diff --git a/tests/utils.ts b/tests/utils.ts index 714976f..1e1dda3 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -1,4 +1,4 @@ -import type { GenericMessage, MessageSubscriptionHandler, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; +import type { GenericMessage, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; import { Cid, DataStream, RecordsWrite } from '@tbd54566975/dwn-sdk-js'; import type { ReadStream } from 'node:fs'; @@ -13,7 +13,7 @@ import { WebSocket } from 'ws'; import type { JsonRpcResponse, JsonRpcRequest, JsonRpcId } from '../src/lib/json-rpc.js'; import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; -import { JsonRpcClient } from '../src/json-rpc-socket.js'; +import { JsonRpcSocket } from '../src/json-rpc-socket.js'; // __filename and __dirname are not defined in ES module scope const __filename = fileURLToPath(import.meta.url); @@ -227,13 +227,12 @@ const MAX_RESPONSE_TIMEOUT = 1_500; export async function subscriptionRequest( url: string, request: JsonRpcRequest, - messageHandler: MessageSubscriptionHandler + messageHandler: (message: GenericMessage) => void, ): Promise<{ status: any, subscription?: { id: string, close: () => Promise } }> { - let resolved: boolean = false; const { id: requestId } = request; - const connection = await JsonRpcClient.connect(url); + const connection = await JsonRpcSocket.connect(url); - const closeSubscription = async (id: JsonRpcId, connection: JsonRpcClient): Promise => { + const closeSubscription = async (id: JsonRpcId, connection: JsonRpcSocket): Promise => { const requestId = uuidv4(); const request = createJsonRpcRequest(requestId, 'subscription.close', { id }); return await connection.request(request); @@ -250,13 +249,13 @@ export async function subscriptionRequest( } // at this point the reply should be DwnRpcResponse - const { status, record, subscription } = result.reply; - if (record) { - messageHandler(record); + const { status, event, subscription } = result.reply; + if (event) { + messageHandler(event); return; } + if (subscription) { - resolved = true; resolve({ status, subscription: { @@ -270,15 +269,12 @@ export async function subscriptionRequest( } } }) - } else { - resolve({ status }); - } + } + + resolve({ status }); }); setTimeout(() => { - if (resolved) { - return; - }; return reject('subscription request timeout'); }, MAX_RESPONSE_TIMEOUT); }); diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 0eb33d0..58a989f 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,6 +1,9 @@ + import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; import { DataStream, Message, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; +import type { Server } from 'http'; + import { expect } from 'chai'; import { base64url } from 'multiformats/bases/base64'; import { v4 as uuidv4 } from 'uuid'; @@ -15,23 +18,26 @@ import { getTestDwn } from './test-dwn.js'; import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest } from './utils.js'; import { HttpApi } from '../src/http-api.js'; -let wsApi: WsApi; -let dwn: Dwn; describe('websocket api', function () { - before(async function () { - dwn = await getTestDwn({ withEvents: true }); + let server: Server; + let httpApi: HttpApi; + let wsApi: WsApi; + let dwn: Dwn; - // set up http api for issuing writes within the tests - const httpApi = new HttpApi(config, dwn); - await httpApi.start(9002); - - wsApi = new WsApi(httpApi.server, dwn); + beforeEach(async function () { + dwn = await getTestDwn({ withEvents: true }); + httpApi = new HttpApi(config, dwn); + server = await httpApi.start(9002); + wsApi = new WsApi(server, dwn); wsApi.start(); }); - after(function () { - wsApi.close(); + afterEach(async function () { + await wsApi.close(); + server.close(); + server.closeAllConnections(); + await dwn.close(); }); it('returns an error response if no request payload is provided', async function () { From 1c9adee17e2ff23aa2a8e73faff8d988e19e18d5 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 15 Feb 2024 18:37:16 -0500 Subject: [PATCH 26/40] match class name in test --- tests/json-rpc-socket.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 69c944f..b0177d6 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -11,7 +11,7 @@ import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/j chai.use(chaiAsPromised); -describe('JSONRPCSocket', () => { +describe('JsonRpcSocket', () => { let wsServer: WebSocketServer; before(async () => { From d69903529b67191d761a436fa4fed4952924a793 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Fri, 16 Feb 2024 17:04:27 -0500 Subject: [PATCH 27/40] fix tests --- tests/dwn-server.spec.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/dwn-server.spec.ts b/tests/dwn-server.spec.ts index 0107e51..9c79327 100644 --- a/tests/dwn-server.spec.ts +++ b/tests/dwn-server.spec.ts @@ -1,3 +1,5 @@ +import type { Dwn } from '@tbd54566975/dwn-sdk-js'; + import { expect } from 'chai'; import { config } from '../src/config.js'; @@ -6,12 +8,12 @@ import { getTestDwn } from './test-dwn.js'; describe('DwnServer', function () { const dwnServerConfig = { ...config }; + let dwn: Dwn; - it('starts with injected dwn', async function () { - const testDwn = await getTestDwn(); + dwn = await getTestDwn(); - const dwnServer = new DwnServer({ config: dwnServerConfig, dwn: testDwn }); + const dwnServer = new DwnServer({ config: dwnServerConfig, dwn }); await dwnServer.start(); dwnServer.stop(() => console.log('server Stop')); @@ -20,7 +22,9 @@ describe('DwnServer', function () { describe('webSocketServerEnabled config', function() { it('should not return a websocket server if disabled', async function() { + dwn = await getTestDwn({ withEvents: true }); const withoutSocketServer = new DwnServer({ + dwn, config: { ...dwnServerConfig, webSocketServerEnabled: false, @@ -35,7 +39,9 @@ describe('DwnServer', function () { }); it('should return a websocket server if enabled', async function() { + dwn = await getTestDwn({ withEvents: true }); const withSocketServer = new DwnServer({ + dwn, config: { ...dwnServerConfig, webSocketServerEnabled: true, From 16694f27b62224b8989a976ab840187c17c879f4 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 21 Feb 2024 15:47:23 -0500 Subject: [PATCH 28/40] update jrpc subscription support - more error proof and concise --- src/connection/socket-connection.ts | 31 ++-- src/json-rpc-api.ts | 3 +- src/json-rpc-handlers/dwn/process-message.ts | 67 ++++---- src/json-rpc-socket.ts | 62 ++++++-- src/lib/json-rpc-router.ts | 12 +- src/lib/json-rpc.ts | 8 +- tests/http-api.spec.ts | 1 - tests/json-rpc-socket.spec.ts | 22 ++- tests/utils.ts | 83 ++++------ tests/ws-api.spec.ts | 159 ++++++++++++++++--- 10 files changed, 300 insertions(+), 148 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 0567daf..e890075 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -1,4 +1,4 @@ -import type { Dwn, GenericMessage } from "@tbd54566975/dwn-sdk-js"; +import type { Dwn, GenericMessage, MessageEvent } from "@tbd54566975/dwn-sdk-js"; import { DwnMethodName } from "@tbd54566975/dwn-sdk-js"; import type { WebSocket } from "ws"; @@ -6,7 +6,7 @@ import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; import type { RequestContext } from "../lib/json-rpc-router.js"; -import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse } from "../lib/json-rpc.js"; +import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse, JsonRpcSubscription } from "../lib/json-rpc.js"; import { requestCounter } from "../metrics.js"; import { jsonRpcApi } from "../json-rpc-api.js"; @@ -15,12 +15,6 @@ import { DwnServerError, DwnServerErrorCode } from "../dwn-error.js"; const HEARTBEAT_INTERVAL = 30_000; -export interface JsonRpcSubscription { - /** JSON RPC Id of the Subscription Request */ - id: JsonRpcId; - close: () => Promise; -} - /** * SocketConnection handles a WebSocket connection to a DWN using JSON RPC. * It also manages references to the long running RPC subscriptions for the connection. @@ -54,6 +48,13 @@ export class SocketConnection { }, HEARTBEAT_INTERVAL); } + /** + * Checks to see if the incoming `JsonRpcId` is already in use for a subscription. + */ + hasSubscription(id: JsonRpcId): boolean { + return this.subscriptions.has(id); + } + /** * Adds a reference for the JSON RPC Subscription to this connection. * Used for cleanup if the connection is closed. @@ -182,7 +183,7 @@ export class SocketConnection { * * Wraps the incoming `message` in a `JSON RPC Success Response` using the original subscription`JSON RPC Id` to send through the WebSocket. */ - private createSubscriptionHandler(id: JsonRpcId): (message: GenericMessage) => void { + private createSubscriptionHandler(id: JsonRpcId): (message: MessageEvent) => void { return (event) => { const response = createJsonRpcSuccessResponse(id, { reply: { event } }); this.send(response); @@ -195,17 +196,23 @@ export class SocketConnection { * Adds a `subscriptionHandler` for `Subscribe` messages. */ private async buildRequestContext(request: JsonRpcRequest): Promise { - const { id, params, method } = request; + const { params, method } = request; + const { subscribe } = params.rpc || {}; + const requestContext: RequestContext = { transport : 'ws', dwn : this.dwn, socketConnection : this, } - if (method === 'dwn.processMessage') { + if (method.startsWith('rpc.subscribe.') && subscribe) { const { message } = params as { message: GenericMessage }; if (message.descriptor.method === DwnMethodName.Subscribe) { - requestContext.subscriptionHandler = this.createSubscriptionHandler(id).bind(this); + const handlerFunc = this.createSubscriptionHandler(subscribe); + requestContext.subscriptionRequest = { + id: subscribe, + subscriptionHandler: (message): void => handlerFunc(message), + } } } diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index 647ff21..cc1dc07 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -6,4 +6,5 @@ import { handleSubscriptionsClose } from './json-rpc-handlers/subscription/index export const jsonRpcApi = new JsonRpcRouter(); jsonRpcApi.on('dwn.processMessage', handleDwnProcessMessage); -jsonRpcApi.on('subscription.close', handleSubscriptionsClose); +jsonRpcApi.on('rpc.subscribe.dwn.processMessage', handleDwnProcessMessage); +jsonRpcApi.on('rpc.subscribe.close', handleSubscriptionsClose); diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 2f88c6f..0d419c7 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -2,26 +2,27 @@ import type { GenericMessage } from '@tbd54566975/dwn-sdk-js'; import { DwnInterfaceName, DwnMethodName } from '@tbd54566975/dwn-sdk-js'; import type { Readable as IsomorphicReadable } from 'readable-stream'; +import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; +import type { JsonRpcSubscription } from '../../lib/json-rpc.js'; import type { HandlerResponse, JsonRpcHandler, } from '../../lib/json-rpc-router.js'; -import { DwnServerErrorCode } from '../../dwn-error.js'; import { createJsonRpcErrorResponse, createJsonRpcSuccessResponse, JsonRpcErrorCodes, } from '../../lib/json-rpc.js'; -import log from 'loglevel'; + export const handleDwnProcessMessage: JsonRpcHandler = async ( dwnRequest, context, ) => { - const { dwn, dataStream, subscriptionHandler, socketConnection, transport } = context; + const { dwn, dataStream, subscriptionRequest, socketConnection, transport } = context; const { target, message } = dwnRequest.params as { target: string, message: GenericMessage }; const requestId = dwnRequest.id ?? uuidv4(); @@ -41,23 +42,41 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( return { jsonRpcResponse }; } + // subscribe methods must come with a subscriptionRequest context + if (message.descriptor.method === DwnMethodName.Subscribe && subscriptionRequest === undefined) { + const jsonRpcResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.InvalidRequest, + `subscribe methods must contain a subscriptionRequest context` + ); + return { jsonRpcResponse }; + } + // Subscribe methods are only supported on 'ws' (WebSockets) - if (transport !== 'ws' && message.descriptor.method === DwnMethodName.Subscribe) { + if (transport !== 'ws' && subscriptionRequest !== undefined) { const jsonRpcResponse = createJsonRpcErrorResponse( requestId, JsonRpcErrorCodes.InvalidParams, - `Subscribe not supported via ${context.transport}` + `subscriptions are not supported via ${context.transport}` + ) + return { jsonRpcResponse }; + } + + if (subscriptionRequest !== undefined && socketConnection?.hasSubscription(subscriptionRequest.id)) { + const jsonRpcResponse = createJsonRpcErrorResponse( + requestId, + JsonRpcErrorCodes.InvalidParams, + `the subscribe id: ${subscriptionRequest.id} is in use by an active subscription` ) return { jsonRpcResponse }; } const reply = await dwn.processMessage(target, message, { dataStream: dataStream as IsomorphicReadable, - subscriptionHandler, + subscriptionHandler: subscriptionRequest?.subscriptionHandler, }); - const { record, subscription } = reply; - + const { record } = reply; // RecordsRead messages return record data as a stream to for accommodate large amounts of data let recordDataStream: IsomorphicReadable; if (record !== undefined && record.data !== undefined) { @@ -66,29 +85,16 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( } // Subscribe messages return a close function to facilitate closing the subscription - if (subscription !== undefined) { - const { close } = subscription; - try { - // adding a reference to the close function for this subscription request to the connection. - // this will facilitate closing the subscription later. - await socketConnection.addSubscription({ id: requestId, close }); - delete reply.subscription.close // not serializable via JSON - } catch(error) { - // close the subscription upon receiving an error here - await close(); - if (error.code === DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdExists) { - // a subscription with this request id already exists - const errorResponse = createJsonRpcErrorResponse( - requestId, - JsonRpcErrorCodes.BadRequest, - `the request id ${requestId} already has an active subscription` - ); - return { jsonRpcResponse: errorResponse }; - } else { - // will catch as an unknown error below - throw new Error('unknown error adding subscription'); - } + if (subscriptionRequest && reply.subscription) { + const { close } = reply.subscription; + // we add a reference to the close function for this subscription request to the socket connection. + // this will facilitate closing the subscription later. + const subscriptionReply: JsonRpcSubscription = { + id: subscriptionRequest.id, + close, } + await socketConnection.addSubscription(subscriptionReply); + delete reply.subscription.close // delete the close method from the reply as it's not JSON serializable and has a held reference. } const jsonRpcResponse = createJsonRpcSuccessResponse(requestId, { reply }); @@ -107,7 +113,6 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( // log the error response log.error('handleDwnProcessMessage error', jsonRpcResponse); - return { jsonRpcResponse } as HandlerResponse; } }; diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 826a353..d84197d 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -3,6 +3,7 @@ import { v4 as uuidv4 } from 'uuid'; import WebSocket from 'ws'; import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; +import { createJsonRpcRequest } from "./lib/json-rpc.js"; // These were arbitrarily chosen, but can be modified via connect options const CONNECT_TIMEOUT = 3_000; @@ -89,26 +90,61 @@ export class JsonRpcSocket { * Sends a JSON-RPC request through the socket and keeps a listener open to read associated responses as they arrive. * Returns a close method to clean up the listener. */ - subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): { close: () => void } { - request.id ??= uuidv4(); + async subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): Promise<{ + response: JsonRpcResponse; + close?: () => Promise; + }> { + + if (!request.method.startsWith('rpc.subscribe.')) { + throw new Error('subscribe rpc messages must include the `rpc.subscribe` prefix'); + } + + // extract optional `rpc.subscribe` param + const { rpc } = request.params; + const { subscribe } = rpc || {}; + const subscriptionId = subscribe || uuidv4(); + + // When subscribing to a JSON RPC Message, we want to generate the subscription update Json PRC Id ahead of time and create a listener. + // We then set the subscription Id within a special rpc.subscribe params namespace preserving any other properties + request.params.rpc = { + ...rpc, + subscribe: subscriptionId, + }; + const messageHandler = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; - if (jsonRpcResponse.id === request.id) { - // if the incoming response id matches the request id, trigger the listener - return listener(jsonRpcResponse); + if (jsonRpcResponse.id === subscriptionId) { + if (jsonRpcResponse.error !== undefined) { + // remove the event listener upon receipt of a JSON RPC Error. + this.socket.removeEventListener('message', messageHandler); + } + + listener(jsonRpcResponse); } }; - - // subscribe to the listener before sending the request this.socket.addEventListener('message', messageHandler); - this.send(request); - return { - close: ():void => { - // removes the listener for this particular request - this.socket.removeEventListener('message', messageHandler); + const response = await this.request(request); + if (response.error) { + this.socket.removeEventListener('message', messageHandler); + return { response } + } + + // clean up listener and create a `rpc.subscribe.close` message to use when closing this JSON RPC subscription + const close = async (): Promise => { + this.socket.removeEventListener('message', messageHandler); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'rpc.subscribe.close', { id: subscriptionId }); + const response = await this.request(request); + if (response.error) { + throw response.error; } - }; + } + + return { + response, + close + } } /** diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index ec63cee..580d375 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -1,16 +1,20 @@ -import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, MessageEvent } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; -import type { JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; +import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from './json-rpc.js'; import type { SocketConnection } from '../connection/socket-connection.js'; export type RequestContext = { transport: 'http' | 'ws'; dwn: Dwn; socketConnection?: SocketConnection; - /** The `GenericMessage` handler associated with a subscription request, only used in `ws` requests */ - subscriptionHandler?: (message: GenericMessage) => void; + subscriptionRequest?: { + /** The JsonRpcId of the subscription handler */ + id: JsonRpcId; + /** The `MessageEvent` handler associated with a subscription request, only used in `ws` requests */ + subscriptionHandler: (message: MessageEvent) => void; + } /** The `Readable` stream associated with a `RecordsWrite` request only used in `http` requests */ dataStream?: Readable; }; diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index fbffd99..a460873 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -15,6 +15,12 @@ export interface JsonRpcError { data?: any; } +export interface JsonRpcSubscription { + /** JSON RPC Id of the Subscription Request */ + id: JsonRpcId; + close: () => Promise; +} + export enum JsonRpcErrorCodes { // JSON-RPC 2.0 pre-defined errors InvalidRequest = -32600, @@ -37,7 +43,7 @@ export interface JsonRpcSuccessResponse { jsonrpc: JsonRpcVersion; id: JsonRpcId; result: any; - error?: undefined; + error?: never; } export interface JsonRpcErrorResponse { diff --git a/tests/http-api.spec.ts b/tests/http-api.spec.ts index 1b391a7..a64121e 100644 --- a/tests/http-api.spec.ts +++ b/tests/http-api.spec.ts @@ -228,7 +228,6 @@ describe('http api', function () { expect(body.error).to.not.exist; const { reply } = body.result; - console.log('reply', reply); expect(reply.status.code).to.equal(202); }); diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index b0177d6..1279aa4 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -4,7 +4,7 @@ import chai, { expect } from 'chai'; import { v4 as uuidv4 } from 'uuid'; import { WebSocketServer } from 'ws'; -import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from '../src/lib/json-rpc.js'; +import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; import { JsonRpcSocket } from '../src/json-rpc-socket.js'; import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; @@ -69,26 +69,34 @@ describe('JsonRpcSocket', () => { wsServer.addListener('connection', (socket) => { socket.on('message', (dataBuffer: Buffer) => { const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; + // initial response + const response = createJsonRpcSuccessResponse(request.id, { reply: {} }) + socket.send(Buffer.from(JSON.stringify(response))); + + const { params } = request; + const { subscribe } = params.rpc || {}; // send 3 messages for (let i = 0; i < 3; i++) { - const response = createJsonRpcSuccessResponse(request.id, { count: i }); + const response = createJsonRpcSuccessResponse(subscribe, { count: i }); socket.send(Buffer.from(JSON.stringify(response))); } }); }); - const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); const requestId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + const subscribeId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'rpc.subscribe.test.method', { param1: 'test-param1', param2: 'test-param2', rpc: { subscribe: subscribeId } }); let responseCounter = 0; - const responseListener = (response: JsonRpcResponse): void => { - expect(response.id).to.equal(request.id); + const responseListener = (response: JsonRpcSuccessResponse): void => { + expect(response.id).to.equal(subscribeId); const { count } = response.result; expect(count).to.equal(responseCounter); responseCounter++; } - const subscription = client.subscribe(request, responseListener); + const subscription = await client.subscribe(request, responseListener); + expect(subscription.response.error).to.be.undefined; // wait for the messages to arrive await new Promise((resolve) => setTimeout(resolve, 5)); // the original response diff --git a/tests/utils.ts b/tests/utils.ts index 1e1dda3..7655a57 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -1,4 +1,4 @@ -import type { GenericMessage, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; +import type { GenericMessage, MessageEvent, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; import { Cid, DataStream, RecordsWrite } from '@tbd54566975/dwn-sdk-js'; import type { ReadStream } from 'node:fs'; @@ -11,7 +11,7 @@ import type { Readable } from 'readable-stream'; import { fileURLToPath } from 'url'; import { WebSocket } from 'ws'; -import type { JsonRpcResponse, JsonRpcRequest, JsonRpcId } from '../src/lib/json-rpc.js'; +import type { JsonRpcResponse, JsonRpcRequest } from '../src/lib/json-rpc.js'; import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; import { JsonRpcSocket } from '../src/json-rpc-socket.js'; @@ -222,60 +222,33 @@ export async function sendWsMessage( }); } -const MAX_RESPONSE_TIMEOUT = 1_500; - -export async function subscriptionRequest( - url: string, +export async function sendWsRequest(options: { + url?: string, + connection?: JsonRpcSocket, request: JsonRpcRequest, - messageHandler: (message: GenericMessage) => void, -): Promise<{ status: any, subscription?: { id: string, close: () => Promise } }> { - const { id: requestId } = request; - const connection = await JsonRpcSocket.connect(url); - - const closeSubscription = async (id: JsonRpcId, connection: JsonRpcSocket): Promise => { - const requestId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'subscription.close', { id }); - return await connection.request(request); - } - - return new Promise<{ status: any, subscription?: { id: string, close: () => Promise } }>((resolve, reject) => { - const { close: subscriptionClose } = connection.subscribe(request, (response) => { - const { result, error } = response; - - // this is an error specific to the `JsonRpcRequest` requesting the subscription - if (error) { - reject(error); - return; - } - - // at this point the reply should be DwnRpcResponse - const { status, event, subscription } = result.reply; - if (event) { - messageHandler(event); - return; - } - - if (subscription) { - resolve({ - status, - subscription: { - ...subscription, - close: async (): Promise => { - subscriptionClose(); - const closeResponse = await closeSubscription(requestId, connection); - if (closeResponse.error?.message !== undefined) { - throw new Error(`unable to close subscription: ${closeResponse.error.message}`); - } - } - } - }) - } - - resolve({ status }); - }); + responseTimeout?: number, +}): Promise { + const { url, connection: incomingConnection , request, responseTimeout } = options; + const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); + return connection.request(request); +} - setTimeout(() => { - return reject('subscription request timeout'); - }, MAX_RESPONSE_TIMEOUT); +export async function subscriptionRequest(options: { + url?: string, + connection?: JsonRpcSocket, + request: JsonRpcRequest, + messageHandler: (event: MessageEvent) => void, + responseTimeout?: number; +}): Promise<{ close?: () => Promise, response: JsonRpcResponse, connection?: JsonRpcSocket }> { + const { url, connection: incomingConnection, request, messageHandler, responseTimeout } = options; + const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); + + const { close, response } = await connection.subscribe(request, (response) => { + const { event } = response.result.reply; + messageHandler(event); }); + + return { + response, close, connection + } } diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 58a989f..486b5fd 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -1,11 +1,13 @@ -import type { Dwn, GenericMessage } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, MessageEvent } from '@tbd54566975/dwn-sdk-js'; import { DataStream, Message, TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; import type { Server } from 'http'; import { expect } from 'chai'; import { base64url } from 'multiformats/bases/base64'; +import type { SinonFakeTimers } from 'sinon'; +import { useFakeTimers } from 'sinon'; import { v4 as uuidv4 } from 'uuid'; import { @@ -15,7 +17,7 @@ import { import { config } from '../src/config.js'; import { WsApi } from '../src/ws-api.js'; import { getTestDwn } from './test-dwn.js'; -import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest } from './utils.js'; +import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest, sendWsRequest } from './utils.js'; import { HttpApi } from '../src/http-api.js'; @@ -24,6 +26,15 @@ describe('websocket api', function () { let httpApi: HttpApi; let wsApi: WsApi; let dwn: Dwn; + let clock: SinonFakeTimers; + + before(() => { + clock = useFakeTimers({ shouldAdvanceTime: true }); + }); + + after(() => { + clock.restore(); + }); beforeEach(async function () { dwn = await getTestDwn({ withEvents: true }); @@ -73,15 +84,14 @@ describe('websocket api', function () { encodedData, }); - const data = await sendWsMessage( - 'ws://127.0.0.1:9002', - JSON.stringify(dwnRequest), - ); - const resp = JSON.parse(data.toString()); - expect(resp.id).to.equal(requestId); - expect(resp.error).to.not.be.undefined; - expect(resp.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); - expect(resp.error.message).to.include('RecordsWrite is not supported via ws'); + const response = await sendWsRequest({ + url:'ws://127.0.0.1:9002', + request: dwnRequest, + }); + expect(response.id).to.equal(requestId); + expect(response.error).to.not.be.undefined; + expect(response.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); + expect(response.error.message).to.include('RecordsWrite is not supported via ws'); }); it('subscribes to records and receives updates', async () => { @@ -95,19 +105,25 @@ describe('websocket api', function () { }); const records: string[] = []; - const subscriptionHandler = async (message: GenericMessage): Promise => { + const subscriptionHandler = async (event: MessageEvent): Promise => { + const { message } = event records.push(await Message.getCid(message)); }; const requestId = uuidv4(); - const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, }); - const response = await subscriptionRequest('ws://127.0.0.1:9002', dwnRequest, subscriptionHandler); - expect(response.status.code).to.equal(200); - expect(response.subscription).to.not.be.undefined; + const { response, close } = await subscriptionRequest({ + url : 'ws://127.0.0.1:9002', + request : dwnRequest, + messageHandler : subscriptionHandler + }); + expect(response.error).to.be.undefined; + expect(response.result.reply.status.code).to.equal(200); + expect(close).to.not.be.undefined; const write1Message = await TestDataGenerator.generateRecordsWrite({ author : alice, @@ -138,7 +154,7 @@ describe('websocket api', function () { expect(writeResult2.status.code).to.equal(202); // close the subscription - await response.subscription.close(); + await close(); await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed expect(records).to.have.members([ @@ -158,18 +174,25 @@ describe('websocket api', function () { }); const records: string[] = []; - const subscriptionHandler = async (message: GenericMessage): Promise => { + const subscriptionHandler = async (event: MessageEvent): Promise => { + const { message } = event; records.push(await Message.getCid(message)); }; const requestId = uuidv4(); - const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, }); - const response = await subscriptionRequest('ws://127.0.0.1:9002', dwnRequest, subscriptionHandler); - expect(response.status.code).to.equal(200); - expect(response.subscription).to.not.be.undefined; + + const { response, close } = await subscriptionRequest({ + url : 'ws://127.0.0.1:9002', + request : dwnRequest, + messageHandler : subscriptionHandler + }); + expect(response.error).to.be.undefined; + expect(response.result.reply.status.code).to.equal(200); + expect(close).to.not.be.undefined; const write1Message = await TestDataGenerator.generateRecordsWrite({ author : alice, @@ -186,7 +209,7 @@ describe('websocket api', function () { expect(writeResult1.status.code).to.equal(202); // close the subscription after only 1 message - await response.subscription.close(); + await close(); // write more messages that won't show up in the subscription const write2Message = await TestDataGenerator.generateRecordsWrite({ @@ -220,4 +243,94 @@ describe('websocket api', function () { await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed expect(records).to.have.members([ await Message.getCid(write1Message.message) ]); }); + + it('should fail to add subscription using a `JsonRpcId` that already exists for a subscription in that socket', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + const { message } = await TestDataGenerator.generateRecordsSubscribe({ + author: alice, + filter: { + schema: 'foo/bar' + } + }); + + const records: string[] = []; + const subscriptionHandler = async (event: MessageEvent): Promise => { + const { message } = event + records.push(await Message.getCid(message)); + }; + + const requestId = uuidv4(); + const subscribeId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + message: message, + target: alice.did, + rpc: { subscribe: subscribeId } + }); + + const { response, close, connection } = await subscriptionRequest({ + url : 'ws://127.0.0.1:9002', + request : dwnRequest, + messageHandler : subscriptionHandler + }); + expect(response.error).to.be.undefined; + expect(response.result.reply.status.code).to.equal(200); + expect(close).to.not.be.undefined; + + + const { message: message2 } = await TestDataGenerator.generateRecordsSubscribe({ filter: { schema: 'bar/baz' }, author: alice }); + + // We are checking for the subscription Id not the request Id + const request2Id = uuidv4(); + const dwnRequest2 = createJsonRpcRequest(request2Id, 'rpc.subscribe.dwn.processMessage', { + message: message2, + target: alice.did, + rpc: { subscribe: subscribeId } + }); + + const { response: response2 } = await subscriptionRequest({ + connection, + request: dwnRequest2, + messageHandler: subscriptionHandler, + }); + expect(response2.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); + expect(response2.error.message).to.contain(`${subscribeId} is in use by an active subscription`); + + const write1Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult1 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write1Message.message, + data : write1Message.dataBytes, + }); + expect(writeResult1.status.code).to.equal(202); + + const write2Message = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult2 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : write2Message.message, + data : write2Message.dataBytes, + }) + expect(writeResult2.status.code).to.equal(202); + + // close the subscription + await close(); + + await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed + expect(records).to.have.members([ + await Message.getCid(write1Message.message), + await Message.getCid(write2Message.message) + ]); + }); }); From fcdf96033082efcbbcb3550f05e0b3d038528020 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Wed, 21 Feb 2024 20:43:49 -0500 Subject: [PATCH 29/40] added `rpc.subscribe.close` method and handling --- src/connection/socket-connection.ts | 11 +++++----- src/json-rpc-api.ts | 1 + src/json-rpc-handlers/subscription/close.ts | 2 +- src/json-rpc-socket.ts | 22 +++++++------------- src/lib/json-rpc.ts | 23 +++++++++++++++++++++ tests/json-rpc-socket.spec.ts | 17 ++++++++------- tests/utils.ts | 3 +++ tests/ws-api.spec.ts | 17 +++++++-------- 8 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index e890075..1178506 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -196,8 +196,7 @@ export class SocketConnection { * Adds a `subscriptionHandler` for `Subscribe` messages. */ private async buildRequestContext(request: JsonRpcRequest): Promise { - const { params, method } = request; - const { subscribe } = params.rpc || {}; + const { params, method, subscribe } = request; const requestContext: RequestContext = { transport : 'ws', @@ -206,11 +205,11 @@ export class SocketConnection { } if (method.startsWith('rpc.subscribe.') && subscribe) { - const { message } = params as { message: GenericMessage }; - if (message.descriptor.method === DwnMethodName.Subscribe) { - const handlerFunc = this.createSubscriptionHandler(subscribe); + const { message } = params as { message?: GenericMessage }; + if (message?.descriptor.method === DwnMethodName.Subscribe) { + const handlerFunc = this.createSubscriptionHandler(subscribe.id); requestContext.subscriptionRequest = { - id: subscribe, + id: subscribe.id, subscriptionHandler: (message): void => handlerFunc(message), } } diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index cc1dc07..bb96f29 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -7,4 +7,5 @@ export const jsonRpcApi = new JsonRpcRouter(); jsonRpcApi.on('dwn.processMessage', handleDwnProcessMessage); jsonRpcApi.on('rpc.subscribe.dwn.processMessage', handleDwnProcessMessage); + jsonRpcApi.on('rpc.subscribe.close', handleSubscriptionsClose); diff --git a/src/json-rpc-handlers/subscription/close.ts b/src/json-rpc-handlers/subscription/close.ts index 3c47fe1..fa15853 100644 --- a/src/json-rpc-handlers/subscription/close.ts +++ b/src/json-rpc-handlers/subscription/close.ts @@ -26,7 +26,7 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( ) => { const requestId = dwnRequest.id ?? uuidv4(); const { socketConnection } = context; - const { id } = dwnRequest.params as { id: JsonRpcId}; + const { id } = dwnRequest.subscribe as { id: JsonRpcId }; let jsonRpcResponse:JsonRpcResponse; try { diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index d84197d..ce327c8 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -3,7 +3,7 @@ import { v4 as uuidv4 } from 'uuid'; import WebSocket from 'ws'; import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; -import { createJsonRpcRequest } from "./lib/json-rpc.js"; +import { createJsonRpcSubscribeRequest } from "./lib/json-rpc.js"; // These were arbitrarily chosen, but can be modified via connect options const CONNECT_TIMEOUT = 3_000; @@ -73,7 +73,6 @@ export class JsonRpcSocket { return resolve(jsonRpsResponse); } }; - // subscribe to the listener before sending the request this.socket.addEventListener('message', handleResponse); this.send(request); @@ -96,21 +95,14 @@ export class JsonRpcSocket { }> { if (!request.method.startsWith('rpc.subscribe.')) { - throw new Error('subscribe rpc messages must include the `rpc.subscribe` prefix'); + throw new Error('subscribe rpc requests must include the `rpc.subscribe` prefix'); } - // extract optional `rpc.subscribe` param - const { rpc } = request.params; - const { subscribe } = rpc || {}; - const subscriptionId = subscribe || uuidv4(); - - // When subscribing to a JSON RPC Message, we want to generate the subscription update Json PRC Id ahead of time and create a listener. - // We then set the subscription Id within a special rpc.subscribe params namespace preserving any other properties - request.params.rpc = { - ...rpc, - subscribe: subscriptionId, - }; + if (!request.subscribe) { + throw new Error('subscribe rpc requests must include subscribe options'); + } + const subscriptionId = request.subscribe.id; const messageHandler = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; if (jsonRpcResponse.id === subscriptionId) { @@ -134,7 +126,7 @@ export class JsonRpcSocket { const close = async (): Promise => { this.socket.removeEventListener('message', messageHandler); const requestId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'rpc.subscribe.close', { id: subscriptionId }); + const request = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, subscriptionId) const response = await this.request(request); if (response.error) { throw response.error; diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index a460873..bb4021d 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -1,3 +1,5 @@ +import { v4 as uuidv4 } from 'uuid'; + export type JsonRpcId = string | number | null; export type JsonRpcParams = any; export type JsonRpcVersion = '2.0'; @@ -7,6 +9,10 @@ export interface JsonRpcRequest { id?: JsonRpcId; method: string; params?: JsonRpcParams; + /** JSON RPC Subscribe Extension Parameters */ + subscribe?: { + id: JsonRpcId + }; } export interface JsonRpcError { @@ -81,6 +87,23 @@ export const createJsonRpcNotification = ( }; }; +export const createJsonRpcSubscribeRequest = ( + id: JsonRpcId, + method: string, + params?: JsonRpcParams, + subscriptionId?: JsonRpcId +): JsonRpcRequest => { + return { + jsonrpc: '2.0', + id, + method, + params, + subscribe: { + id: subscriptionId ?? uuidv4(), + } + } +} + export const createJsonRpcRequest = ( id: JsonRpcId, method: string, diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 1279aa4..20f2daf 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -7,7 +7,7 @@ import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; import { JsonRpcSocket } from '../src/json-rpc-socket.js'; -import { createJsonRpcRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; +import { createJsonRpcRequest, createJsonRpcSubscribeRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; chai.use(chaiAsPromised); @@ -72,12 +72,10 @@ describe('JsonRpcSocket', () => { // initial response const response = createJsonRpcSuccessResponse(request.id, { reply: {} }) socket.send(Buffer.from(JSON.stringify(response))); - - const { params } = request; - const { subscribe } = params.rpc || {}; + const { subscribe } = request; // send 3 messages for (let i = 0; i < 3; i++) { - const response = createJsonRpcSuccessResponse(subscribe, { count: i }); + const response = createJsonRpcSuccessResponse(subscribe.id, { count: i }); socket.send(Buffer.from(JSON.stringify(response))); } }); @@ -85,7 +83,12 @@ describe('JsonRpcSocket', () => { const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); const requestId = uuidv4(); const subscribeId = uuidv4(); - const request = createJsonRpcRequest(requestId, 'rpc.subscribe.test.method', { param1: 'test-param1', param2: 'test-param2', rpc: { subscribe: subscribeId } }); + const request = createJsonRpcSubscribeRequest( + requestId, + 'rpc.subscribe.test.method', + { param1: 'test-param1', param2: 'test-param2' }, + subscribeId, + ); let responseCounter = 0; const responseListener = (response: JsonRpcSuccessResponse): void => { @@ -101,7 +104,7 @@ describe('JsonRpcSocket', () => { await new Promise((resolve) => setTimeout(resolve, 5)); // the original response expect(responseCounter).to.equal(3); - subscription.close(); + await subscription.close(); }); it('sends message', async () => { diff --git a/tests/utils.ts b/tests/utils.ts index 7655a57..55761e6 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -242,6 +242,9 @@ export async function subscriptionRequest(options: { }): Promise<{ close?: () => Promise, response: JsonRpcResponse, connection?: JsonRpcSocket }> { const { url, connection: incomingConnection, request, messageHandler, responseTimeout } = options; const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); + request.subscribe ??= { + id: uuidv4(), + }; const { close, response } = await connection.subscribe(request, (response) => { const { event } = response.result.reply; diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 486b5fd..502ce7c 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -12,6 +12,7 @@ import { v4 as uuidv4 } from 'uuid'; import { createJsonRpcRequest, + createJsonRpcSubscribeRequest, JsonRpcErrorCodes, } from '../src/lib/json-rpc.js'; import { config } from '../src/config.js'; @@ -111,7 +112,7 @@ describe('websocket api', function () { }; const requestId = uuidv4(); - const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, }); @@ -262,11 +263,10 @@ describe('websocket api', function () { const requestId = uuidv4(); const subscribeId = uuidv4(); - const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, - target: alice.did, - rpc: { subscribe: subscribeId } - }); + target: alice.did + }, subscribeId); const { response, close, connection } = await subscriptionRequest({ url : 'ws://127.0.0.1:9002', @@ -282,11 +282,10 @@ describe('websocket api', function () { // We are checking for the subscription Id not the request Id const request2Id = uuidv4(); - const dwnRequest2 = createJsonRpcRequest(request2Id, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest2 = createJsonRpcSubscribeRequest(request2Id, 'rpc.subscribe.dwn.processMessage', { message: message2, - target: alice.did, - rpc: { subscribe: subscribeId } - }); + target: alice.did + }, subscribeId); const { response: response2 } = await subscriptionRequest({ connection, From 7de0ad1de5b243cd628fb28e20e9341a1b4310d8 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 22 Feb 2024 22:48:58 -0500 Subject: [PATCH 30/40] increase testing across the board --- src/connection/socket-connection.ts | 15 +- src/json-rpc-handlers/subscription/close.ts | 10 ++ src/json-rpc-socket.ts | 20 +-- src/lib/json-rpc-router.ts | 4 +- src/lib/json-rpc.ts | 8 - tests/connection/connection-manager.spec.ts | 45 ++++++ tests/connection/socket-connection.spec.ts | 159 ++++++++++++++++++++ tests/dwn-process-message.spec.ts | 73 ++++++++- tests/json-rpc-socket.spec.ts | 83 +++++++++- tests/rpc-subscribe-close.spec.ts | 124 +++++++++++++++ tests/utils.ts | 24 ++- tests/ws-api.spec.ts | 10 +- 12 files changed, 530 insertions(+), 45 deletions(-) create mode 100644 tests/connection/connection-manager.spec.ts create mode 100644 tests/connection/socket-connection.spec.ts create mode 100644 tests/rpc-subscribe-close.spec.ts diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 1178506..82c18c9 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -148,7 +148,6 @@ export class SocketConnection { JsonRpcErrorCodes.BadRequest, (error as Error).message ); - return this.send(errorResponse); }; @@ -166,16 +165,10 @@ export class SocketConnection { } /** - * Sends a JSON encoded Buffer through the Websocket. Accepts a callback, if none is provided an error logger is used. + * Sends a JSON encoded Buffer through the Websocket. */ - private send(response: JsonRpcResponse | JsonRpcErrorResponse, cb?: (error?: Error) => void): void { - if (!cb) { - cb = (error):void => { - if(error) { log.error('socket send error', error, response); } - } - } - - this.socket.send(Buffer.from(JSON.stringify(response)), cb); + private send(response: JsonRpcResponse | JsonRpcErrorResponse): void { + this.socket.send(Buffer.from(JSON.stringify(response))); } /** @@ -185,7 +178,7 @@ export class SocketConnection { */ private createSubscriptionHandler(id: JsonRpcId): (message: MessageEvent) => void { return (event) => { - const response = createJsonRpcSuccessResponse(id, { reply: { event } }); + const response = createJsonRpcSuccessResponse(id, { event }); this.send(response); } } diff --git a/src/json-rpc-handlers/subscription/close.ts b/src/json-rpc-handlers/subscription/close.ts index fa15853..55de23a 100644 --- a/src/json-rpc-handlers/subscription/close.ts +++ b/src/json-rpc-handlers/subscription/close.ts @@ -25,6 +25,16 @@ export const handleSubscriptionsClose: JsonRpcHandler = async ( context, ) => { const requestId = dwnRequest.id ?? uuidv4(); + if (context.socketConnection === undefined) { + const jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidRequest, 'socket connection does not exist'); + return { jsonRpcResponse }; + } + + if (dwnRequest.subscribe === undefined) { + const jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidRequest, 'subscribe options do not exist'); + return { jsonRpcResponse }; + } + const { socketConnection } = context; const { id } = dwnRequest.subscribe as { id: JsonRpcId }; diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index ce327c8..941f366 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -2,7 +2,7 @@ import log from 'loglevel'; import { v4 as uuidv4 } from 'uuid'; import WebSocket from 'ws'; -import type { JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; +import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; import { createJsonRpcSubscribeRequest } from "./lib/json-rpc.js"; // These were arbitrarily chosen, but can be modified via connect options @@ -89,7 +89,7 @@ export class JsonRpcSocket { * Sends a JSON-RPC request through the socket and keeps a listener open to read associated responses as they arrive. * Returns a close method to clean up the listener. */ - async subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): Promise<{ + async subscribe(request: JsonRpcRequest, listener: (response: JsonRpcResponse) => void): Promise<{ response: JsonRpcResponse; close?: () => Promise; }> { @@ -109,8 +109,8 @@ export class JsonRpcSocket { if (jsonRpcResponse.error !== undefined) { // remove the event listener upon receipt of a JSON RPC Error. this.socket.removeEventListener('message', messageHandler); + this.closeSubscription(subscriptionId); } - listener(jsonRpcResponse); } }; @@ -125,12 +125,7 @@ export class JsonRpcSocket { // clean up listener and create a `rpc.subscribe.close` message to use when closing this JSON RPC subscription const close = async (): Promise => { this.socket.removeEventListener('message', messageHandler); - const requestId = uuidv4(); - const request = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, subscriptionId) - const response = await this.request(request); - if (response.error) { - throw response.error; - } + await this.closeSubscription(subscriptionId); } return { @@ -139,11 +134,16 @@ export class JsonRpcSocket { } } + private closeSubscription(id: JsonRpcId): Promise { + const requestId = uuidv4(); + const request = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + return this.request(request); + } + /** * Sends a JSON-RPC request through the socket. You must subscribe to a message listener separately to capture the response. */ send(request: JsonRpcRequest):void { this.socket.send(Buffer.from(JSON.stringify(request))); - return; } } \ No newline at end of file diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 580d375..75e9adf 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -1,4 +1,4 @@ -import type { Dwn, MessageEvent } from '@tbd54566975/dwn-sdk-js'; +import type { Dwn, EventSubscriptionHandler } from '@tbd54566975/dwn-sdk-js'; import type { Readable } from 'node:stream'; @@ -13,7 +13,7 @@ export type RequestContext = { /** The JsonRpcId of the subscription handler */ id: JsonRpcId; /** The `MessageEvent` handler associated with a subscription request, only used in `ws` requests */ - subscriptionHandler: (message: MessageEvent) => void; + subscriptionHandler: EventSubscriptionHandler; } /** The `Readable` stream associated with a `RecordsWrite` request only used in `http` requests */ dataStream?: Readable; diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index bb4021d..30ee5d5 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -127,11 +127,3 @@ export const createJsonRpcSuccessResponse = ( result: result ?? null, }; }; - -export function parseJson(text: string): object | null { - try { - return JSON.parse(text); - } catch { - return null; - } -} diff --git a/tests/connection/connection-manager.spec.ts b/tests/connection/connection-manager.spec.ts new file mode 100644 index 0000000..657c859 --- /dev/null +++ b/tests/connection/connection-manager.spec.ts @@ -0,0 +1,45 @@ +import type { Dwn } from '@tbd54566975/dwn-sdk-js'; + +import chaiAsPromised from 'chai-as-promised'; +import chai, { expect } from 'chai'; + +import sinon from 'sinon'; +import { WebSocket } from 'ws'; +import { getTestDwn } from '../test-dwn.js'; +import { InMemoryConnectionManager } from '../../src/connection/connection-manager.js'; + +chai.use(chaiAsPromised); + +describe('InMemoryConnectionManager', () => { + let dwn: Dwn; + let connectionManager: InMemoryConnectionManager; + + beforeEach(async () => { + dwn = await getTestDwn({ withEvents: true }); + connectionManager = new InMemoryConnectionManager(dwn); + }); + + afterEach(async () => { + await connectionManager.closeAll(); + await dwn.close(); + sinon.restore(); + }); + + it('adds connection to the connections map and closes all', async () => { + const socket1 = sinon.createStubInstance(WebSocket); + await connectionManager.connect(socket1); + expect((connectionManager as any).connections.size).to.equal(1); + + const socket2 = sinon.createStubInstance(WebSocket); + await connectionManager.connect(socket2); + expect((connectionManager as any).connections.size).to.equal(2); + }); + + xit('closes all connections', async () => { + const socket = sinon.createStubInstance(WebSocket); + await connectionManager.connect(socket); + expect((connectionManager as any).connections.size).to.equal(1); + await connectionManager.closeAll(); + expect((connectionManager as any).connections.size).to.equal(0); + }); +}); \ No newline at end of file diff --git a/tests/connection/socket-connection.spec.ts b/tests/connection/socket-connection.spec.ts new file mode 100644 index 0000000..3f033a5 --- /dev/null +++ b/tests/connection/socket-connection.spec.ts @@ -0,0 +1,159 @@ +import type { Dwn } from '@tbd54566975/dwn-sdk-js'; + +import chaiAsPromised from 'chai-as-promised'; +import chai, { expect } from 'chai'; + +import sinon from 'sinon'; +import { WebSocket } from 'ws'; +import { SocketConnection } from '../../src/connection/socket-connection.js'; +import { getTestDwn } from '../test-dwn.js'; +import log from 'loglevel'; + +chai.use(chaiAsPromised); + +describe('SocketConnection', () => { + let dwn: Dwn; + + before(async () => { + dwn = await getTestDwn(); + }); + + after(async () => { + await dwn.close(); + sinon.restore(); + }); + + it('should assign socket handlers', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + expect(socket.on.callCount).to.equal(4); + expect(socket.on.args.map(arg => arg[0])).to.have.members(['message', 'close', 'error', 'pong']); + await connection.close(); + }); + + it('should add a subscription to the subscription manager map', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + const subscriptionRequest = { + id: 'id', + method: 'method', + params: { param1: 'param' }, + close: async ():Promise => {} + } + + await connection.addSubscription(subscriptionRequest); + expect((connection as any).subscriptions.size).to.equal(1); + await connection.close(); + expect((connection as any).subscriptions.size).to.equal(0); + }); + + it('should reject a subscription with an Id of an existing subscription', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + + const id = 'some-id'; + + const subscriptionRequest = { + id, + method: 'method', + params: { param1: 'param' }, + close: async ():Promise => {} + } + + await connection.addSubscription(subscriptionRequest); + expect((connection as any).subscriptions.size).to.equal(1); + + const addDuplicatePromise = connection.addSubscription(subscriptionRequest); + await expect(addDuplicatePromise).to.eventually.be.rejectedWith(`the subscription with id ${id} already exists`); + expect((connection as any).subscriptions.size).to.equal(1); + await connection.close(); + expect((connection as any).subscriptions.size).to.equal(0); + }); + + it('should close a subscription and remove it from the connection manager map', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + + const id = 'some-id'; + + const subscriptionRequest = { + id, + method: 'method', + params: { param1: 'param' }, + close: async ():Promise => {} + } + + await connection.addSubscription(subscriptionRequest); + expect((connection as any).subscriptions.size).to.equal(1); + + await connection.closeSubscription(id); + expect((connection as any).subscriptions.size).to.equal(0); + + const closeAgainPromise = connection.closeSubscription(id); + await expect(closeAgainPromise).to.eventually.be.rejectedWith(`the subscription with id ${id} was not found`); + await connection.close(); + }); + + it('hasSubscription returns whether a subscription with the id already exists', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + const subscriptionRequest = { + id: 'id', + method: 'method', + params: { param1: 'param' }, + close: async ():Promise => {} + } + + await connection.addSubscription(subscriptionRequest); + expect((connection as any).subscriptions.size).to.equal(1); + expect(connection.hasSubscription(subscriptionRequest.id)).to.be.true; + expect(connection.hasSubscription('does-not-exist')).to.be.false; + + await connection.closeSubscription(subscriptionRequest.id); + expect(connection.hasSubscription(subscriptionRequest.id)).to.be.false; + await connection.close(); + }); + + it('should close if pong is not triggered between heartbeat intervals', async () => { + const socket = sinon.createStubInstance(WebSocket); + const clock = sinon.useFakeTimers(); + const connection = new SocketConnection(socket, dwn); + const closeSpy = sinon.spy(connection, 'close'); + + clock.tick(60_100); // interval has to run twice + clock.restore(); + + expect(closeSpy.callCount).to.equal(1); + }); + + it('should not close if pong is called within the heartbeat interval', async () => { + const socket = sinon.createStubInstance(WebSocket); + const clock = sinon.useFakeTimers(); + const connection = new SocketConnection(socket, dwn); + const closeSpy = sinon.spy(connection, 'close'); + + (connection as any).pong(); // trigger a pong + clock.tick(30_100); // first interval + + (connection as any).pong(); // trigger a pong + clock.tick(30_100); // second interval + + expect(closeSpy.callCount).to.equal(0); + + clock.tick(30_100); // another interval without a ping + clock.restore(); + expect(closeSpy.callCount).to.equal(1); + }); + + it('logs an error and closes connection if error is triggered', async () => { + const socket = sinon.createStubInstance(WebSocket); + const connection = new SocketConnection(socket, dwn); + const logSpy = sinon.stub(log, 'error'); + const closeSpy = sinon.spy(connection, 'close'); + + (connection as any).error(new Error('some error')); + + expect(logSpy.callCount).to.equal(1); + expect(closeSpy.callCount).to.equal(1); + }); +}); \ No newline at end of file diff --git a/tests/dwn-process-message.spec.ts b/tests/dwn-process-message.spec.ts index 5249da2..fe4f3d1 100644 --- a/tests/dwn-process-message.spec.ts +++ b/tests/dwn-process-message.spec.ts @@ -1,9 +1,10 @@ import { expect } from 'chai'; +import sinon from 'sinon'; import { v4 as uuidv4 } from 'uuid'; import { handleDwnProcessMessage } from '../src/json-rpc-handlers/dwn/process-message.js'; import type { RequestContext } from '../src/lib/json-rpc-router.js'; -import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; +import { JsonRpcErrorCodes, createJsonRpcRequest } from '../src/lib/json-rpc.js'; import { getTestDwn } from './test-dwn.js'; import { createRecordsWriteMessage } from './utils.js'; import { TestDataGenerator } from '@tbd54566975/dwn-sdk-js'; @@ -62,4 +63,74 @@ describe('handleDwnProcessMessage', function () { expect(reply.entries).to.be.undefined; await dwn.close(); }); + + it('should fail if no subscriptionRequest context exists for a `Subscribe` message', async function () { + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + message: { + descriptor: { interface: 'Records', method: 'Subscribe' }, + }, + target: 'did:key:abc1234', + }); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws' }; + + const { jsonRpcResponse } = await handleDwnProcessMessage( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InvalidRequest); + expect(jsonRpcResponse.error.message).to.equal('subscribe methods must contain a subscriptionRequest context'); + await dwn.close(); + }); + + it('should fail on http requests for a `Subscribe` message', async function () { + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + message: { + descriptor: { interface: 'Records', method: 'Subscribe' }, + }, + target: 'did:key:abc1234', + }); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'http', subscriptionRequest: { id: 'test', subscriptionHandler: () => {}} }; + + const { jsonRpcResponse } = await handleDwnProcessMessage( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); + expect(jsonRpcResponse.error.message).to.equal('subscriptions are not supported via http'); + await dwn.close(); + }); + + it('should return a JsonRpc Internal Error for an unexpected thrown error within the handler', async function () { + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'dwn.processMessage', { + message: { + descriptor: { interface: 'Records' }, + }, + target: 'did:key:abc1234', + }); + + const dwn = await getTestDwn(); + sinon.stub(dwn, 'processMessage').throws(new Error('unexpected error')); + const context: RequestContext = { dwn, transport: 'http' }; + + const { jsonRpcResponse } = await handleDwnProcessMessage( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InternalError); + expect(jsonRpcResponse.error.message).to.equal('unexpected error'); + await dwn.close(); + }); }); diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 20f2daf..7897438 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -7,7 +7,7 @@ import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; import { JsonRpcSocket } from '../src/json-rpc-socket.js'; -import { createJsonRpcRequest, createJsonRpcSubscribeRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; +import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcRequest, createJsonRpcSubscribeRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; chai.use(chaiAsPromised); @@ -80,6 +80,7 @@ describe('JsonRpcSocket', () => { } }); }); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); const requestId = uuidv4(); const subscribeId = uuidv4(); @@ -126,9 +127,87 @@ describe('JsonRpcSocket', () => { await expect(receivedPromise).to.eventually.eql({ reply: { id: request.id }}); }); + it('closes subscription upon receiving a JsonRpc Error for a long running subscription', async () => { + let closed = true; + wsServer.addListener('connection', (socket) => { + closed = false; + socket.on('message', (dataBuffer: Buffer) => { + const request = JSON.parse(dataBuffer.toString()) as JsonRpcRequest; + if (request.method.startsWith('rpc.subscribe') && request.method !== 'rpc.subscribe.close') { + // initial response + const response = createJsonRpcSuccessResponse(request.id, { reply: {} }) + socket.send(Buffer.from(JSON.stringify(response))); + const { subscribe } = request; + + // send 1 valid message + const message1 = createJsonRpcSuccessResponse(subscribe.id, { message: 1 }); + socket.send(Buffer.from(JSON.stringify(message1))); + + // send a json rpc error + const jsonRpcError = createJsonRpcErrorResponse(subscribe.id, JsonRpcErrorCodes.InternalError, 'some error'); + socket.send(Buffer.from(JSON.stringify(jsonRpcError))); + + // send a 2nd message that shouldn't be handled + const message2 = createJsonRpcSuccessResponse(subscribe.id, { message: 2 }); + socket.send(Buffer.from(JSON.stringify(message2))); + } else if (request.method === 'rpc.subscribe.close') { + closed = true; + } + }); + }); + + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); + const requestId = uuidv4(); + const subscribeId = uuidv4(); + const request = createJsonRpcSubscribeRequest( + requestId, + 'rpc.subscribe.test.method', + { param1: 'test-param1', param2: 'test-param2' }, + subscribeId, + ); + + let responseCounter = 0; + let errorCounter = 0; + const responseListener = (response: JsonRpcSuccessResponse): void => { + expect(response.id).to.equal(subscribeId); + if (response.error) { + errorCounter++; + } + + if (response.result) { + responseCounter++; + } + } + + const subscription = await client.subscribe(request, responseListener); + expect(subscription.response.error).to.be.undefined; + // wait for the messages to arrive + await new Promise((resolve) => setTimeout(resolve, 5)); + // the original response + expect(responseCounter).to.equal(1); + expect(errorCounter).to.equal(1); + expect(closed).to.equal(true); + }); + + it('only JSON RPC Methods prefixed with `rpc.subscribe.` are accepted for a subscription', async () => { + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'test.method', { param1: 'test-param1', param2: 'test-param2' }); + const subscribePromise = client.subscribe(request, () => {}); + await expect(subscribePromise).to.eventually.be.rejectedWith('subscribe rpc requests must include the `rpc.subscribe` prefix'); + }); + + it('subscribe methods must contain a subscribe object within the request which contains the subscription JsonRpcId', async () => { + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); + const requestId = uuidv4(); + const request = createJsonRpcRequest(requestId, 'rpc.subscribe.test.method', { param1: 'test-param1', param2: 'test-param2' }); + const subscribePromise = client.subscribe(request, () => {}); + await expect(subscribePromise).to.eventually.be.rejectedWith('subscribe rpc requests must include subscribe options'); + }); + xit('calls onerror handler', async () => { }); - xit('calls onclose handler', async () => { + xit('calls onclose hanhler', async () => { }); }); diff --git a/tests/rpc-subscribe-close.spec.ts b/tests/rpc-subscribe-close.spec.ts new file mode 100644 index 0000000..06ad6c5 --- /dev/null +++ b/tests/rpc-subscribe-close.spec.ts @@ -0,0 +1,124 @@ +import { expect } from 'chai'; +import sinon from 'sinon'; +import { v4 as uuidv4 } from 'uuid'; + +import type { RequestContext } from '../src/lib/json-rpc-router.js'; +import { JsonRpcErrorCodes, createJsonRpcRequest, createJsonRpcSubscribeRequest } from '../src/lib/json-rpc.js'; +import { getTestDwn } from './test-dwn.js'; +import { handleSubscriptionsClose } from '../src/json-rpc-handlers/subscription/close.js'; +import { SocketConnection } from '../src/connection/socket-connection.js'; +import { DwnServerError, DwnServerErrorCode } from '../src/dwn-error.js'; + +describe('handleDwnProcessMessage', function () { + it('should return an error if no socket connection exists', async function () { + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.close', { }); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws' }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InvalidRequest); + expect(jsonRpcResponse.error.message).to.equal('socket connection does not exist'); + }); + + it('should return an error if no subscribe options exist', async function () { + const requestId = uuidv4(); + const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.close', { }); + const socketConnection = sinon.createStubInstance(SocketConnection); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws', socketConnection }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InvalidRequest); + expect(jsonRpcResponse.error.message).to.equal('subscribe options do not exist'); + }); + + it('should return an error if close subscription throws ConnectionSubscriptionJsonRpcIdNotFound', async function () { + const requestId = uuidv4(); + const id = 'some-id'; + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const socketConnection = sinon.createStubInstance(SocketConnection); + socketConnection.closeSubscription.throws(new DwnServerError( + DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdNotFound, + '' + )); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws', socketConnection }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); + expect(jsonRpcResponse.error.message).to.equal(`subscription ${id} does not exist.`); + }); + + it('should return an error if close subscription throws ConnectionSubscriptionJsonRpcIdNotFound', async function () { + const requestId = uuidv4(); + const id = 'some-id'; + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const socketConnection = sinon.createStubInstance(SocketConnection); + socketConnection.closeSubscription.throws(new Error('unknown error')); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws', socketConnection }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + + expect(jsonRpcResponse.error).to.exist; + expect(jsonRpcResponse.error.code).to.equal(JsonRpcErrorCodes.InternalError); + expect(jsonRpcResponse.error.message).to.equal(`unknown subscription close error for ${id}: unknown error`); + }); + + it('should return a success', async function () { + const requestId = uuidv4(); + const id = 'some-id'; + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const socketConnection = sinon.createStubInstance(SocketConnection); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws', socketConnection }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + expect(jsonRpcResponse.error).to.not.exist; + }); + + it('handler should generate a recordId if one is not provided with the request', async function () { + const requestId = uuidv4(); + const id = 'some-id'; + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + delete dwnRequest.id; // delete request id + + const socketConnection = sinon.createStubInstance(SocketConnection); + + const dwn = await getTestDwn(); + const context: RequestContext = { dwn, transport: 'ws', socketConnection }; + + const { jsonRpcResponse } = await handleSubscriptionsClose( + dwnRequest, + context, + ); + expect(jsonRpcResponse.error).to.not.exist; + }); +}); diff --git a/tests/utils.ts b/tests/utils.ts index 55761e6..e7b7f8e 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -1,4 +1,4 @@ -import type { GenericMessage, MessageEvent, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; +import type { EventSubscriptionHandler, GenericMessage, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; import { Cid, DataStream, RecordsWrite } from '@tbd54566975/dwn-sdk-js'; import type { ReadStream } from 'node:fs'; @@ -233,12 +233,24 @@ export async function sendWsRequest(options: { return connection.request(request); } -export async function subscriptionRequest(options: { - url?: string, +/** + * A helper method for testing JSON RPC socket subscription requests to the DWN. + * + * If a connection is not provided, creates a new connection to the url provided. + * If no subscribe options are provided, creates a subscribe id. + * Attempts to subscribe and returns the response, close function and connection. + */ +export async function subscribeToMessageEvents(options: { + /** json rpc socket connection, mutually exclusive with url */ connection?: JsonRpcSocket, + /** url to connect to if no connection is provided */ + url?: string, + /** the request to use for subscription */ request: JsonRpcRequest, - messageHandler: (event: MessageEvent) => void, - responseTimeout?: number; + /** the message handler to use for incoming events */ + messageHandler: EventSubscriptionHandler, + /** optional response timeout for new connections */ + responseTimeout?: number, }): Promise<{ close?: () => Promise, response: JsonRpcResponse, connection?: JsonRpcSocket }> { const { url, connection: incomingConnection, request, messageHandler, responseTimeout } = options; const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); @@ -247,7 +259,7 @@ export async function subscriptionRequest(options: { }; const { close, response } = await connection.subscribe(request, (response) => { - const { event } = response.result.reply; + const { event } = response.result; messageHandler(event); }); diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 502ce7c..893a592 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -18,7 +18,7 @@ import { import { config } from '../src/config.js'; import { WsApi } from '../src/ws-api.js'; import { getTestDwn } from './test-dwn.js'; -import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscriptionRequest, sendWsRequest } from './utils.js'; +import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscribeToMessageEvents, sendWsRequest } from './utils.js'; import { HttpApi } from '../src/http-api.js'; @@ -117,7 +117,7 @@ describe('websocket api', function () { target: alice.did, }); - const { response, close } = await subscriptionRequest({ + const { response, close } = await subscribeToMessageEvents({ url : 'ws://127.0.0.1:9002', request : dwnRequest, messageHandler : subscriptionHandler @@ -186,7 +186,7 @@ describe('websocket api', function () { target: alice.did, }); - const { response, close } = await subscriptionRequest({ + const { response, close } = await subscribeToMessageEvents({ url : 'ws://127.0.0.1:9002', request : dwnRequest, messageHandler : subscriptionHandler @@ -268,7 +268,7 @@ describe('websocket api', function () { target: alice.did }, subscribeId); - const { response, close, connection } = await subscriptionRequest({ + const { response, close, connection } = await subscribeToMessageEvents({ url : 'ws://127.0.0.1:9002', request : dwnRequest, messageHandler : subscriptionHandler @@ -287,7 +287,7 @@ describe('websocket api', function () { target: alice.did }, subscribeId); - const { response: response2 } = await subscriptionRequest({ + const { response: response2 } = await subscribeToMessageEvents({ connection, request: dwnRequest2, messageHandler: subscriptionHandler, From f54fac22e1fbd1713a5cbcfd8beb5accc937f55c Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Thu, 22 Feb 2024 22:59:14 -0500 Subject: [PATCH 31/40] remove utils that were not very useful --- tests/utils.ts | 51 ++---------------------------------------- tests/ws-api.spec.ts | 53 ++++++++++++++++++++++---------------------- 2 files changed, 28 insertions(+), 76 deletions(-) diff --git a/tests/utils.ts b/tests/utils.ts index e7b7f8e..af10b0a 100644 --- a/tests/utils.ts +++ b/tests/utils.ts @@ -1,4 +1,4 @@ -import type { EventSubscriptionHandler, GenericMessage, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; +import type { GenericMessage, Persona, UnionMessageReply } from '@tbd54566975/dwn-sdk-js'; import { Cid, DataStream, RecordsWrite } from '@tbd54566975/dwn-sdk-js'; import type { ReadStream } from 'node:fs'; @@ -11,9 +11,8 @@ import type { Readable } from 'readable-stream'; import { fileURLToPath } from 'url'; import { WebSocket } from 'ws'; -import type { JsonRpcResponse, JsonRpcRequest } from '../src/lib/json-rpc.js'; +import type { JsonRpcResponse } from '../src/lib/json-rpc.js'; import { createJsonRpcRequest } from '../src/lib/json-rpc.js'; -import { JsonRpcSocket } from '../src/json-rpc-socket.js'; // __filename and __dirname are not defined in ES module scope const __filename = fileURLToPath(import.meta.url); @@ -221,49 +220,3 @@ export async function sendWsMessage( }; }); } - -export async function sendWsRequest(options: { - url?: string, - connection?: JsonRpcSocket, - request: JsonRpcRequest, - responseTimeout?: number, -}): Promise { - const { url, connection: incomingConnection , request, responseTimeout } = options; - const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); - return connection.request(request); -} - -/** - * A helper method for testing JSON RPC socket subscription requests to the DWN. - * - * If a connection is not provided, creates a new connection to the url provided. - * If no subscribe options are provided, creates a subscribe id. - * Attempts to subscribe and returns the response, close function and connection. - */ -export async function subscribeToMessageEvents(options: { - /** json rpc socket connection, mutually exclusive with url */ - connection?: JsonRpcSocket, - /** url to connect to if no connection is provided */ - url?: string, - /** the request to use for subscription */ - request: JsonRpcRequest, - /** the message handler to use for incoming events */ - messageHandler: EventSubscriptionHandler, - /** optional response timeout for new connections */ - responseTimeout?: number, -}): Promise<{ close?: () => Promise, response: JsonRpcResponse, connection?: JsonRpcSocket }> { - const { url, connection: incomingConnection, request, messageHandler, responseTimeout } = options; - const connection = incomingConnection ?? await JsonRpcSocket.connect(url, { responseTimeout }); - request.subscribe ??= { - id: uuidv4(), - }; - - const { close, response } = await connection.subscribe(request, (response) => { - const { event } = response.result; - messageHandler(event); - }); - - return { - response, close, connection - } -} diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 893a592..03ce7b7 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -18,8 +18,9 @@ import { import { config } from '../src/config.js'; import { WsApi } from '../src/ws-api.js'; import { getTestDwn } from './test-dwn.js'; -import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage, subscribeToMessageEvents, sendWsRequest } from './utils.js'; +import { createRecordsWriteMessage, sendWsMessage, sendHttpMessage } from './utils.js'; import { HttpApi } from '../src/http-api.js'; +import { JsonRpcSocket } from '../src/json-rpc-socket.js'; describe('websocket api', function () { @@ -85,10 +86,9 @@ describe('websocket api', function () { encodedData, }); - const response = await sendWsRequest({ - url:'ws://127.0.0.1:9002', - request: dwnRequest, - }); + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + const response = await connection.request(dwnRequest); + expect(response.id).to.equal(requestId); expect(response.error).to.not.be.undefined; expect(response.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); @@ -117,11 +117,12 @@ describe('websocket api', function () { target: alice.did, }); - const { response, close } = await subscribeToMessageEvents({ - url : 'ws://127.0.0.1:9002', - request : dwnRequest, - messageHandler : subscriptionHandler + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + const { response, close } = await connection.subscribe(dwnRequest, (response) => { + const { event } = response.result; + subscriptionHandler(event); }); + expect(response.error).to.be.undefined; expect(response.result.reply.status.code).to.equal(200); expect(close).to.not.be.undefined; @@ -181,16 +182,18 @@ describe('websocket api', function () { }; const requestId = uuidv4(); - const dwnRequest = createJsonRpcRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const subscribeId = uuidv4(); + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, - }); + }, subscribeId); - const { response, close } = await subscribeToMessageEvents({ - url : 'ws://127.0.0.1:9002', - request : dwnRequest, - messageHandler : subscriptionHandler + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + const { response, close } = await connection.subscribe(dwnRequest, (response) => { + const { event } = response.result; + subscriptionHandler(event); }); + expect(response.error).to.be.undefined; expect(response.result.reply.status.code).to.equal(200); expect(close).to.not.be.undefined; @@ -268,15 +271,11 @@ describe('websocket api', function () { target: alice.did }, subscribeId); - const { response, close, connection } = await subscribeToMessageEvents({ - url : 'ws://127.0.0.1:9002', - request : dwnRequest, - messageHandler : subscriptionHandler + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + const { close } = await connection.subscribe(dwnRequest, (response) => { + const { event } = response.result; + subscriptionHandler(event); }); - expect(response.error).to.be.undefined; - expect(response.result.reply.status.code).to.equal(200); - expect(close).to.not.be.undefined; - const { message: message2 } = await TestDataGenerator.generateRecordsSubscribe({ filter: { schema: 'bar/baz' }, author: alice }); @@ -287,11 +286,11 @@ describe('websocket api', function () { target: alice.did }, subscribeId); - const { response: response2 } = await subscribeToMessageEvents({ - connection, - request: dwnRequest2, - messageHandler: subscriptionHandler, + const { response: response2 } = await connection.subscribe(dwnRequest2, (response) => { + const { event } = response.result; + subscriptionHandler(event); }); + expect(response2.error.code).to.equal(JsonRpcErrorCodes.InvalidParams); expect(response2.error.message).to.contain(`${subscribeId} is in use by an active subscription`); From a1caaae6b5a29a9804510b2f920760f7c001f135 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 26 Feb 2024 19:01:22 -0500 Subject: [PATCH 32/40] increase test covrage for connection manager and add comments --- src/connection/connection-manager.ts | 9 +-- src/connection/socket-connection.ts | 18 ++++- src/json-rpc-handlers/dwn/process-message.ts | 8 +- src/json-rpc-socket.ts | 11 +-- src/lib/json-rpc-router.ts | 1 + tests/connection/connection-manager.spec.ts | 37 +++++++--- tests/json-rpc-socket.spec.ts | 19 ++++- tests/ws-api.spec.ts | 77 ++++++++++++++++++++ 8 files changed, 151 insertions(+), 29 deletions(-) diff --git a/src/connection/connection-manager.ts b/src/connection/connection-manager.ts index 250c97e..b98aae8 100644 --- a/src/connection/connection-manager.ts +++ b/src/connection/connection-manager.ts @@ -23,13 +23,12 @@ export class InMemoryConnectionManager implements ConnectionManager { constructor(private dwn: Dwn, private connections: Map = new Map()) {} async connect(socket: WebSocket): Promise { - const connection = new SocketConnection(socket, this.dwn); - this.connections.set(socket, connection); - // attach to the socket's close handler to clean up this connection. - socket.on('close', () => { - // the connection internally already cleans itself up upon a socket close event, we just ned to remove it from our set. + const connection = new SocketConnection(socket, this.dwn, () => { + // this is the onClose handler to clean up any closed connections. this.connections.delete(socket); }); + + this.connections.set(socket, connection); } async closeAll(): Promise { diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 82c18c9..678c306 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -26,7 +26,8 @@ export class SocketConnection { constructor( private socket: WebSocket, - private dwn: Dwn + private dwn: Dwn, + private onClose?: () => void ){ socket.on('message', this.message.bind(this)); socket.on('close', this.close.bind(this)); @@ -93,6 +94,7 @@ export class SocketConnection { */ async close(): Promise { clearInterval(this.heartbeatInterval); + // clean up all socket event listeners this.socket.removeAllListeners(); @@ -107,6 +109,11 @@ export class SocketConnection { // close the socket. this.socket.close(); + + // if there was a close handler passed call it after the connection has been closed + if (this.onClose !== undefined) { + this.onClose(); + } } /** @@ -122,8 +129,12 @@ export class SocketConnection { */ private async error(error:Error): Promise{ log.error(`SocketConnection error, terminating connection`, error); - this.socket.terminate(); - await this.close(); + try { + this.socket.terminate(); + await this.close(); + } catch(error) { + console.log('error within error call'); + } } /** @@ -197,6 +208,7 @@ export class SocketConnection { socketConnection : this, } + // methods that expect a long-running subscription begin with `rpc.subscribe.` if (method.startsWith('rpc.subscribe.') && subscribe) { const { message } = params as { message?: GenericMessage }; if (message?.descriptor.method === DwnMethodName.Subscribe) { diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 0d419c7..3a12d1c 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -62,6 +62,8 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( return { jsonRpcResponse }; } + // if this is a subscription request, we first check if the connection has a subscription with this Id + // we do this ahead of time to prevent opening a subscription on the dwn only to close it after attempting to add it to the subscription manager if (subscriptionRequest !== undefined && socketConnection?.hasSubscription(subscriptionRequest.id)) { const jsonRpcResponse = createJsonRpcErrorResponse( requestId, @@ -84,9 +86,9 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( delete reply.record.data; // not serializable via JSON } - // Subscribe messages return a close function to facilitate closing the subscription if (subscriptionRequest && reply.subscription) { const { close } = reply.subscription; + // Subscribe messages return a close function to facilitate closing the subscription // we add a reference to the close function for this subscription request to the socket connection. // this will facilitate closing the subscription later. const subscriptionReply: JsonRpcSubscription = { @@ -111,8 +113,8 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( e.message, ); - // log the error response - log.error('handleDwnProcessMessage error', jsonRpcResponse); + // log the unhandled error response + log.error('handleDwnProcessMessage error', jsonRpcResponse, e); return { jsonRpcResponse } as HandlerResponse; } }; diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 941f366..eae5a68 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -30,21 +30,22 @@ export class JsonRpcSocket { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; const socket = new WebSocket(url); - if (onclose === undefined) { + + socket.onclose = onclose; + socket.onerror = onerror; + + if (socket.onclose === undefined) { socket.onclose = ():void => { log.info(`JSON RPC Socket close ${url}`); } } - if (onerror === undefined) { + if (socket.onerror === undefined) { socket.onerror = (error?: any):void => { log.error(`JSON RPC Socket error ${url}`, error); } } - socket.onclose = onclose; - socket.onerror = onerror; - return new Promise((resolve, reject) => { socket.on('open', () => { resolve(new JsonRpcSocket(socket, responseTimeout)); diff --git a/src/lib/json-rpc-router.ts b/src/lib/json-rpc-router.ts index 75e9adf..8fd1408 100644 --- a/src/lib/json-rpc-router.ts +++ b/src/lib/json-rpc-router.ts @@ -8,6 +8,7 @@ import type { SocketConnection } from '../connection/socket-connection.js'; export type RequestContext = { transport: 'http' | 'ws'; dwn: Dwn; + /** the socket connection associated with this request if over sockets */ socketConnection?: SocketConnection; subscriptionRequest?: { /** The JsonRpcId of the subscription handler */ diff --git a/tests/connection/connection-manager.spec.ts b/tests/connection/connection-manager.spec.ts index 657c859..ba6850b 100644 --- a/tests/connection/connection-manager.spec.ts +++ b/tests/connection/connection-manager.spec.ts @@ -4,41 +4,56 @@ import chaiAsPromised from 'chai-as-promised'; import chai, { expect } from 'chai'; import sinon from 'sinon'; -import { WebSocket } from 'ws'; import { getTestDwn } from '../test-dwn.js'; import { InMemoryConnectionManager } from '../../src/connection/connection-manager.js'; +import { config } from '../../src/config.js'; +import { WsApi } from '../../src/ws-api.js'; +import type { Server } from 'http'; +import { HttpApi } from '../../src/http-api.js'; +import { JsonRpcSocket } from '../../src/json-rpc-socket.js'; chai.use(chaiAsPromised); describe('InMemoryConnectionManager', () => { let dwn: Dwn; - let connectionManager: InMemoryConnectionManager; + let connectionManager: InMemoryConnectionManager; + let server: Server + let wsApi: WsApi; beforeEach(async () => { dwn = await getTestDwn({ withEvents: true }); connectionManager = new InMemoryConnectionManager(dwn); + const httpApi = new HttpApi(config, dwn); + server = await httpApi.start(9002); + wsApi = new WsApi(server, dwn, connectionManager); + wsApi.start(); }); afterEach(async () => { await connectionManager.closeAll(); await dwn.close(); + await wsApi.close(); + server.close(); + server.closeAllConnections(); sinon.restore(); }); - it('adds connection to the connections map and closes all', async () => { - const socket1 = sinon.createStubInstance(WebSocket); - await connectionManager.connect(socket1); + it('adds connection to the connections and removes it if that connection is closed', async () => { + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); expect((connectionManager as any).connections.size).to.equal(1); + connection.close(); - const socket2 = sinon.createStubInstance(WebSocket); - await connectionManager.connect(socket2); - expect((connectionManager as any).connections.size).to.equal(2); + await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to be fired + expect((connectionManager as any).connections.size).to.equal(0); }); - xit('closes all connections', async () => { - const socket = sinon.createStubInstance(WebSocket); - await connectionManager.connect(socket); + it('closes all connections on `closeAll`', async () => { + await JsonRpcSocket.connect('ws://127.0.0.1:9002'); expect((connectionManager as any).connections.size).to.equal(1); + + await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + expect((connectionManager as any).connections.size).to.equal(2); + await connectionManager.closeAll(); expect((connectionManager as any).connections.size).to.equal(0); }); diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 7897438..ddf1f04 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -2,6 +2,7 @@ import chaiAsPromised from 'chai-as-promised'; import chai, { expect } from 'chai'; import { v4 as uuidv4 } from 'uuid'; +import sinon from 'sinon'; import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; @@ -205,9 +206,23 @@ describe('JsonRpcSocket', () => { await expect(subscribePromise).to.eventually.be.rejectedWith('subscribe rpc requests must include subscribe options'); }); - xit('calls onerror handler', async () => { + it('calls onclose handler', async () => { + const onCloseHandler = { onclose: ():void => {} }; + const onCloseSpy = sinon.spy(onCloseHandler, 'onclose'); + const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onclose: onCloseHandler.onclose }); + client.close(); + + await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive + expect(onCloseSpy.callCount).to.equal(1); }); - xit('calls onclose hanhler', async () => { + xit('calls onerror handler', async () => { + const onErrorHandler = { onerror: ():void => {} }; + const onErrorSpy = sinon.spy(onErrorHandler, 'onerror'); + + await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onerror: onErrorHandler.onerror }); + + await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive + expect(onErrorSpy.callCount).to.equal(1, 'error'); }); }); diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 03ce7b7..74cc898 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -331,4 +331,81 @@ describe('websocket api', function () { await Message.getCid(write2Message.message) ]); }); + + it('should receive an updated message as well as the initial write when subscribing to a record', async () => { + const alice = await TestDataGenerator.generateDidKeyPersona(); + + // write an initial message + const initialWrite = await TestDataGenerator.generateRecordsWrite({ + author : alice, + schema : 'foo/bar', + dataFormat : 'text/plain' + }); + + const writeResult1 = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : initialWrite.message, + data : initialWrite.dataBytes, + }); + expect(writeResult1.status.code).to.equal(202); + + // subscribe to 'foo/bar' messages + const { message } = await TestDataGenerator.generateRecordsSubscribe({ + author: alice, + filter: { + schema: 'foo/bar' + } + }); + + const records: string[] = []; + const subscriptionHandler = async (event: MessageEvent): Promise => { + const { message, initialWrite } = event + if (initialWrite) { + records.push(await Message.getCid(initialWrite)); + } + records.push(await Message.getCid(message)); + }; + + const requestId = uuidv4(); + const subscribeId = uuidv4(); + const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + message: message, + target: alice.did + }, subscribeId); + + const connection = await JsonRpcSocket.connect('ws://127.0.0.1:9002'); + const { close } = await connection.subscribe(dwnRequest, (response) => { + const { event } = response.result; + subscriptionHandler(event); + }); + + // wait for potential records to process and confirm that initial write has not been processed + await new Promise(resolve => setTimeout(resolve, 5)); + expect(records.length).length.to.equal(0); + + // update the initial message + const updatedMessage = await TestDataGenerator.generateFromRecordsWrite({ + author : alice, + existingWrite : initialWrite.recordsWrite, + }); + + const updateResult = await sendHttpMessage({ + url : 'http://localhost:9002', + target : alice.did, + message : updatedMessage.message, + data : updatedMessage.dataBytes, + }); + expect(updateResult.status.code).to.equal(202); + + await close(); + + await new Promise(resolve => setTimeout(resolve, 5)); // wait for records to be processed + + // both initial and update should exist now + expect(records).to.have.members([ + await Message.getCid(initialWrite.message), + await Message.getCid(updatedMessage.message) + ]); + }); }); From 7c9983fd1d35130641a1ae550f9160496133fe6f Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 26 Feb 2024 19:25:14 -0500 Subject: [PATCH 33/40] review comments --- src/connection/socket-connection.ts | 12 ++++++------ src/http-api.ts | 4 ++-- src/index.ts | 2 +- src/json-rpc-api.ts | 8 ++++---- src/json-rpc-handlers/subscription/close.ts | 10 +++++----- src/json-rpc-socket.ts | 4 ++-- src/lib/json-rpc.ts | 10 ++++------ tests/json-rpc-socket.spec.ts | 12 ++++++------ 8 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 678c306..6733f14 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -9,7 +9,7 @@ import type { RequestContext } from "../lib/json-rpc-router.js"; import type { JsonRpcErrorResponse, JsonRpcId, JsonRpcRequest, JsonRpcResponse, JsonRpcSubscription } from "../lib/json-rpc.js"; import { requestCounter } from "../metrics.js"; -import { jsonRpcApi } from "../json-rpc-api.js"; +import { jsonRpcRouter } from "../json-rpc-api.js"; import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcSuccessResponse } from "../lib/json-rpc.js"; import { DwnServerError, DwnServerErrorCode } from "../dwn-error.js"; @@ -163,7 +163,7 @@ export class SocketConnection { }; const requestContext = await this.buildRequestContext(jsonRequest); - const { jsonRpcResponse } = await jsonRpcApi.handle(jsonRequest, requestContext); + const { jsonRpcResponse } = await jsonRpcRouter.handle(jsonRequest, requestContext); if (jsonRpcResponse.error) { requestCounter.inc({ method: jsonRequest.method, error: 1 }); } else { @@ -200,7 +200,7 @@ export class SocketConnection { * Adds a `subscriptionHandler` for `Subscribe` messages. */ private async buildRequestContext(request: JsonRpcRequest): Promise { - const { params, method, subscribe } = request; + const { params, method, subscription } = request; const requestContext: RequestContext = { transport : 'ws', @@ -209,12 +209,12 @@ export class SocketConnection { } // methods that expect a long-running subscription begin with `rpc.subscribe.` - if (method.startsWith('rpc.subscribe.') && subscribe) { + if (method.startsWith('rpc.subscribe.') && subscription) { const { message } = params as { message?: GenericMessage }; if (message?.descriptor.method === DwnMethodName.Subscribe) { - const handlerFunc = this.createSubscriptionHandler(subscribe.id); + const handlerFunc = this.createSubscriptionHandler(subscription.id); requestContext.subscriptionRequest = { - id: subscribe.id, + id: subscription.id, subscriptionHandler: (message): void => handlerFunc(message), } } diff --git a/src/http-api.ts b/src/http-api.ts index ff0c1a2..e5bbb24 100644 --- a/src/http-api.ts +++ b/src/http-api.ts @@ -17,7 +17,7 @@ import { createJsonRpcErrorResponse, JsonRpcErrorCodes } from './lib/json-rpc.js import type { DwnServerConfig } from './config.js'; import { config } from './config.js'; import { type DwnServerError } from './dwn-error.js'; -import { jsonRpcApi } from './json-rpc-api.js'; +import { jsonRpcRouter } from './json-rpc-api.js'; import { requestCounter, responseHistogram } from './metrics.js'; import type { RegistrationManager } from './registration/registration-manager.js'; @@ -149,7 +149,7 @@ export class HttpApi { transport : 'http', dataStream : requestDataStream, }; - const { jsonRpcResponse, dataStream: responseDataStream } = await jsonRpcApi.handle(dwnRpcRequest, requestContext as RequestContext); + const { jsonRpcResponse, dataStream: responseDataStream } = await jsonRpcRouter.handle(dwnRpcRequest, requestContext as RequestContext); // If the handler catches a thrown exception and returns a JSON RPC InternalError, return the equivalent // HTTP 500 Internal Server Error with the response. diff --git a/src/index.ts b/src/index.ts index e77275c..77b43b5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,6 @@ export { DwnServerConfig } from './config.js'; export { DwnServer, DwnServerOptions } from './dwn-server.js'; export { HttpApi } from './http-api.js'; -export { jsonRpcApi } from './json-rpc-api.js'; +export { jsonRpcRouter } from './json-rpc-api.js'; export { EStoreType, BackendTypes, StoreType } from './storage.js'; export { WsApi } from './ws-api.js'; diff --git a/src/json-rpc-api.ts b/src/json-rpc-api.ts index bb96f29..d3e4d39 100644 --- a/src/json-rpc-api.ts +++ b/src/json-rpc-api.ts @@ -3,9 +3,9 @@ import { JsonRpcRouter } from './lib/json-rpc-router.js'; import { handleDwnProcessMessage } from './json-rpc-handlers/dwn/index.js'; import { handleSubscriptionsClose } from './json-rpc-handlers/subscription/index.js'; -export const jsonRpcApi = new JsonRpcRouter(); +export const jsonRpcRouter = new JsonRpcRouter(); -jsonRpcApi.on('dwn.processMessage', handleDwnProcessMessage); -jsonRpcApi.on('rpc.subscribe.dwn.processMessage', handleDwnProcessMessage); +jsonRpcRouter.on('dwn.processMessage', handleDwnProcessMessage); +jsonRpcRouter.on('rpc.subscribe.dwn.processMessage', handleDwnProcessMessage); -jsonRpcApi.on('rpc.subscribe.close', handleSubscriptionsClose); +jsonRpcRouter.on('rpc.subscribe.close', handleSubscriptionsClose); diff --git a/src/json-rpc-handlers/subscription/close.ts b/src/json-rpc-handlers/subscription/close.ts index 55de23a..fda3761 100644 --- a/src/json-rpc-handlers/subscription/close.ts +++ b/src/json-rpc-handlers/subscription/close.ts @@ -16,27 +16,27 @@ import { /** * Closes a subscription tied to a specific `SocketConnection`. * - * @param dwnRequest must include JsonRpcId of the subscription request within the `params`. + * @param jsonRpcRequest must include JsonRpcId of the subscription request within a `subscription object`. * @param context must include the associated `SocketConnection`. * */ export const handleSubscriptionsClose: JsonRpcHandler = async ( - dwnRequest, + jsonRpcRequest, context, ) => { - const requestId = dwnRequest.id ?? uuidv4(); + const requestId = jsonRpcRequest.id ?? uuidv4(); if (context.socketConnection === undefined) { const jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidRequest, 'socket connection does not exist'); return { jsonRpcResponse }; } - if (dwnRequest.subscribe === undefined) { + if (jsonRpcRequest.subscription === undefined) { const jsonRpcResponse = createJsonRpcErrorResponse(requestId, JsonRpcErrorCodes.InvalidRequest, 'subscribe options do not exist'); return { jsonRpcResponse }; } const { socketConnection } = context; - const { id } = dwnRequest.subscribe as { id: JsonRpcId }; + const { id } = jsonRpcRequest.subscription as { id: JsonRpcId }; let jsonRpcResponse:JsonRpcResponse; try { diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index eae5a68..2028eb1 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -99,11 +99,11 @@ export class JsonRpcSocket { throw new Error('subscribe rpc requests must include the `rpc.subscribe` prefix'); } - if (!request.subscribe) { + if (!request.subscription) { throw new Error('subscribe rpc requests must include subscribe options'); } - const subscriptionId = request.subscribe.id; + const subscriptionId = request.subscription.id; const messageHandler = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; if (jsonRpcResponse.id === subscriptionId) { diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index 30ee5d5..f555bf2 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -1,5 +1,3 @@ -import { v4 as uuidv4 } from 'uuid'; - export type JsonRpcId = string | number | null; export type JsonRpcParams = any; export type JsonRpcVersion = '2.0'; @@ -9,8 +7,8 @@ export interface JsonRpcRequest { id?: JsonRpcId; method: string; params?: JsonRpcParams; - /** JSON RPC Subscribe Extension Parameters */ - subscribe?: { + /** JSON RPC Subscription Extension Parameters */ + subscription?: { id: JsonRpcId }; } @@ -98,8 +96,8 @@ export const createJsonRpcSubscribeRequest = ( id, method, params, - subscribe: { - id: subscriptionId ?? uuidv4(), + subscription: { + id: subscriptionId, } } } diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index ddf1f04..35e04c5 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -73,10 +73,10 @@ describe('JsonRpcSocket', () => { // initial response const response = createJsonRpcSuccessResponse(request.id, { reply: {} }) socket.send(Buffer.from(JSON.stringify(response))); - const { subscribe } = request; + const { subscription } = request; // send 3 messages for (let i = 0; i < 3; i++) { - const response = createJsonRpcSuccessResponse(subscribe.id, { count: i }); + const response = createJsonRpcSuccessResponse(subscription.id, { count: i }); socket.send(Buffer.from(JSON.stringify(response))); } }); @@ -138,18 +138,18 @@ describe('JsonRpcSocket', () => { // initial response const response = createJsonRpcSuccessResponse(request.id, { reply: {} }) socket.send(Buffer.from(JSON.stringify(response))); - const { subscribe } = request; + const { subscription } = request; // send 1 valid message - const message1 = createJsonRpcSuccessResponse(subscribe.id, { message: 1 }); + const message1 = createJsonRpcSuccessResponse(subscription.id, { message: 1 }); socket.send(Buffer.from(JSON.stringify(message1))); // send a json rpc error - const jsonRpcError = createJsonRpcErrorResponse(subscribe.id, JsonRpcErrorCodes.InternalError, 'some error'); + const jsonRpcError = createJsonRpcErrorResponse(subscription.id, JsonRpcErrorCodes.InternalError, 'some error'); socket.send(Buffer.from(JSON.stringify(jsonRpcError))); // send a 2nd message that shouldn't be handled - const message2 = createJsonRpcSuccessResponse(subscribe.id, { message: 2 }); + const message2 = createJsonRpcSuccessResponse(subscription.id, { message: 2 }); socket.send(Buffer.from(JSON.stringify(message2))); } else if (request.method === 'rpc.subscribe.close') { closed = true; From 6bb0edeb318102bacf84a68d88b3987381a9506d Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Mon, 26 Feb 2024 21:15:42 -0500 Subject: [PATCH 34/40] review suggestions --- src/json-rpc-socket.ts | 14 +++++++------- src/lib/json-rpc.ts | 2 +- tests/json-rpc-socket.spec.ts | 6 +++--- tests/rpc-subscribe-close.spec.ts | 14 ++++++++------ tests/ws-api.spec.ts | 12 ++++++------ 5 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 2028eb1..3e04680 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -3,7 +3,7 @@ import { v4 as uuidv4 } from 'uuid'; import WebSocket from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcResponse } from "./lib/json-rpc.js"; -import { createJsonRpcSubscribeRequest } from "./lib/json-rpc.js"; +import { createJsonRpcSubscriptionRequest } from "./lib/json-rpc.js"; // These were arbitrarily chosen, but can be modified via connect options const CONNECT_TIMEOUT = 3_000; @@ -104,28 +104,28 @@ export class JsonRpcSocket { } const subscriptionId = request.subscription.id; - const messageHandler = (event: { data: any }):void => { + const socketEventListener = (event: { data: any }):void => { const jsonRpcResponse = JSON.parse(event.data.toString()) as JsonRpcResponse; if (jsonRpcResponse.id === subscriptionId) { if (jsonRpcResponse.error !== undefined) { // remove the event listener upon receipt of a JSON RPC Error. - this.socket.removeEventListener('message', messageHandler); + this.socket.removeEventListener('message', socketEventListener); this.closeSubscription(subscriptionId); } listener(jsonRpcResponse); } }; - this.socket.addEventListener('message', messageHandler); + this.socket.addEventListener('message', socketEventListener); const response = await this.request(request); if (response.error) { - this.socket.removeEventListener('message', messageHandler); + this.socket.removeEventListener('message', socketEventListener); return { response } } // clean up listener and create a `rpc.subscribe.close` message to use when closing this JSON RPC subscription const close = async (): Promise => { - this.socket.removeEventListener('message', messageHandler); + this.socket.removeEventListener('message', socketEventListener); await this.closeSubscription(subscriptionId); } @@ -137,7 +137,7 @@ export class JsonRpcSocket { private closeSubscription(id: JsonRpcId): Promise { const requestId = uuidv4(); - const request = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const request = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.close', {}, id); return this.request(request); } diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index f555bf2..8b3550e 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -85,7 +85,7 @@ export const createJsonRpcNotification = ( }; }; -export const createJsonRpcSubscribeRequest = ( +export const createJsonRpcSubscriptionRequest = ( id: JsonRpcId, method: string, params?: JsonRpcParams, diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 35e04c5..d63489f 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -8,7 +8,7 @@ import { WebSocketServer } from 'ws'; import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; import { JsonRpcSocket } from '../src/json-rpc-socket.js'; -import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcRequest, createJsonRpcSubscribeRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; +import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcRequest, createJsonRpcSubscriptionRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; chai.use(chaiAsPromised); @@ -85,7 +85,7 @@ describe('JsonRpcSocket', () => { const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); const requestId = uuidv4(); const subscribeId = uuidv4(); - const request = createJsonRpcSubscribeRequest( + const request = createJsonRpcSubscriptionRequest( requestId, 'rpc.subscribe.test.method', { param1: 'test-param1', param2: 'test-param2' }, @@ -160,7 +160,7 @@ describe('JsonRpcSocket', () => { const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { responseTimeout: 5 }); const requestId = uuidv4(); const subscribeId = uuidv4(); - const request = createJsonRpcSubscribeRequest( + const request = createJsonRpcSubscriptionRequest( requestId, 'rpc.subscribe.test.method', { param1: 'test-param1', param2: 'test-param2' }, diff --git a/tests/rpc-subscribe-close.spec.ts b/tests/rpc-subscribe-close.spec.ts index 06ad6c5..5877387 100644 --- a/tests/rpc-subscribe-close.spec.ts +++ b/tests/rpc-subscribe-close.spec.ts @@ -3,7 +3,7 @@ import sinon from 'sinon'; import { v4 as uuidv4 } from 'uuid'; import type { RequestContext } from '../src/lib/json-rpc-router.js'; -import { JsonRpcErrorCodes, createJsonRpcRequest, createJsonRpcSubscribeRequest } from '../src/lib/json-rpc.js'; +import { JsonRpcErrorCodes, createJsonRpcRequest, createJsonRpcSubscriptionRequest } from '../src/lib/json-rpc.js'; import { getTestDwn } from './test-dwn.js'; import { handleSubscriptionsClose } from '../src/json-rpc-handlers/subscription/close.js'; import { SocketConnection } from '../src/connection/socket-connection.js'; @@ -48,7 +48,7 @@ describe('handleDwnProcessMessage', function () { it('should return an error if close subscription throws ConnectionSubscriptionJsonRpcIdNotFound', async function () { const requestId = uuidv4(); const id = 'some-id'; - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.close', {}, id); const socketConnection = sinon.createStubInstance(SocketConnection); socketConnection.closeSubscription.throws(new DwnServerError( DwnServerErrorCode.ConnectionSubscriptionJsonRpcIdNotFound, @@ -71,7 +71,7 @@ describe('handleDwnProcessMessage', function () { it('should return an error if close subscription throws ConnectionSubscriptionJsonRpcIdNotFound', async function () { const requestId = uuidv4(); const id = 'some-id'; - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.close', {}, id); const socketConnection = sinon.createStubInstance(SocketConnection); socketConnection.closeSubscription.throws(new Error('unknown error')); @@ -91,7 +91,7 @@ describe('handleDwnProcessMessage', function () { it('should return a success', async function () { const requestId = uuidv4(); const id = 'some-id'; - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.close', {}, id); const socketConnection = sinon.createStubInstance(SocketConnection); const dwn = await getTestDwn(); @@ -104,10 +104,10 @@ describe('handleDwnProcessMessage', function () { expect(jsonRpcResponse.error).to.not.exist; }); - it('handler should generate a recordId if one is not provided with the request', async function () { + it('handler should generate a request Id if one is not provided with the request', async function () { const requestId = uuidv4(); const id = 'some-id'; - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.close', {}, id); + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.close', {}, id); delete dwnRequest.id; // delete request id const socketConnection = sinon.createStubInstance(SocketConnection); @@ -120,5 +120,7 @@ describe('handleDwnProcessMessage', function () { context, ); expect(jsonRpcResponse.error).to.not.exist; + expect(jsonRpcResponse.id).to.exist; + expect(jsonRpcResponse.id).to.not.equal(id); }); }); diff --git a/tests/ws-api.spec.ts b/tests/ws-api.spec.ts index 74cc898..34a2150 100644 --- a/tests/ws-api.spec.ts +++ b/tests/ws-api.spec.ts @@ -12,7 +12,7 @@ import { v4 as uuidv4 } from 'uuid'; import { createJsonRpcRequest, - createJsonRpcSubscribeRequest, + createJsonRpcSubscriptionRequest, JsonRpcErrorCodes, } from '../src/lib/json-rpc.js'; import { config } from '../src/config.js'; @@ -112,7 +112,7 @@ describe('websocket api', function () { }; const requestId = uuidv4(); - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, }); @@ -183,7 +183,7 @@ describe('websocket api', function () { const requestId = uuidv4(); const subscribeId = uuidv4(); - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did, }, subscribeId); @@ -266,7 +266,7 @@ describe('websocket api', function () { const requestId = uuidv4(); const subscribeId = uuidv4(); - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did }, subscribeId); @@ -281,7 +281,7 @@ describe('websocket api', function () { // We are checking for the subscription Id not the request Id const request2Id = uuidv4(); - const dwnRequest2 = createJsonRpcSubscribeRequest(request2Id, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest2 = createJsonRpcSubscriptionRequest(request2Id, 'rpc.subscribe.dwn.processMessage', { message: message2, target: alice.did }, subscribeId); @@ -369,7 +369,7 @@ describe('websocket api', function () { const requestId = uuidv4(); const subscribeId = uuidv4(); - const dwnRequest = createJsonRpcSubscribeRequest(requestId, 'rpc.subscribe.dwn.processMessage', { + const dwnRequest = createJsonRpcSubscriptionRequest(requestId, 'rpc.subscribe.dwn.processMessage', { message: message, target: alice.did }, subscribeId); From db403d69477685a8ea754115c8f12457542444ed Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 11:55:46 -0500 Subject: [PATCH 35/40] add coverage to json-rpc-socet --- src/json-rpc-socket.ts | 6 ++++-- tests/json-rpc-socket.spec.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 3e04680..fb3695b 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -18,6 +18,8 @@ export interface JsonRpcSocketOptions { onclose?: () => void; /** optional socket error handler */ onerror?: (error?: any) => void; + /** optional already connected socket to inject */ + socket?: WebSocket; } /** @@ -34,13 +36,13 @@ export class JsonRpcSocket { socket.onclose = onclose; socket.onerror = onerror; - if (socket.onclose === undefined) { + if (!socket.onclose) { socket.onclose = ():void => { log.info(`JSON RPC Socket close ${url}`); } } - if (socket.onerror === undefined) { + if (!socket.onerror) { socket.onerror = (error?: any):void => { log.error(`JSON RPC Socket error ${url}`, error); } diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index d63489f..d773dca 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -9,6 +9,7 @@ import type { JsonRpcId, JsonRpcRequest, JsonRpcSuccessResponse } from '../src/l import { JsonRpcSocket } from '../src/json-rpc-socket.js'; import { JsonRpcErrorCodes, createJsonRpcErrorResponse, createJsonRpcRequest, createJsonRpcSubscriptionRequest, createJsonRpcSuccessResponse } from '../src/lib/json-rpc.js'; +import log from 'loglevel'; chai.use(chaiAsPromised); @@ -207,6 +208,7 @@ describe('JsonRpcSocket', () => { }); it('calls onclose handler', async () => { + // test injected handler const onCloseHandler = { onclose: ():void => {} }; const onCloseSpy = sinon.spy(onCloseHandler, 'onclose'); const client = await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onclose: onCloseHandler.onclose }); @@ -214,6 +216,18 @@ describe('JsonRpcSocket', () => { await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive expect(onCloseSpy.callCount).to.equal(1); + + // test default logger + const logInfoSpy = sinon.spy(log, 'info'); + const defaultClient = await JsonRpcSocket.connect('ws://127.0.0.1:9003'); + defaultClient.close(); + + await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive + expect(logInfoSpy.callCount).to.equal(1); + + // extract log message from argument + const logMessage:string = logInfoSpy.args[0][0]!; + expect(logMessage).to.equal('JSON RPC Socket close ws://127.0.0.1:9003'); }); xit('calls onerror handler', async () => { From 72c8a1dc9fce875d3142ea0fbc538114db75d65a Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 12:33:37 -0500 Subject: [PATCH 36/40] removed test code --- src/connection/socket-connection.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/connection/socket-connection.ts b/src/connection/socket-connection.ts index 6733f14..49625c1 100644 --- a/src/connection/socket-connection.ts +++ b/src/connection/socket-connection.ts @@ -129,12 +129,8 @@ export class SocketConnection { */ private async error(error:Error): Promise{ log.error(`SocketConnection error, terminating connection`, error); - try { - this.socket.terminate(); - await this.close(); - } catch(error) { - console.log('error within error call'); - } + this.socket.terminate(); + await this.close(); } /** From fdc0ee58db333569365368e02b16d05e49475ac2 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 14:35:49 -0500 Subject: [PATCH 37/40] unecessary socket injection --- src/json-rpc-socket.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index fb3695b..0ee4a30 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -18,8 +18,6 @@ export interface JsonRpcSocketOptions { onclose?: () => void; /** optional socket error handler */ onerror?: (error?: any) => void; - /** optional already connected socket to inject */ - socket?: WebSocket; } /** From 077db217f31004dda7240a37c8c7deecc9e13de7 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 15:52:12 -0500 Subject: [PATCH 38/40] remove unecessary JsonRpcParams = any export --- src/json-rpc-socket.ts | 2 +- src/lib/json-rpc.ts | 9 ++++----- tests/json-rpc-socket.spec.ts | 4 +++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/json-rpc-socket.ts b/src/json-rpc-socket.ts index 0ee4a30..3b3cc7b 100644 --- a/src/json-rpc-socket.ts +++ b/src/json-rpc-socket.ts @@ -29,7 +29,7 @@ export class JsonRpcSocket { static async connect(url: string, options: JsonRpcSocketOptions = {}): Promise { const { connectTimeout = CONNECT_TIMEOUT, responseTimeout = RESPONSE_TIMEOUT, onclose, onerror } = options; - const socket = new WebSocket(url); + const socket = new WebSocket(url, { timeout: connectTimeout }); socket.onclose = onclose; socket.onerror = onerror; diff --git a/src/lib/json-rpc.ts b/src/lib/json-rpc.ts index 8b3550e..d54a9cf 100644 --- a/src/lib/json-rpc.ts +++ b/src/lib/json-rpc.ts @@ -1,12 +1,11 @@ export type JsonRpcId = string | number | null; -export type JsonRpcParams = any; export type JsonRpcVersion = '2.0'; export interface JsonRpcRequest { jsonrpc: JsonRpcVersion; id?: JsonRpcId; method: string; - params?: JsonRpcParams; + params?: any; /** JSON RPC Subscription Extension Parameters */ subscription?: { id: JsonRpcId @@ -76,7 +75,7 @@ export const createJsonRpcErrorResponse = ( export const createJsonRpcNotification = ( method: string, - params?: JsonRpcParams, + params?: any, ): JsonRpcRequest => { return { jsonrpc: '2.0', @@ -88,7 +87,7 @@ export const createJsonRpcNotification = ( export const createJsonRpcSubscriptionRequest = ( id: JsonRpcId, method: string, - params?: JsonRpcParams, + params?: any, subscriptionId?: JsonRpcId ): JsonRpcRequest => { return { @@ -105,7 +104,7 @@ export const createJsonRpcSubscriptionRequest = ( export const createJsonRpcRequest = ( id: JsonRpcId, method: string, - params?: JsonRpcParams, + params?: any, ): JsonRpcRequest => { return { jsonrpc: '2.0', diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index d773dca..6822717 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -234,7 +234,9 @@ describe('JsonRpcSocket', () => { const onErrorHandler = { onerror: ():void => {} }; const onErrorSpy = sinon.spy(onErrorHandler, 'onerror'); - await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onerror: onErrorHandler.onerror }); + await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onerror: onErrorHandler.onerror, connectTimeout: 1 }); + // const serverSocket = [...wsServer.clients][0]; + // serverSocket.emit('error', { type: 'error', target: null, message: 'unknown error' }); await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive expect(onErrorSpy.callCount).to.equal(1, 'error'); From ad90dd2360dcf06ef39994f113ab865904746765 Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 15:55:15 -0500 Subject: [PATCH 39/40] update comment --- src/json-rpc-handlers/dwn/process-message.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/json-rpc-handlers/dwn/process-message.ts b/src/json-rpc-handlers/dwn/process-message.ts index 3a12d1c..bed6db3 100644 --- a/src/json-rpc-handlers/dwn/process-message.ts +++ b/src/json-rpc-handlers/dwn/process-message.ts @@ -64,6 +64,7 @@ export const handleDwnProcessMessage: JsonRpcHandler = async ( // if this is a subscription request, we first check if the connection has a subscription with this Id // we do this ahead of time to prevent opening a subscription on the dwn only to close it after attempting to add it to the subscription manager + // otherwise the subscription manager would throw an error that the Id is already in use and we would close the open subscription on the DWN. if (subscriptionRequest !== undefined && socketConnection?.hasSubscription(subscriptionRequest.id)) { const jsonRpcResponse = createJsonRpcErrorResponse( requestId, From 95bb520c75bfcc4dcae0504e514c4992482d8fdc Mon Sep 17 00:00:00 2001 From: Liran Cohen Date: Tue, 27 Feb 2024 15:56:13 -0500 Subject: [PATCH 40/40] remove unused test internals --- tests/json-rpc-socket.spec.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/json-rpc-socket.spec.ts b/tests/json-rpc-socket.spec.ts index 6822717..6f1c4c4 100644 --- a/tests/json-rpc-socket.spec.ts +++ b/tests/json-rpc-socket.spec.ts @@ -230,15 +230,5 @@ describe('JsonRpcSocket', () => { expect(logMessage).to.equal('JSON RPC Socket close ws://127.0.0.1:9003'); }); - xit('calls onerror handler', async () => { - const onErrorHandler = { onerror: ():void => {} }; - const onErrorSpy = sinon.spy(onErrorHandler, 'onerror'); - - await JsonRpcSocket.connect('ws://127.0.0.1:9003', { onerror: onErrorHandler.onerror, connectTimeout: 1 }); - // const serverSocket = [...wsServer.clients][0]; - // serverSocket.emit('error', { type: 'error', target: null, message: 'unknown error' }); - - await new Promise((resolve) => setTimeout(resolve, 5)); // wait for close event to arrive - expect(onErrorSpy.callCount).to.equal(1, 'error'); - }); + xit('calls onerror handler', async () => {}); });