From 61d81d4771e827a713f5f13ba372a37cbb4b05f5 Mon Sep 17 00:00:00 2001
From: Camden Phalen <cphalen@hubspot.com>
Date: Wed, 4 Oct 2023 13:47:30 -0400
Subject: [PATCH] Add logging/errors to PortManagerServer

---
 constants/ports.ts                  |  2 ++
 lang/en.lyaml                       | 11 +++++++++
 lib/portManager.ts                  |  2 +-
 {lib => utils}/PortManagerServer.ts | 38 ++++++++++++++++++++++-------
 4 files changed, 43 insertions(+), 10 deletions(-)
 rename {lib => utils}/PortManagerServer.ts (72%)

diff --git a/constants/ports.ts b/constants/ports.ts
index d0fe422f..b99a0901 100644
--- a/constants/ports.ts
+++ b/constants/ports.ts
@@ -1,3 +1,5 @@
 export const MIN_PORT_NUMBER = 1024;
 export const MAX_PORT_NUMBER = 65535;
+
+// TODO: What's a good port for the port manager?
 export const PORT_MANAGER_SERVER_PORT = 12345;
diff --git a/lang/en.lyaml b/lang/en.lyaml
index 591844fa..bde6d2ee 100644
--- a/lang/en.lyaml
+++ b/lang/en.lyaml
@@ -112,6 +112,12 @@ en:
         creatingPath: "Making {{ path }} if needed"
       logging:
         creatingFile: "Creating file at {{ path }}"
+    utils:
+      PortManagerServer:
+        started: "PortManagerServer running on port {{ port }}"
+        setPort: "Server with instanceId {{ instanceId }} assigned to port {{ port }}"
+        deletedPort: "Server with instanceId {{ instanceId }} unassigned from port {{ port }}"
+        close: "PortManagerServer shutting down."
   errors:
     archive:
       extractZip:
@@ -175,6 +181,11 @@ en:
         configIgnore: "Unable to determine if config file is properly ignored by git."
       notify:
         filePath: "Unable to notify file '{{ filePath }}'"
+      PortManagerServer:
+        duplicateInstance: "Failed to start PortManagerServer. An instance of PortManagerServer is already running."
+        404: "Could not find a server with instanceId {{ instanceId }}"
+        409: "Failed to assign port. Server with instanceId {{ instanceId }} is already running on port {{ port }}"
+        400: "Invalid port requested. Port must be between 1024 and 65535."
     config:
       cliConfiguration:
         noConfigLoaded: "No config loaded."
diff --git a/lib/portManager.ts b/lib/portManager.ts
index 53097c21..d5531c62 100644
--- a/lib/portManager.ts
+++ b/lib/portManager.ts
@@ -1,6 +1,6 @@
 import axios from 'axios';
 
-import PortManagerServer from './PortManagerServer';
+import PortManagerServer from '../utils/PortManagerServer';
 import { detectPort } from '../utils/detectPort';
 import { PORT_MANAGER_SERVER_PORT } from '../constants/ports';
 
diff --git a/lib/PortManagerServer.ts b/utils/PortManagerServer.ts
similarity index 72%
rename from lib/PortManagerServer.ts
rename to utils/PortManagerServer.ts
index ec051318..4fcc6c62 100644
--- a/lib/PortManagerServer.ts
+++ b/utils/PortManagerServer.ts
@@ -2,17 +2,22 @@ import express, { Express, Request, Response } from 'express';
 import { Server } from 'http';
 import cors from 'cors';
 
-import { detectPort } from '../utils/detectPort';
+import { detectPort } from './detectPort';
 import {
   MIN_PORT_NUMBER,
   MAX_PORT_NUMBER,
   PORT_MANAGER_SERVER_PORT,
 } from '../constants/ports';
+import { throwErrorWithMessage } from '../errors/standardErrors';
+import { debug } from './logger';
+import { i18n } from './lang';
 
 type ServerPortMap = {
   [instanceId: string]: number;
 };
 
+const i18nKey = 'utils.PortManagerServer';
+
 class PortManagerServer {
   app?: Express;
   server?: Server;
@@ -24,14 +29,16 @@ class PortManagerServer {
 
   init(): void {
     if (this.app) {
-      throw new Error('Port manager server is already running');
+      throwErrorWithMessage(`${i18nKey}.duplicateInstance`);
     }
     this.app = express();
     this.app.use(express.json());
     this.app.use(cors());
     this.setupRoutes();
     this.server = this.app.listen(PORT_MANAGER_SERVER_PORT, () =>
-      console.log('Running port manager')
+      debug(`${i18nKey}.started`, {
+        port: PORT_MANAGER_SERVER_PORT,
+      })
     );
   }
 
@@ -48,13 +55,22 @@ class PortManagerServer {
   }
 
   setPort(instanceId: string, port: number) {
+    debug(`${i18nKey}.setPort`, { instanceId, port });
     this.serverPortMap[instanceId] = port;
   }
 
+  deletePort(instanceId: string) {
+    debug(`${i18nKey}.deletePort`, {
+      instanceId,
+      port: this.serverPortMap[instanceId],
+    });
+    delete this.serverPortMap[instanceId];
+  }
+
   send404(res: Response, instanceId: string) {
     res
       .status(404)
-      .send(`Could not find a server with instanceId ${instanceId}`);
+      .send(i18n(`errors.${i18nKey}.404`, { instanceId: instanceId }));
   }
 
   getServers = async (req: Request, res: Response): Promise<void> => {
@@ -79,11 +95,14 @@ class PortManagerServer {
     const { instanceId, port } = req.body;
 
     if (this.serverPortMap[instanceId]) {
-      res
-        .status(409)
-        .send('This server instance has already been assigned a port');
+      res.status(409).send(
+        i18n(`errors.${i18nKey}.409`, {
+          instanceId,
+          port: this.serverPortMap[instanceId],
+        })
+      );
     } else if (port && (port < MIN_PORT_NUMBER || port > MAX_PORT_NUMBER)) {
-      res.status(400).send('Port must be between 1024 and 65535');
+      res.status(400).send(i18n(`errors.${i18nKey}.400`));
     } else {
       const portToUse = await detectPort(port);
       this.setPort(instanceId, portToUse);
@@ -97,7 +116,7 @@ class PortManagerServer {
     const port = this.serverPortMap[instanceId];
 
     if (port) {
-      delete this.serverPortMap[instanceId];
+      this.deletePort(instanceId);
       res.send(200);
     } else {
       this.send404(res, instanceId);
@@ -106,6 +125,7 @@ class PortManagerServer {
 
   closeServer = (req: Request, res: Response): void => {
     if (this.server) {
+      debug(`${i18nKey}.close`);
       res.send(200);
       this.server.close();
     }