feat: Split up server creation and request handling

This allows us to decouple the WebSocket listening from the HTTP configs,
making these features completely orthogonal.

test/unit/server/BaseHttpServerFactory.test.ts

@@ -1,128 +0,0 @@
-import type { Server } from 'http';
-import request from 'supertest';
-import type { BaseHttpServerOptions } from '../../../src/server/BaseHttpServerFactory';
-import { BaseHttpServerFactory } from '../../../src/server/BaseHttpServerFactory';
-import type { HttpHandler } from '../../../src/server/HttpHandler';
-import type { HttpResponse } from '../../../src/server/HttpResponse';
-import { joinFilePath } from '../../../src/util/PathUtil';
-import { getPort } from '../../util/Util';
-
-const port = getPort('BaseHttpServerFactory');
-
-const handler: jest.Mocked<HttpHandler> = {
-  handleSafe: jest.fn(async(input: { response: HttpResponse }): Promise<void> => {
-    input.response.writeHead(200);
-    input.response.end();
-  }),
-} as any;
-
-describe('A BaseHttpServerFactory', (): void => {
-  let server: Server;
-
-  const options: [string, BaseHttpServerOptions | undefined][] = [
-    [ 'http', undefined ],
-    [ 'https', {
-      https: true,
-      key: joinFilePath(__dirname, '../../assets/https/server.key'),
-      cert: joinFilePath(__dirname, '../../assets/https/server.cert'),
-    }],
-  ];
-
-  describe.each(options)('with %s', (protocol, httpOptions): void => {
-    let rejectTls: string | undefined;
-    beforeAll(async(): Promise<void> => {
-      // Allow self-signed certificate
-      rejectTls = process.env.NODE_TLS_REJECT_UNAUTHORIZED;
-      process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
-
-      const factory = new BaseHttpServerFactory(handler, httpOptions);
-      server = factory.startServer(port);
-    });
-
-    beforeEach(async(): Promise<void> => {
-      jest.clearAllMocks();
-    });
-
-    afterAll(async(): Promise<void> => {
-      process.env.NODE_TLS_REJECT_UNAUTHORIZED = rejectTls;
-      server.close();
-    });
-
-    it('sends incoming requests to the handler.', async(): Promise<void> => {
-      await request(server).get('/').set('Host', 'test.com').expect(200);
-
-      expect(handler.handleSafe).toHaveBeenCalledTimes(1);
-      expect(handler.handleSafe).toHaveBeenLastCalledWith({
-        request: expect.objectContaining({
-          headers: expect.objectContaining({ host: 'test.com' }),
-        }),
-        response: expect.objectContaining({}),
-      });
-    });
-
-    it('returns a 404 when the handler does not do anything.', async(): Promise<void> => {
-      handler.handleSafe.mockResolvedValueOnce(undefined);
-
-      await expect(request(server).get('/').expect(404)).resolves.toBeDefined();
-    });
-
-    it('writes an error to the HTTP response without the stack trace.', async(): Promise<void> => {
-      handler.handleSafe.mockRejectedValueOnce(new Error('dummyError'));
-
-      const res = await request(server).get('/').expect(500);
-      expect(res.headers['content-type']).toBe('text/plain; charset=utf-8');
-      expect(res.text).toBe('Error: dummyError\n');
-    });
-
-    it('does not write an error if the response had been started.', async(): Promise<void> => {
-      handler.handleSafe.mockImplementationOnce(async(input: { response: HttpResponse }): Promise<void> => {
-        input.response.write('content');
-        throw new Error('dummyError');
-      });
-
-      const res = await request(server).get('/');
-      expect(res.text).not.toContain('dummyError');
-    });
-
-    it('throws unknown errors if its handler throw non-Error objects.', async(): Promise<void> => {
-      handler.handleSafe.mockRejectedValueOnce('apple');
-
-      const res = await request(server).get('/').expect(500);
-      expect(res.text).toContain('Unknown error: apple.');
-    });
-
-    it('can handle errors on the HttpResponse.', async(): Promise<void> => {
-      // This just makes sure the logging line is covered.
-      // Actually destroying the request to trigger an error causes issues for supertest
-      handler.handleSafe.mockImplementationOnce(async(input): Promise<void> => {
-        input.request.emit('error', new Error('bad request'));
-      });
-      await request(server).get('/').expect(404);
-    });
-  });
-
-  describe('with showStackTrace enabled', (): void => {
-    const httpOptions = {
-      http: true,
-      showStackTrace: true,
-    };
-
-    beforeAll(async(): Promise<void> => {
-      const factory = new BaseHttpServerFactory(handler, httpOptions);
-      server = factory.startServer(port);
-    });
-
-    afterAll(async(): Promise<void> => {
-      server.close();
-    });
-
-    it('does not print the stack if that option is disabled.', async(): Promise<void> => {
-      const error = new Error('dummyError');
-      handler.handleSafe.mockRejectedValueOnce(error);
-
-      const res = await request(server).get('/').expect(500);
-      expect(res.headers['content-type']).toBe('text/plain; charset=utf-8');
-      expect(res.text).toBe(`${error.stack}\n`);
-    });
-  });
-});

