fix tracking server leak

Author: allansson

src/main/script.ts

@@ -100,10 +100,9 @@ export const runScript = async ({
       K6_BROWSER_ARGS: proxyArgs.join(','),
       K6_TESTING_COLORIZE: 'false',
     },
+    disposables: [trackingServer],
   })
 
-  testRun.addDisposable(trackingServer)
-
   testRun.on('log', ({ entry }) => {
     browserWindow.webContents.send(ScriptHandler.Log, entry)
   })

src/utils/k6/client.ts

@@ -49,6 +49,7 @@ interface RunArgs {
   insecureSkipTLSVerify?: boolean
   noUsageReport?: boolean
   env?: Record<string, string>
+  disposables?: Array<AsyncDisposable | Disposable>
 }
 
 export class ArchiveError extends Error {
@@ -119,6 +120,7 @@ export class K6Client {
     insecureSkipTLSVerify,
     noUsageReport,
     env = {},
+    disposables = [],
   }: RunArgs): TestRun {
     const args = [
       ['--log-format', 'json'],
@@ -133,7 +135,7 @@ export class K6Client {
       env,
     })
 
-    return new TestRun(process)
+    return new TestRun(process, disposables)
   }
 
   #wait(k6: ChildProcessWithoutNullStreams): Promise<SpawnResult> {

src/utils/k6/testRun.ts

@@ -115,7 +115,7 @@ interface TestRunEventMap {
    * Called when the test run has stopped, i.e. after the
    * done, abort or error events have been emitted.
    */
-  stop: void
+  stop: EmptyObject
 
   /**
    * Fired when a log entry is emitted by the test run.
@@ -125,13 +125,20 @@ interface TestRunEventMap {
 
 export class TestRun extends EventEmitter<TestRunEventMap> {
   #process: ChildProcessWithoutNullStreams
+  #disposer = new AsyncDisposableStack()
 
   #checks: Check[] = []
-  #disposables: Array<AsyncDisposable | Disposable> = []
 
-  constructor(process: ChildProcessWithoutNullStreams) {
+  constructor(
+    process: ChildProcessWithoutNullStreams,
+    disposables: Array<AsyncDisposable | Disposable> = []
+  ) {
     super()
 
+    for (const disposable of disposables) {
+      this.#disposer.use(disposable)
+    }
+
     process.on('spawn', this.#handleStart)
 
     process.on('error', this.#handleError)
@@ -168,26 +175,22 @@ export class TestRun extends EventEmitter<TestRunEventMap> {
 
     this.#process = process
 
-    this.on('done', this.#emitStop)
-    this.on('abort', this.#emitStop)
-    this.on('error', this.#emitStop)
+    this.on('done', this.#handleStop)
+    this.on('abort', this.#handleStop)
+    this.on('error', this.#handleStop)
   }
 
   isRunning(): boolean {
     return this.#process.pid != undefined && this.#process.exitCode === null
   }
 
   async stop(): Promise<void> {
-    await Promise.all([
-      this.#kill(),
-      ...this.#disposables.map((disposable) => {
-        if (Symbol.asyncDispose in disposable) {
-          return disposable[Symbol.asyncDispose]()
-        }
-
-        return disposable[Symbol.dispose]()
-      }),
-    ])
+    // If we encounter an error while disposing we assume the resources have already
+    // been disposed and ignore the error. `disposeAsync` is idempotent so calling it
+    // multiple times is safe.
+    await Promise.allSettled([this.#kill(), this.#disposer.disposeAsync()])
+
+    this.emit('stop', {})
   }
 
   #kill() {
@@ -252,11 +255,13 @@ export class TestRun extends EventEmitter<TestRunEventMap> {
     }
   }
 
-  #emitStop = () => {
-    this.emit('stop', undefined)
-  }
-
-  addDisposable(disposable: AsyncDisposable | Disposable) {
-    this.#disposables.push(disposable)
+  #handleStop = async () => {
+    try {
+      await this.#disposer.disposeAsync()
+    } catch (error) {
+      console.error('Error disposing children of test run', error)
+    } finally {
+      this.emit('stop', {})
+    }
   }
 }