Standardize disposals

Author: code-asher

src/node/main.ts

@@ -157,7 +157,6 @@ export const runCodeServer = async (
   }
 
   let linkAgent: undefined | ChildProcessWithoutNullStreams
-
   try {
     linkAgent = startLink(serverAddress)
     linkAgent.on("error", (error) => {
@@ -194,13 +193,11 @@ export const runCodeServer = async (
     }
   }
 
-  const dispose = () => {
-    disposeApp()
-    linkAgent?.kill()
-  }
-
   return {
     server,
-    dispose,
+    dispose: () => {
+      disposeApp()
+      linkAgent?.kill()
+    },
   }
 }

src/node/routes/index.ts

@@ -15,7 +15,6 @@ import { Heart } from "../heart"
 import { ensureAuthenticated, redirect } from "../http"
 import { PluginAPI } from "../plugin"
 import { getMediaMime, paths } from "../util"
-import { wrapper } from "../wrapper"
 import * as apps from "./apps"
 import * as domainProxy from "./domainProxy"
 import { errorHandler, wsErrorHandler } from "./errors"
@@ -46,9 +45,6 @@ export const register = async (
       })
     })
   })
-  server.on("close", () => {
-    heart.dispose()
-  })
 
   app.disable("x-powered-by")
   wsApp.disable("x-powered-by")
@@ -114,13 +110,13 @@ export const register = async (
     })
   })
 
+  let pluginApi: PluginAPI
   if (!process.env.CS_DISABLE_PLUGINS) {
     const workingDir = args._ && args._.length > 0 ? path.resolve(args._[args._.length - 1]) : undefined
-    const pluginApi = new PluginAPI(logger, process.env.CS_PLUGIN, process.env.CS_PLUGIN_PATH, workingDir)
+    pluginApi = new PluginAPI(logger, process.env.CS_PLUGIN, process.env.CS_PLUGIN_PATH, workingDir)
     await pluginApi.loadPlugins()
     pluginApi.mount(app, wsApp)
     app.use("/api/applications", ensureAuthenticated, apps.router(pluginApi))
-    wrapper.onDispose(() => pluginApi.dispose())
   }
 
   app.use(express.json())
@@ -170,6 +166,8 @@ export const register = async (
   wsApp.use(wsErrorHandler)
 
   return () => {
+    heart.dispose()
+    pluginApi?.dispose()
     vscode?.codeServerMain.dispose()
   }
 }

test/unit/node/plugin.test.ts

@@ -58,7 +58,7 @@ describe("plugin", () => {
   })
 
   afterAll(async () => {
-    await s.close()
+    await s.dispose()
   })
 
   it("/api/applications", async () => {

test/unit/node/proxy.test.ts

@@ -24,7 +24,7 @@ describe("proxy", () => {
   })
 
   afterAll(async () => {
-    await nhooyrDevServer.close()
+    await nhooyrDevServer.dispose()
   })
 
   beforeEach(() => {
@@ -33,7 +33,7 @@ describe("proxy", () => {
 
   afterEach(async () => {
     if (codeServer) {
-      await codeServer.close()
+      await codeServer.dispose()
       codeServer = undefined
     }
   })

test/unit/node/routes/health.test.ts

@@ -6,7 +6,7 @@ describe("health", () => {
 
   afterEach(async () => {
     if (codeServer) {
-      await codeServer.close()
+      await codeServer.dispose()
       codeServer = undefined
     }
   })

test/unit/node/routes/login.test.ts

@@ -58,17 +58,11 @@ describe("login", () => {
     afterEach(async () => {
       process.env.PASSWORD = previousEnvPassword
       if (_codeServer) {
-        await _codeServer.close()
+        await _codeServer.dispose()
         _codeServer = undefined
       }
     })
 
-    afterAll(async () => {
-      if (_codeServer) {
-        _codeServer.dispose()
-      }
-    })
-
     it("should return HTML with 'Missing password' message", async () => {
       const resp = await codeServer().fetch("/login", { method: "POST" })
 

test/unit/node/routes/static.test.ts

@@ -33,17 +33,11 @@ describe("/_static", () => {
 
   afterEach(async () => {
     if (_codeServer) {
-      await _codeServer.close()
+      await _codeServer.dispose()
       _codeServer = undefined
     }
   })
 
-  afterAll(async () => {
-    if (_codeServer) {
-      _codeServer.dispose()
-    }
-  })
-
   function commonTests() {
     it("should return a 404 when a file is not provided", async () => {
       const resp = await codeServer().fetch(`/_static/`)

test/utils/httpserver.ts

@@ -60,9 +60,9 @@ export class HttpServer {
   }
 
   /**
-   * close cleans up the server.
+   * Clean up the server.
    */
-  public close(): Promise<void> {
+  public dispose(): Promise<void> {
     return new Promise((resolve, reject) => {
       // Close will not actually close anything; it just waits until everything
       // is closed.
@@ -74,6 +74,8 @@ export class HttpServer {
         resolve()
       })
 
+      this._dispose?.()
+
       // If there are sockets remaining we might need to force close them or
       // this promise might never resolve.
       if (this.sockets.size > 0) {
@@ -90,11 +92,6 @@ export class HttpServer {
     })
   }
 
-  public dispose() {
-    console.log("Disposing HTTP Server")
-    this._dispose?.()
-  }
-
   /**
    * fetch fetches the request path.
    * The request path must be rooted!