test/unit/server/BaseServerFactory.test.ts

@@ -0,0 +1,75 @@
+import type { RequestListener, Server } from 'http';
+import request from 'supertest';
+import type { BaseServerFactoryOptions } from '../../../src/server/BaseServerFactory';
+import { BaseServerFactory } from '../../../src/server/BaseServerFactory';
+import type { ServerConfigurator } from '../../../src/server/ServerConfigurator';
+import { joinFilePath } from '../../../src/util/PathUtil';
+import { getPort } from '../../util/Util';
+
+const port = getPort('BaseServerFactory');
+
+describe('A BaseServerFactory', (): void => {
+  let server: Server;
+
+  const options: [string, BaseServerFactoryOptions | undefined][] = [
+    [ 'http', undefined ],
+    [ 'https', {
+      https: true,
+      key: joinFilePath(__dirname, '../../assets/https/server.key'),
+      cert: joinFilePath(__dirname, '../../assets/https/server.cert'),
+    }],
+  ];
+
+  describe.each(options)('with %s', (protocol, httpOptions): void => {
+    let rejectTls: string | undefined;
+    let configurator: ServerConfigurator;
+    let mockRequestHandler: jest.MockedFn<RequestListener>;
+
+    beforeAll(async(): Promise<void> => {
+      // Allow self-signed certificate
+      rejectTls = process.env.NODE_TLS_REJECT_UNAUTHORIZED;
+      process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';
+
+      mockRequestHandler = jest.fn();
+
+      configurator = {
+        async handleSafe(serv: Server): Promise<void> {
+          serv.on('request', mockRequestHandler);
+        },
+      } as any;
+
+      const factory = new BaseServerFactory(configurator, httpOptions);
+      server = await factory.createServer();
+
+      server.listen(port);
+    });
+
+    beforeEach(async(): Promise<void> => {
+      jest.clearAllMocks();
+    });
+
+    afterAll(async(): Promise<void> => {
+      process.env.NODE_TLS_REJECT_UNAUTHORIZED = rejectTls;
+      server.close();
+    });
+
+    it('emits a request event on requests.', async(): Promise<void> => {
+      let resolveProm: (value: unknown) => void;
+      const requestProm = new Promise((resolve): void => {
+        resolveProm = resolve;
+      });
+      server.on('request', (req, res): void => {
+        resolveProm(req);
+        res.writeHead(200);
+        res.end();
+      });
+      await request(server).get('/').set('Host', 'test.com').expect(200);
+
+      await expect(requestProm).resolves.toEqual(expect.objectContaining({
+        headers: expect.objectContaining({ host: 'test.com' }),
+      }));
+
+      expect(mockRequestHandler).toHaveBeenCalledTimes(1);
+    });
+  });
+});

test/unit/server/HandlerServerConfigurator.test.ts

