major code quality improvements

Author: tshmieldev

bun.lock

@@ -3,4 +3,4 @@
 coveragePathIgnorePatterns = [
   "src/lib/docker.ts",
   "src/lib/files.ts"
-]
+]

bunfig.toml

@@ -14,6 +14,9 @@
         "zod": "^4.0.14"
     },
     "devDependencies": {
-        "@types/bun": "^1.3.1"
+        "@types/bun": "^1.3.1",
+        "@typescript-eslint/eslint-plugin": "^8.46.2",
+        "@typescript-eslint/parser": "^8.46.2",
+        "eslint": "^9.38.0"
     }
 }

package.json

@@ -0,0 +1,19 @@
+[tool.pyright]
+# Global settings
+reportIncompatibleMethodOverride = false
+
+# Main configuration for src/lib/python
+include = ["src/lib/python/**/*.py"]
+reportOptionalMemberAccess = false
+reportMissingImports = false
+reportAny = false
+
+[[tool.pyright.executionEnvironments]]
+# Here, pretty much all rules must be disabled
+# This code will be executed in a container
+# And so, the linter does not know the context, and is unhappy about it.
+root = "tests/data"
+reportIncompatibleMethodOverride = false
+reportMissingImports = false
+reportAny = false
+reportMissingTypeArgument = false

pyproject.toml

@@ -7,6 +7,9 @@ import { printMetrics } from "../lib/prometheus";
 
 const app = new OpenAPIHono();
 
+// Path is overriden, as I prefer to have the
+// full path in lib/openapi.ts to keep the
+// semantics there and in index.ts
 app.openapi(
     { ...metricsRoute, path: "/" },
     async (c): Promise<RouteConfigToTypedResponse<typeof metricsRoute>> => {
@@ -22,8 +25,9 @@ app.openapi(
                 401,
             );
         }
-        // @ts-ignore This will return 200
-        return printMetrics(c);
+        return printMetrics(c) as unknown as RouteConfigToTypedResponse<
+            typeof metricsRoute
+        >;
     },
 );
 

src/controllers/metrics.ts

@@ -1,14 +1,20 @@
-import { OpenAPIHono, type RouteConfigToTypedResponse } from "@hono/zod-openapi";
+import {
+    OpenAPIHono,
+    type RouteConfigToTypedResponse,
+} from "@hono/zod-openapi";
 import { statusRoute } from "../lib/openapi";
 import { getActiveContainers } from "../lib/docker";
 
 const app = new OpenAPIHono();
 
+// Path is overriden, as I prefer to have the
+// full path in lib/openapi.ts to keep the
+// semantics there and in index.ts
 app.openapi(
     { ...statusRoute, path: "/" },
     async (c): Promise<RouteConfigToTypedResponse<typeof statusRoute>> => {
         return c.json({ runningContainers: getActiveContainers().size }, 200);
     },
 );
 
-export default app;
+export default app;

src/controllers/status.ts

@@ -1,98 +1,114 @@
 import {
-  OpenAPIHono,
-  type RouteConfigToTypedResponse,
+    OpenAPIHono,
+    type RouteConfigToTypedResponse,
 } from "@hono/zod-openapi";
 import { runUnitTestRequestSchema } from "../lib/validation";
 import { v4 } from "uuid";
 import {
-  cleanupSessionDirectory,
-  createSessionDirectory,
-  prepareUnitTestFiles,
+    cleanupSessionDirectory,
+    createSessionDirectory,
+    prepareUnitTestFiles,
 } from "../lib/files";
 import {
-  buildDockerCommand,
-  getContainerName,
-  trackContainer,
-  untrackContainer,
+    buildDockerCommand,
+    getContainerName,
+    trackContainer,
+    untrackContainer,
 } from "../lib/docker";
 import { unitTestRoute } from "../lib/openapi";
 
 const app = new OpenAPIHono();
 
