fix(Benchmark): improved error reporting and fix sysbench race condition

admin/app/controllers/benchmark_controller.ts

@@ -179,7 +179,9 @@ export default class BenchmarkController {
         percentile: submitResult.percentile,
       })
     } catch (error) {
-      return response.status(400).send({
+      // Pass through the status code from the service if available, otherwise default to 400
+      const statusCode = (error as any).statusCode || 400
+      return response.status(statusCode).send({
         success: false,
         error: error.message,
       })

admin/app/services/benchmark_service.ts

@@ -26,6 +26,7 @@ import { randomUUID, createHmac } from 'node:crypto'
 import { DockerService } from './docker_service.js'
 import { SERVICE_NAMES } from '../../constants/service_names.js'
 import { BROADCAST_CHANNELS } from '../../constants/broadcast.js'
+import Dockerode from 'dockerode'
 
 // HMAC secret for signing submissions to the benchmark repository
 // This provides basic protection against casual API abuse.
@@ -186,8 +187,14 @@ export class BenchmarkService {
 
       return response.data as RepositorySubmitResponse
     } catch (error) {
-      logger.error(`Failed to submit benchmark to repository: ${error.message}`)
-      throw new Error(`Failed to submit benchmark: ${error.message}`)
+      const detail = error.response?.data?.error || error.message || 'Unknown error'
+      const statusCode = error.response?.status
+      logger.error(`Failed to submit benchmark to repository: ${detail} (Status: ${statusCode})`)
+      
+      // Create an error with the status code attached for proper handling upstream
+      const err: any = new Error(`Failed to submit benchmark: ${detail}`)
+      err.statusCode = statusCode
+      throw err
     }
   }
 
@@ -707,23 +714,26 @@ export class BenchmarkService {
    * Run a sysbench command in a Docker container
    */
   private async _runSysbenchCommand(cmd: string[]): Promise<string> {
+    let container: Dockerode.Container | null = null
     try {
       // Create container with TTY to avoid multiplexed output
-      const container = await this.dockerService.docker.createContainer({
+      container = await this.dockerService.docker.createContainer({
         Image: SYSBENCH_IMAGE,
         Cmd: cmd,
         name: `${SYSBENCH_CONTAINER_NAME}_${Date.now()}`,
         Tty: true, // Important: prevents multiplexed stdout/stderr headers
         HostConfig: {
-          AutoRemove: true,
+          AutoRemove: false, // Don't auto-remove to avoid race condition with fetching logs
         },
       })
 
       // Start container
       await container.start()
 
-      // Wait for completion and get logs
+      // Wait for completion
       await container.wait()
+      
+      // Get logs after container has finished
       const logs = await container.logs({
         stdout: true,
         stderr: true,
@@ -734,8 +744,24 @@ export class BenchmarkService {
         .replace(/[\x00-\x08]/g, '') // Remove control characters
         .trim()
 
+      // Manually remove the container after getting logs
+      try {
+        await container.remove()
+      } catch (removeError) {
+        // Log but don't fail if removal fails (container might already be gone)
+        logger.warn(`Failed to remove sysbench container: ${removeError.message}`)
+      }
+
       return output
     } catch (error) {
+      // Clean up container on error if it exists
+      if (container) {
+        try {
+          await container.remove({ force: true })
+        } catch (removeError) {
+          // Ignore removal errors
+        }
+      }
       logger.error(`Sysbench command failed: ${error.message}`)
       throw new Error(`Sysbench command failed: ${error.message}`)
     }

admin/docs/release-notes.md

@@ -6,6 +6,8 @@
 - **Collections**: Complete overhaul of collection management with dynamic manifests, database tracking of installed resources, and improved UI for managing ZIM files and map assets
 - **Collections**: Added support for checking if newer versions of installed resources are available based on manifest data
 ### Bug Fixes
+- **Benchmark**: Improved error handling and status code propagation for better user feedback on submission failures
+- **Benchmark**: Fix a race condition in the sysbench container management that could lead to benchmark test failures
 
 ### Improvements
 

admin/inertia/lib/api.ts

@@ -448,10 +448,20 @@ class API {
   }
 
   async submitBenchmark(benchmark_id: string, anonymous: boolean) {
-    return catchInternal(async () => {
+    try {
       const response = await this.client.post<SubmitBenchmarkResponse>('/benchmark/submit', { benchmark_id, anonymous })
       return response.data
-    })()
+    } catch (error: any) {
+      // For 409 Conflict errors, throw a specific error that the UI can handle
+      if (error.response?.status === 409) {
+        const err = new Error(error.response?.data?.error || 'This benchmark has already been submitted to the repository')
+        ;(err as any).status = 409
+        throw err
+      }
+      // For other errors, extract the message and throw
+      const errorMessage = error.response?.data?.error || error.message || 'Failed to submit benchmark'
+      throw new Error(errorMessage)
+    }
   }
 
   async subscribeToReleaseNotes(email: string) {

admin/inertia/pages/settings/benchmark.tsx

@@ -173,8 +173,13 @@ export default function BenchmarkPage(props: {
       refetchLatest()
       queryClient.invalidateQueries({ queryKey: ['benchmark', 'history'] })
     },
-    onError: (error: Error) => {
-      setSubmitError(error.message)
+    onError: (error: any) => {
+      // Check if this is a 409 Conflict error (already submitted)
+      if (error.status === 409) {
+        setSubmitError('A benchmark for this system with the same or higher score has already been submitted.')
+      } else {
+        setSubmitError(error.message)
+      }
     },
   })