@@ -0,0 +1,143 @@
+import { EventEmitter } from 'events';
+import type { ServerResponse, IncomingMessage, Server } from 'http';
+import { Readable } from 'stream';
+import type { Logger } from '../../../src/logging/Logger';
+import { getLoggerFor } from '../../../src/logging/LogUtil';
+import { HandlerServerConfigurator } from '../../../src/server/HandlerServerConfigurator';
+import type { HttpHandler } from '../../../src/server/HttpHandler';
+import { flushPromises } from '../../util/Util';
+
+jest.mock('../../../src/logging/LogUtil', (): any => {
+  const logger: Logger =
+    { error: jest.fn(), info: jest.fn() } as any;
+  return { getLoggerFor: (): Logger => logger };
+});
+
+describe('A HandlerServerConfigurator', (): void => {
+  const logger: jest.Mocked<Logger> = getLoggerFor('mock') as any;
+  let request: jest.Mocked<IncomingMessage>;
+  let response: jest.Mocked<ServerResponse>;
+  let server: Server;
+  let handler: jest.Mocked<HttpHandler>;
+  let listener: HandlerServerConfigurator;
+
+  beforeEach(async(): Promise<void> => {
+    // Clearing the logger mock
+    jest.clearAllMocks();
+    request = Readable.from('') as any;
+    request.method = 'GET';
+    request.url = '/';
+
+    response = {
+      headersSent: false,
+      end: jest.fn(),
+      setHeader: jest.fn(),
+      writeHead: jest.fn(),
+    } as any;
+    response.end.mockImplementation((): any => {
+      response.headersSent = true;
+    });
+    response.writeHead.mockReturnValue(response);
+
+    server = new EventEmitter() as any;
+
+    handler = {
+      handleSafe: jest.fn((): void => {
+        response.headersSent = true;
+      }),
+    } as any;
+
+    listener = new HandlerServerConfigurator(handler);
+    await listener.handle(server);
+  });
+
+  it('sends incoming requests to the handler.', async(): Promise<void> => {
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(handler.handleSafe).toHaveBeenCalledTimes(1);
+    expect(handler.handleSafe).toHaveBeenLastCalledWith({ request, response });
+
+    expect(response.setHeader).toHaveBeenCalledTimes(0);
+    expect(response.writeHead).toHaveBeenCalledTimes(0);
+    expect(response.end).toHaveBeenCalledTimes(0);
+  });
+
+  it('returns a 404 when the handler does not do anything.', async(): Promise<void> => {
+    handler.handleSafe.mockImplementation(jest.fn());
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(response.setHeader).toHaveBeenCalledTimes(0);
+    expect(response.writeHead).toHaveBeenCalledTimes(1);
+    expect(response.writeHead).toHaveBeenLastCalledWith(404);
+    expect(response.end).toHaveBeenCalledTimes(1);
+    expect(response.end).toHaveBeenLastCalledWith();
+  });
+
+  it('writes an error to the HTTP response without the stack trace.', async(): Promise<void> => {
+    handler.handleSafe.mockRejectedValueOnce(new Error('dummyError'));
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(response.setHeader).toHaveBeenCalledTimes(1);
+    expect(response.setHeader).toHaveBeenLastCalledWith('Content-Type', 'text/plain; charset=utf-8');
+    expect(response.writeHead).toHaveBeenCalledTimes(1);
+    expect(response.writeHead).toHaveBeenLastCalledWith(500);
+    expect(response.end).toHaveBeenCalledTimes(1);
+    expect(response.end).toHaveBeenLastCalledWith('Error: dummyError\n');
+  });
+
+  it('does not write an error if the response had been started.', async(): Promise<void> => {
+    response.headersSent = true;
+    handler.handleSafe.mockRejectedValueOnce(new Error('dummyError'));
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(response.setHeader).toHaveBeenCalledTimes(0);
+    expect(response.writeHead).toHaveBeenCalledTimes(0);
+    expect(response.end).toHaveBeenCalledTimes(1);
+    expect(response.end).toHaveBeenLastCalledWith();
+  });
+
+  it('throws unknown errors if its handler throw non-Error objects.', async(): Promise<void> => {
+    handler.handleSafe.mockRejectedValueOnce('apple');
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(response.setHeader).toHaveBeenCalledTimes(1);
+    expect(response.setHeader).toHaveBeenLastCalledWith('Content-Type', 'text/plain; charset=utf-8');
+    expect(response.writeHead).toHaveBeenCalledTimes(1);
+    expect(response.writeHead).toHaveBeenLastCalledWith(500);
+    expect(response.end).toHaveBeenCalledTimes(1);
+    expect(response.end).toHaveBeenLastCalledWith('Unknown error: apple.\n');
+  });
+
+  it('can handle errors on the HttpResponse.', async(): Promise<void> => {
+    handler.handleSafe.mockImplementationOnce(async(input): Promise<void> => {
+      input.request.emit('error', new Error('bad request'));
+    });
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(logger.error).toHaveBeenCalledTimes(1);
+    expect(logger.error).toHaveBeenLastCalledWith('Request error: bad request');
+  });
+
+  it('prints the stack trace if that option is enabled.', async(): Promise<void> => {
+    server.removeAllListeners();
+    listener = new HandlerServerConfigurator(handler, true);
+    await listener.handle(server);
+    const error = new Error('dummyError');
+    handler.handleSafe.mockRejectedValueOnce(error);
+    server.emit('request', request, response);
+    await flushPromises();
+
+    expect(response.setHeader).toHaveBeenCalledTimes(1);
+    expect(response.setHeader).toHaveBeenLastCalledWith('Content-Type', 'text/plain; charset=utf-8');
+    expect(response.writeHead).toHaveBeenCalledTimes(1);
+    expect(response.writeHead).toHaveBeenLastCalledWith(500);
+    expect(response.end).toHaveBeenCalledTimes(1);
+    expect(response.end).toHaveBeenLastCalledWith(`${error.stack}\n`);
+  });
+});