+// Path is overriden, as I prefer to have the
+// full path in lib/openapi.ts to keep the
+// semantics there and in index.ts
 app.openapi(
-  { ...unitTestRoute, path: "/" },
-  async (c): Promise<RouteConfigToTypedResponse<typeof unitTestRoute>> => {
-    let body = null;
+    { ...unitTestRoute, path: "/" },
+    async (c): Promise<RouteConfigToTypedResponse<typeof unitTestRoute>> => {
+        let body = null;
 
-    try {
-      body = await c.req.json();
-    } catch {
-      return c.json({ success: false, error: "Invalid JSON" }, 400);
-    }
+        try {
+            body = await c.req.json();
+        } catch {
+            return c.json({ success: false, error: "Invalid JSON" }, 400);
+        }
 
-    const validatedData = runUnitTestRequestSchema.safeParse(body);
+        const validatedData = runUnitTestRequestSchema.safeParse(body);
 
-    if (!validatedData.success) {
-      return c.json(
-        { success: false, error: validatedData.error.message },
-        400,
-      );
-    }
+        if (!validatedData.success) {
+            return c.json(
+                { success: false, error: validatedData.error.message },
+                400,
+            );
+        }
 
-    const { user_code, unit_tests, api_key, config } = validatedData.data;
+        const { user_code, unit_tests, api_key, config } = validatedData.data;
 
-    if (api_key !== Bun.env.API_KEY) {
-      return c.json({ success: false, error: "Invalid API key" }, 401);
-    }
+        if (api_key !== Bun.env.API_KEY) {
+            return c.json({ success: false, error: "Invalid API key" }, 401);
+        }
 
-    // Create unique session
-    const sessionId = v4();
-    const sessionPath = await createSessionDirectory(sessionId);
-    const containerName = getContainerName(sessionId);
+        // Create unique session
+        const sessionId = v4();
+        const sessionPath = await createSessionDirectory(sessionId);
+        const containerName = getContainerName(sessionId);
 
-    try {
-      // Setup files and container
-      await prepareUnitTestFiles(sessionPath, user_code, unit_tests);
-      trackContainer(containerName);
+        try {
+            // Setup files and container
+            await prepareUnitTestFiles(sessionPath, user_code, unit_tests);
+            trackContainer(containerName);
 
-      // Execute in Docker
-      const command = buildDockerCommand(
-        sessionId,
-        sessionPath,
-        "unit-test",
-        config,
-      );
-      const proc = Bun.spawn(command, {
-        stdout: "pipe",
-        stderr: "pipe",
-      });
+            // Execute in Docker
+            const command = buildDockerCommand(sessionId, sessionPath, config);
+            const proc = Bun.spawn(command, {
+                stdout: "pipe",
+                stderr: "pipe",
+            });
 
-      const stdout = await new Response(proc.stdout).text();
-      const stderr = await new Response(proc.stderr).text();
+            const stdout = await new Response(proc.stdout).text();
+            const stderr = await new Response(proc.stderr).text();
 
-      // Wait for process to complete and get exit code
-      const exitCode = await proc.exited;
+            // Wait for process to complete and get exit code
+            const exitCode = await proc.exited;
 
-      const result = stdout ? JSON.parse(stdout) : undefined;
+            const result = stdout ? JSON.parse(stdout) : undefined;
 
-      return c.json(
-        {
-          success: exitCode == 0,
-          runalyzer_output: result,
-          runalyzer_errors:
-            stderr || (exitCode === 124 ? "Time limit exceeded" : ""),
-          exit_code: exitCode,
-        },
-        200,
-      );
-    } catch (error) {
-      console.error(error);
-      return c.json({ success: false, error: "Internal server error" }, 500);
-    } finally {
-      untrackContainer(containerName);
-      await cleanupSessionDirectory(sessionPath, sessionId);
-    }
-  },
+            // Yes, this could be one return
+            // But typescript wants it to be split
+            // Because of the discriminatory union
+            if (exitCode === 0) {
+                return c.json(
+                    {
+                        success: true,
+                        runalyzer_output: result,
+                        runalyzer_errors: stderr,
+                        exit_code: exitCode,
+                    },
+                    200,
+                );
+            }
+            return c.json(
+                {
+                    success: false,
+                    runalyzer_output: result,
+                    runalyzer_errors:
+                        stderr ||
+                        (exitCode === 124 ? "Time limit exceeded" : ""),
+                    exit_code: exitCode,
+                },
+                200,
+            );
+        } catch (error) {
+            console.error(error);
+            return c.json(
+                { success: false, error: "Internal server error" },
+                500,
+            );
+        } finally {
+            untrackContainer(containerName);
+            await cleanupSessionDirectory(sessionPath, sessionId);
+        }
+    },
 );
 
 export default app;

