Merge pull request #862 from open-rpc/feat/allow-passing-express-app-for-integration

Allow passing existing connect app

Author: shanejonas

README.md

@@ -153,10 +153,14 @@ const UdpIpcTransport = new IPCServerTranport(UDPIPCTransportOptions);
 
 ```
 import { HTTPServerTransport, HTTPSServerTransport } from "@open-rpc/server-js";
+import express from "express";
+
+const existingApp = express();
 
 const httpOptions = {
   middleware: [ cors({ origin: "*" }) ],
-  port: 4345
+  port: 4345,
+  app: existingApp, // optional existing express/connect app
 };
 const httpsOptions = { // extends https://nodejs.org/api/https.html#https_https_createserver_options_requestlistener
   middleware: [ cors({ origin: "*" }) ],

package.json

@@ -20,7 +20,8 @@
     "build": "tsc",
     "watch:build": "tsc --watch",
     "watch:test": "jest --watch",
-    "lint": "eslint . --ext .ts"
+    "lint": "eslint . --ext .ts",
+    "lint:fix": "eslint . --ext .ts --fix"
   },
   "author": "",
   "license": "Apache-2.0",

src/server.test.ts

@@ -0,0 +1,189 @@
+/* eslint-disable @typescript-eslint/no-var-requires */
+import Server from './server';
+import { ServerTransport } from './transports/server-transport';
+import { Router } from './router';
+
+// Create test classes we'll use directly
+class TestRouter implements Partial<Router> {
+  public isMethodImplemented = jest.fn();
+  public call = jest.fn();
+}
+
+class TestTransport extends ServerTransport {
+  public options: any;
+  
+  constructor(options: any = {}) {
+    super();
+    this.options = options;
+  }
+  
+  public start = jest.fn().mockResolvedValue(undefined);
+  public stop = jest.fn().mockResolvedValue(undefined);
+}
+
+// Create factory functions to use in tests
+const createTestRouter = () => new TestRouter() as unknown as Router;
+const createTestTransport = (options: any = {}) => new TestTransport(options);
+
+// Mock MethodCallValidator
+jest.mock('@open-rpc/schema-utils-js', () => ({
+  MethodCallValidator: jest.fn().mockImplementation(() => ({
+    validate: jest.fn(),
+  })),
+}));
+
+describe('Server', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+    jest.spyOn(console, 'log').mockImplementation(() => { /* no-op */ });
+  });
+
+  it('initializes without routers or transports when no options provided', () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    expect((server as any).routers).toHaveLength(0);
+    expect((server as any).transports).toHaveLength(0);
+  });
+
+  it('adds router when constructed with methodMapping', () => {
+    // Mock Router constructor
+    const originalRouter = require('./router').Router;
+    const mockRouter = createTestRouter();
+    require('./router').Router = jest.fn().mockReturnValue(mockRouter);
+    
+    const mapping = {} as any;
+    const server = new Server({ openrpcDocument: {} as any, methodMapping: mapping });
+    
+    // Verify Router was created with expected args
+    expect(require('./router').Router).toHaveBeenCalledWith({} as any, mapping);
+    expect((server as any).routers).toHaveLength(1);
+    
+    // Restore original Router
+    require('./router').Router = originalRouter;
+  });
+
+  it('adds default transport when constructed with transportConfigs', () => {
+    // Mock the transport factory
+    const originalTransports = require('./transports').default;
+    const mockTransport = createTestTransport({ port: 123 });
+    require('./transports').default = { 
+      HTTPTransport: jest.fn().mockReturnValue(mockTransport)
+    };
+    
+    // Mock the Router class to prevent it from trying to use actual MethodCallValidator
+    const originalRouter = require('./router').Router;
+    require('./router').Router = jest.fn().mockReturnValue(createTestRouter());
+    
+    const opts = { port: 123 } as any;
+    const server = new Server({
+      openrpcDocument: {} as any,
+      methodMapping: {} as any,
+      transportConfigs: [{ type: 'HTTPTransport', options: opts }],
+    });
+    
+    expect(console.log).toHaveBeenCalledWith(
+      `Adding Transport of the type HTTPTransport on port ${opts.port}`,
+    );
+    
+    expect((server as any).transports).toHaveLength(1);
+    expect((server as any).transports[0]).toBe(mockTransport);
+    expect(mockTransport.options).toEqual(opts);
+    
+    // Restore original modules
+    require('./transports').default = originalTransports;
+    require('./router').Router = originalRouter;
+  });
+
+  it('throws error on invalid transport type in addDefaultTransport', () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    expect(() => {
+      (server as any).addDefaultTransport('InvalidTransport' as any, {} as any);
+    }).toThrow(
+      'The transport "InvalidTransport" is not a valid transport type.',
+    );
+  });
+
+  it('registers transport and attaches existing routers in addTransport', () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    
+    // Create test data
+    const router1 = createTestRouter();
+    const router2 = createTestRouter();
+    (server as any).routers = [router1, router2];
+    
+    const transport = createTestTransport();
+    jest.spyOn(transport, 'addRouter');
+    
+    server.addTransport(transport);
+    
+    expect(transport.addRouter).toHaveBeenCalledTimes(2);
+    expect(transport.addRouter).toHaveBeenCalledWith(router1);
+    expect(transport.addRouter).toHaveBeenCalledWith(router2);
+    expect((server as any).transports).toContain(transport);
+  });
+
+  it('registers router and attaches to existing transports in addRouter', () => {
+    // Mock Router constructor
+    const originalRouter = require('./router').Router;
+    const mockRouter = createTestRouter();
+    require('./router').Router = jest.fn().mockReturnValue(mockRouter);
+    
+    const server = new Server({ openrpcDocument: {} as any });
+    
+    // Create test data
+    const transport = createTestTransport();
+    jest.spyOn(transport, 'addRouter');
+    (server as any).transports = [transport];
+    
+    const router = server.addRouter({} as any, {} as any);
+    
+    expect(require('./router').Router).toHaveBeenCalledWith({}, {} as any);
+    expect(transport.addRouter).toHaveBeenCalledWith(router);
+    expect((server as any).routers).toContain(router);
+    
+    // Restore original Router
+    require('./router').Router = originalRouter;
+  });
+
+  it('deregisters router and detaches from transports in removeRouter', () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    
+    // Create test data
+    const router = createTestRouter();
+    const transport = createTestTransport();
+    jest.spyOn(transport, 'removeRouter');
+    
+    (server as any).transports = [transport];
+    (server as any).routers = [router];
+    
+    server.removeRouter(router);
+    
+    expect((server as any).routers).not.toContain(router);
+    expect(transport.removeRouter).toHaveBeenCalledWith(router);
+  });
+
+  it('calls start on transports in start', async () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    
+    // Create test data
+    const transport = createTestTransport();
+    
+    (server as any).transports = [transport];
+    
+    await server.start();
+    
+    expect(transport.start).toHaveBeenCalled();
+  });
+
+  it('calls stop on transports in stop', async () => {
+    const server = new Server({ openrpcDocument: {} as any });
+    
+    // Create test data
+    const transport = createTestTransport();
+    
+    (server as any).transports = [transport];
+    
+    await server.stop();
+    
+    expect(transport.stop).toHaveBeenCalled();
+  });
+});

src/server.ts

@@ -71,8 +71,16 @@ export default class Server {
     this.transports.forEach((transport) => transport.removeRouter(routerToRemove));
   }
 
-  public start() {
-    this.transports.forEach((transport) => transport.start());
+  public async start() {
+    for (const transport of this.transports) {
+      await transport.start();
+    }
+  }
+
+  public async stop() {
+    for (const transport of this.transports) {
+      await transport.stop();
+    }
   }
 
 }

src/transports/http.test.ts

@@ -3,6 +3,8 @@ import { parseOpenRPCDocument } from "@open-rpc/schema-utils-js";
 import { Router } from "../router";
 import HTTPTransport from "./http";
 import { JSONRPCResponse } from "./server-transport";
+import connect from "connect";
+import http from "http";
 
 describe("http transport", () => {
   let transport: HTTPTransport;
@@ -17,11 +19,11 @@ describe("http transport", () => {
 
     transport.addRouter(router);
 
-    transport.start();
+    await transport.start();
   });
 
-  afterAll(() => {
-    transport.stop();
+  afterAll(async () => {
+    await transport.stop();
   });
 
   it("can start an http server that works", async () => {
@@ -60,4 +62,84 @@ describe("http transport", () => {
     const pluckedResult = result.map((r: JSONRPCResponse) => r.result);
     expect(pluckedResult).toEqual([4, 8]);
   });
+
+  it("allows using an existing app", async () => {
+    const app = connect();
+    const simpleMathExample = await parseOpenRPCDocument(examples.simpleMath);
+    const localTransport = new HTTPTransport({ middleware: [], port: 9700, app });
+    const router = new Router(simpleMathExample, { mockMode: true });
+    localTransport.addRouter(router);
+
+    try {
+      await localTransport.start();
+
+      const { result } = await fetch("http://localhost:9700", {
+        body: JSON.stringify({
+          id: "2",
+          jsonrpc: "2.0",
+          method: "addition",
+          params: [2, 2],
+        }),
+        headers: { "Content-Type": "application/json" },
+        method: "post",
+      }).then((res) => res.json() as Promise<JSONRPCResponse>);
+
+      expect(result).toBe(4);
+    } finally {
+      await localTransport.stop();
+    }
+  }, 30000);
+
+  it("handles errors when stopping the server", async () => {
+    const errorTransport = new HTTPTransport({
+      middleware: [],
+      port: 9703,
+    });
+    let serverInstance: any;
+    let originalCloseFn: ((callback?: (err?: Error) => void) => http.Server) | null = null;
+
+    try {
+      await errorTransport.start();
+      serverInstance = (errorTransport as any).server;
+      originalCloseFn = serverInstance.close.bind(serverInstance);
+
+      const mockError = new Error("Mock close error");
+      serverInstance.close = (callback: (err?: Error) => void) => {
+        callback(mockError);
+        originalCloseFn?.((_err?: Error) => { /* an actual close attempt */ });
+      };
+
+      await expect(errorTransport.stop()).rejects.toThrow("Mock close error");
+
+    } finally {
+      if (serverInstance && serverInstance.listening) {
+        // Restore original close method before attempting to stop for cleanup
+        if (originalCloseFn) {
+          serverInstance.close = originalCloseFn;
+        }
+        try {
+          await errorTransport.stop(); // Attempt to stop for cleanup
+        } catch (cleanupError) {
+          // Ignore cleanup errors if the main test assertion passed/failed as expected
+          console.warn("Error during test server cleanup (port 9703):", cleanupError);
+        }
+      }
+    }
+  });
+
+  it("handles errors when starting the server", async () => {
+    const errorTransport = new HTTPTransport({
+      middleware: [],
+      port: 9707,
+    });
+    const serverInstance = (errorTransport as any).server;
+    const originalListen = serverInstance.listen.bind(serverInstance);
+    serverInstance.listen = (port: number, cb: (err?: Error) => void) => {
+      cb(new Error("Mock listen error"));
+      return serverInstance;
+    };
+    await expect(errorTransport.start()).rejects.toThrow("Mock listen error");
+    serverInstance.listen = originalListen;
+    // Do not call stop, since server never started
+  });
 });