test/unit/server/WebSocketServerConfigurator.test.ts

@@ -0,0 +1,80 @@
+import { EventEmitter } from 'events';
+import type { Server } from 'http';
+import type { WebSocket } from 'ws';
+import type { Logger } from '../../../src/logging/Logger';
+import { getLoggerFor } from '../../../src/logging/LogUtil';
+import type { HttpRequest } from '../../../src/server/HttpRequest';
+import { WebSocketServerConfigurator } from '../../../src/server/WebSocketServerConfigurator';
+import { flushPromises } from '../../util/Util';
+
+jest.mock('ws', (): any => ({
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  WebSocketServer: jest.fn().mockImplementation((): any => ({
+    handleUpgrade(upgradeRequest: any, socket: any, head: any, callback: any): void {
+      callback(socket, upgradeRequest);
+    },
+  })),
+}));
+
+jest.mock('../../../src/logging/LogUtil', (): any => {
+  const logger: Logger =
+    { error: jest.fn(), info: jest.fn() } as any;
+  return { getLoggerFor: (): Logger => logger };
+});
+
+class SimpleWebSocketConfigurator extends WebSocketServerConfigurator {
+  public async handleConnection(): Promise<void> {
+    // Will be overwritten
+  }
+}
+
+describe('A WebSocketServerConfigurator', (): void => {
+  const logger: jest.Mocked<Logger> = getLoggerFor('mock') as any;
+  let server: Server;
+  let webSocket: WebSocket;
+  let upgradeRequest: HttpRequest;
+  let listener: jest.Mocked<SimpleWebSocketConfigurator>;
+
+  beforeEach(async(): Promise<void> => {
+    // Clearing the logger mock
+    jest.clearAllMocks();
+    server = new EventEmitter() as any;
+    webSocket = new EventEmitter() as any;
+    webSocket.send = jest.fn();
+    webSocket.close = jest.fn();
+
+    upgradeRequest = { url: `/foo` } as any;
+
+    listener = new SimpleWebSocketConfigurator() as any;
+    listener.handleConnection = jest.fn().mockResolvedValue('');
+    await listener.handle(server);
+  });
+
+  it('attaches an upgrade listener to any server it gets.', async(): Promise<void> => {
+    server = new EventEmitter() as any;
+    expect(server.listenerCount('upgrade')).toBe(0);
+    await listener.handle(server);
+    expect(server.listenerCount('upgrade')).toBe(1);
+  });
+
+  it('calls the handleConnection function when there is a new WebSocket.', async(): Promise<void> => {
+    server.emit('upgrade', upgradeRequest, webSocket);
+
+    await flushPromises();
+
+    expect(listener.handleConnection).toHaveBeenCalledTimes(1);
+    expect(listener.handleConnection).toHaveBeenLastCalledWith(webSocket, upgradeRequest);
+    expect(logger.error).toHaveBeenCalledTimes(0);
+  });
+
+  it('logs an error if something went wrong handling the connection.', async(): Promise<void> => {
+    listener.handleConnection.mockRejectedValue(new Error('bad input'));
+    server.emit('upgrade', upgradeRequest, webSocket);
+
+    await flushPromises();
+
+    expect(listener.handleConnection).toHaveBeenCalledTimes(1);
+    expect(logger.error).toHaveBeenCalledTimes(1);
+    expect(logger.error).toHaveBeenLastCalledWith('Something went wrong handling a WebSocket connection: bad input');
+  });
+});