src/controllers/unit-tests.ts

@@ -8,7 +8,6 @@ let activeContainers = new Set<string>();
 export function buildDockerCommand(
     sessionId: string,
     sessionPath: string,
-    type: "unit-test",
     config?: Config,
 ): string[] {
     const containerName = `code-runner-${sessionId}`;
@@ -56,17 +55,21 @@ export function untrackContainer(containerName: string): void {
 
 export async function cleanupContainer(containerName: string): Promise<void> {
     try {
-        Bun.spawn(["docker", "kill", containerName], {
+        const killProc = Bun.spawn(["docker", "kill", containerName], {
             stdout: "ignore",
             stderr: "ignore",
         });
+        // Wait for the kill command to complete before attempting to remove the container.
+        // This prevents a race condition where `docker rm` is called before the container has stopped.
+        await killProc.exited;
         Bun.spawn(["docker", "rm", "-f", containerName], {
             stdout: "ignore",
             stderr: "ignore",
         });
         activeContainers.delete(containerName);
     } catch (err) {
-        // Ignore cleanup errors
+        // This is non-crucial as containers have the --rm flag
+        console.error(`Error cleaning up container ${containerName}:`, err);
     }
 }
 

src/lib/docker.ts

@@ -88,8 +88,8 @@ export const metricsRoute = createRoute({
     responses: {
         200: {
             content: {
-                "application/json": {
-                    schema: z.any(), // Managed by prometheus
+                "text/plain": {
+                    schema: z.string(),
                 },
             },
             description: "Server metrics as returned by printMetrics().",

src/lib/openapi.ts

@@ -4,6 +4,7 @@
 import sys
 import io
 import time
+
 from unittests import test
 
 try:
@@ -12,7 +13,7 @@
     sys.__stdout__.write(
         json.dumps(
             {
-                "runalyzer_errors": "Nie znaleziono funkcji solution.",
+                "runalyzer_errors": "Solution function not found.",
                 "test_result": None,
                 "stdout": None,
                 "stderr": None,
@@ -22,6 +23,8 @@
     )
     exit(1)
 
+MAX_LENGTH = 1024
+
 
 def truncate_output(output, max_chars=256):
     """Truncate output to max_chars and add ... if needed"""
@@ -52,8 +55,8 @@ def truncate_output(output, max_chars=256):
         exit(1)
 
     # Get captured output and truncate
-    stdout_data = truncate_output(stdout_capture.getvalue(), 1024)
-    stderr_data = truncate_output(stderr_capture.getvalue(), 1024)
+    stdout_data = truncate_output(stdout_capture.getvalue(), MAX_LENGTH)
+    stderr_data = truncate_output(stderr_capture.getvalue(), MAX_LENGTH)
 
     # Redirect further output to /dev/null
     with open(os.devnull, "w") as devnull:
@@ -66,8 +69,8 @@ def truncate_output(output, max_chars=256):
                         "stdout": stdout_data,
                         "stderr": stderr_data,
                         "duration": duration,
-                        "truncated": len(stdout_capture.getvalue()) > 1024
-                        or len(stderr_capture.getvalue()) > 1024,
+                        "truncated": len(stdout_capture.getvalue()) > MAX_LENGTH
+                        or len(stderr_capture.getvalue()) > MAX_LENGTH,
                     }
                 )
             )