test/unit/server/WebSocketServerFactory.test.ts

@@ -1,54 +0,0 @@
-import type { Server } from 'http';
-import request from 'supertest';
-import { WebSocket } from 'ws';
-import { BaseHttpServerFactory } from '../../../src/server/BaseHttpServerFactory';
-import type { HttpHandlerInput } from '../../../src/server/HttpHandler';
-import { HttpHandler } from '../../../src/server/HttpHandler';
-import type { HttpRequest } from '../../../src/server/HttpRequest';
-import { WebSocketHandler } from '../../../src/server/WebSocketHandler';
-import { WebSocketServerFactory } from '../../../src/server/WebSocketServerFactory';
-
-class SimpleHttpHandler extends HttpHandler {
-  public async handle(input: HttpHandlerInput): Promise<void> {
-    input.response.end('SimpleHttpHandler');
-  }
-}
-
-class SimpleWebSocketHandler extends WebSocketHandler {
-  public host: any;
-
-  public async handle(input: { webSocket: WebSocket; upgradeRequest: HttpRequest }): Promise<void> {
-    input.webSocket.send('SimpleWebSocketHandler');
-    input.webSocket.close();
-    this.host = input.upgradeRequest.headers.host;
-  }
-}
-
-describe('SimpleWebSocketHandler', (): void => {
-  let webSocketHandler: SimpleWebSocketHandler;
-  let server: Server;
-
-  beforeAll(async(): Promise<void> => {
-    const httpHandler = new SimpleHttpHandler();
-    webSocketHandler = new SimpleWebSocketHandler();
-    const httpServerFactory = new BaseHttpServerFactory(httpHandler);
-    const webSocketServerFactory = new WebSocketServerFactory(httpServerFactory, webSocketHandler);
-    server = webSocketServerFactory.startServer(5556);
-  });
-
-  afterAll(async(): Promise<void> => {
-    server.close();
-  });
-
-  it('has a functioning HTTP interface.', async(): Promise<void> => {
-    const result = await request(server).get('/').expect('SimpleHttpHandler');
-    expect(result).toBeDefined();
-  });
-
-  it('has a functioning WebSockets interface.', async(): Promise<void> => {
-    const client = new WebSocket('ws://localhost:5556');
-    const buffer = await new Promise<Buffer>((resolve): any => client.on('message', resolve));
-    expect(buffer.toString()).toBe('SimpleWebSocketHandler');
-    expect(webSocketHandler.host).toBe('localhost:5556');
-  });
-});