mirror of
https://github.com/Crosstalk-Solutions/project-nomad.git
synced 2026-03-28 11:39:26 +01:00
fix: standardize API error responses, fix parseInt radix, harden Docker service
- Create api_response.ts helper for consistent { success, error/data } format
- Add radix parameter (10) to all parseInt calls across controllers and services
- Fix race condition in DockerService by making in-memory guard atomic
- Fix container command splitting to handle quoted arguments properly
- Stop leaking internal error.message to API responses; log details server-side
https://claude.ai/code/session_01JFvpTYgm8GiE4vJ4cJKsFx
This commit is contained in:
Claude
parent
9ca20a99e5
commit
def1a0733f
|
|
@ -1,5 +1,6 @@
|
|||
import { inject } from '@adonisjs/core'
|
||||
import type { HttpContext } from '@adonisjs/core/http'
|
||||
import logger from '@adonisjs/core/services/logger'
|
||||
import { BenchmarkService } from '#services/benchmark_service'
|
||||
import { runBenchmarkValidator, submitBenchmarkValidator } from '#validators/benchmark'
|
||||
import { RunBenchmarkJob } from '#jobs/run_benchmark_job'
|
||||
|
|
@ -52,9 +53,11 @@ export default class BenchmarkController {
|
|||
result,
|
||||
})
|
||||
} catch (error) {
|
||||
const detail = error instanceof Error ? error.message : String(error)
|
||||
logger.error(`[BenchmarkController] Sync benchmark failed: ${detail}`)
|
||||
return response.status(500).send({
|
||||
success: false,
|
||||
error: error.message,
|
||||
error: 'Benchmark execution failed',
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -168,11 +171,12 @@ export default class BenchmarkController {
|
|||
percentile: submitResult.percentile,
|
||||
})
|
||||
} catch (error) {
|
||||
// Pass through the status code from the service if available, otherwise default to 400
|
||||
const detail = error instanceof Error ? error.message : String(error)
|
||||
logger.error(`[BenchmarkController] Submit failed: ${detail}`)
|
||||
const statusCode = (error as any).statusCode || 400
|
||||
return response.status(statusCode).send({
|
||||
success: false,
|
||||
error: error.message,
|
||||
error: 'Failed to submit benchmark results',
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { createSessionSchema, updateSessionSchema, addMessageSchema } from '#val
|
|||
import KVStore from '#models/kv_store'
|
||||
import { SystemService } from '#services/system_service'
|
||||
import { SERVICE_NAMES } from '../../constants/service_names.js'
|
||||
import { apiSuccess, apiError } from '../helpers/api_response.js'
|
||||
|
||||
@inject()
|
||||
export default class ChatsController {
|
||||
|
|
@ -13,9 +14,9 @@ export default class ChatsController {
|
|||
async inertia({ inertia, response }: HttpContext) {
|
||||
const aiAssistantInstalled = await this.systemService.checkServiceInstalled(SERVICE_NAMES.OLLAMA)
|
||||
if (!aiAssistantInstalled) {
|
||||
return response.status(404).json({ error: 'AI Assistant service not installed' })
|
||||
return response.status(404).json({ success: false, error: 'AI Assistant service not installed' })
|
||||
}
|
||||
|
||||
|
||||
const chatSuggestionsEnabled = await KVStore.getValue('chat.suggestionsEnabled')
|
||||
return inertia.render('chat', {
|
||||
settings: {
|
||||
|
|
@ -29,11 +30,11 @@ export default class ChatsController {
|
|||
}
|
||||
|
||||
async show({ params, response }: HttpContext) {
|
||||
const sessionId = parseInt(params.id)
|
||||
const sessionId = parseInt(params.id, 10)
|
||||
const session = await this.chatService.getSession(sessionId)
|
||||
|
||||
if (!session) {
|
||||
return response.status(404).json({ error: 'Session not found' })
|
||||
return response.status(404).json({ success: false, error: 'Session not found' })
|
||||
}
|
||||
|
||||
return session
|
||||
|
|
@ -45,58 +46,48 @@ export default class ChatsController {
|
|||
const session = await this.chatService.createSession(data.title, data.model)
|
||||
return response.status(201).json(session)
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to create session',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to create session', error)
|
||||
}
|
||||
}
|
||||
|
||||
async suggestions({ response }: HttpContext) {
|
||||
try {
|
||||
const suggestions = await this.chatService.getChatSuggestions()
|
||||
return response.status(200).json({ suggestions })
|
||||
return response.status(200).json({ success: true, suggestions })
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to get suggestions',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to get suggestions', error)
|
||||
}
|
||||
}
|
||||
|
||||
async update({ params, request, response }: HttpContext) {
|
||||
try {
|
||||
const sessionId = parseInt(params.id)
|
||||
const sessionId = parseInt(params.id, 10)
|
||||
const data = await request.validateUsing(updateSessionSchema)
|
||||
const session = await this.chatService.updateSession(sessionId, data)
|
||||
return session
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to update session',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to update session', error)
|
||||
}
|
||||
}
|
||||
|
||||
async destroy({ params, response }: HttpContext) {
|
||||
try {
|
||||
const sessionId = parseInt(params.id)
|
||||
const sessionId = parseInt(params.id, 10)
|
||||
await this.chatService.deleteSession(sessionId)
|
||||
return response.status(204)
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to delete session',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to delete session', error)
|
||||
}
|
||||
}
|
||||
|
||||
async addMessage({ params, request, response }: HttpContext) {
|
||||
try {
|
||||
const sessionId = parseInt(params.id)
|
||||
const sessionId = parseInt(params.id, 10)
|
||||
const data = await request.validateUsing(addMessageSchema)
|
||||
const message = await this.chatService.addMessage(sessionId, data.role, data.content)
|
||||
return response.status(201).json(message)
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to add message',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to add message', error)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -105,9 +96,7 @@ export default class ChatsController {
|
|||
const result = await this.chatService.deleteAllSessions()
|
||||
return response.status(200).json(result)
|
||||
} catch (error) {
|
||||
return response.status(500).json({
|
||||
error: error instanceof Error ? error.message : 'Failed to delete all sessions',
|
||||
})
|
||||
return apiError(response, 500, 'Failed to delete all sessions', error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import app from '@adonisjs/core/services/app'
|
|||
import { randomBytes } from 'node:crypto'
|
||||
import { sanitizeFilename } from '../utils/fs.js'
|
||||
import { deleteFileSchema, getJobStatusSchema } from '#validators/rag'
|
||||
import { apiError } from '../helpers/api_response.js'
|
||||
|
||||
@inject()
|
||||
export default class RagController {
|
||||
|
|
@ -14,7 +15,7 @@ export default class RagController {
|
|||
public async upload({ request, response }: HttpContext) {
|
||||
const uploadedFile = request.file('file', { size: '50mb' })
|
||||
if (!uploadedFile) {
|
||||
return response.status(400).json({ error: 'No file uploaded' })
|
||||
return response.status(400).json({ success: false, error: 'No file uploaded' })
|
||||
}
|
||||
|
||||
if (!uploadedFile.isValid) {
|
||||
|
|
@ -38,6 +39,7 @@ export default class RagController {
|
|||
})
|
||||
|
||||
return response.status(202).json({
|
||||
success: true,
|
||||
message: result.message,
|
||||
jobId: result.jobId,
|
||||
fileName,
|
||||
|
|
@ -58,7 +60,7 @@ export default class RagController {
|
|||
const status = await EmbedFileJob.getStatus(fullPath)
|
||||
|
||||
if (!status.exists) {
|
||||
return response.status(404).json({ error: 'Job not found for this file' })
|
||||
return response.status(404).json({ success: false, error: 'Job not found for this file' })
|
||||
}
|
||||
|
||||
return response.status(200).json(status)
|
||||
|
|
@ -66,16 +68,16 @@ export default class RagController {
|
|||
|
||||
public async getStoredFiles({ response }: HttpContext) {
|
||||
const files = await this.ragService.getStoredFiles()
|
||||
return response.status(200).json({ files })
|
||||
return response.status(200).json({ success: true, files })
|
||||
}
|
||||
|
||||
public async deleteFile({ request, response }: HttpContext) {
|
||||
const { source } = await request.validateUsing(deleteFileSchema)
|
||||
const result = await this.ragService.deleteFileBySource(source)
|
||||
if (!result.success) {
|
||||
return response.status(500).json({ error: result.message })
|
||||
return response.status(500).json({ success: false, error: result.message })
|
||||
}
|
||||
return response.status(200).json({ message: result.message })
|
||||
return response.status(200).json({ success: true, message: result.message })
|
||||
}
|
||||
|
||||
public async scanAndSync({ response }: HttpContext) {
|
||||
|
|
@ -83,7 +85,7 @@ export default class RagController {
|
|||
const syncResult = await this.ragService.scanAndSyncStorage()
|
||||
return response.status(200).json(syncResult)
|
||||
} catch (error) {
|
||||
return response.status(500).json({ error: 'Error scanning and syncing storage', details: error.message })
|
||||
return apiError(response, 500, 'Error scanning and syncing storage', error)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { CheckServiceUpdatesJob } from '#jobs/check_service_updates_job'
|
|||
import { affectServiceValidator, checkLatestVersionValidator, installServiceValidator, subscribeToReleaseNotesValidator, updateServiceValidator } from '#validators/system';
|
||||
import { inject } from '@adonisjs/core'
|
||||
import type { HttpContext } from '@adonisjs/core/http'
|
||||
import { apiError } from '../helpers/api_response.js'
|
||||
|
||||
@inject()
|
||||
export default class SystemController {
|
||||
|
|
@ -35,7 +36,7 @@ export default class SystemController {
|
|||
if (result.success) {
|
||||
response.send({ success: true, message: result.message });
|
||||
} else {
|
||||
response.status(400).send({ error: result.message });
|
||||
response.status(400).send({ success: false, error: result.message });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -43,7 +44,7 @@ export default class SystemController {
|
|||
const payload = await request.validateUsing(affectServiceValidator);
|
||||
const result = await this.dockerService.affectContainer(payload.service_name, payload.action);
|
||||
if (!result) {
|
||||
response.internalServerError({ error: 'Failed to affect service' });
|
||||
response.internalServerError({ success: false, error: 'Failed to affect service' });
|
||||
return;
|
||||
}
|
||||
response.send({ success: result.success, message: result.message });
|
||||
|
|
@ -58,7 +59,7 @@ export default class SystemController {
|
|||
const payload = await request.validateUsing(installServiceValidator);
|
||||
const result = await this.dockerService.forceReinstall(payload.service_name);
|
||||
if (!result) {
|
||||
response.internalServerError({ error: 'Failed to force reinstall service' });
|
||||
response.internalServerError({ success: false, error: 'Failed to force reinstall service' });
|
||||
return;
|
||||
}
|
||||
response.send({ success: result.success, message: result.message });
|
||||
|
|
@ -94,6 +95,7 @@ export default class SystemController {
|
|||
|
||||
if (!status) {
|
||||
response.status(500).send({
|
||||
success: false,
|
||||
error: 'Failed to retrieve update status',
|
||||
});
|
||||
return;
|
||||
|
|
@ -132,7 +134,7 @@ export default class SystemController {
|
|||
.first()
|
||||
|
||||
if (!service) {
|
||||
return response.status(404).send({ error: `Service ${serviceName} not found or not installed` })
|
||||
return response.status(404).send({ success: false, error: `Service ${serviceName} not found or not installed` })
|
||||
}
|
||||
|
||||
try {
|
||||
|
|
@ -142,9 +144,9 @@ export default class SystemController {
|
|||
hostArch,
|
||||
service.source_repo
|
||||
)
|
||||
response.send({ versions: updates })
|
||||
response.send({ success: true, versions: updates })
|
||||
} catch (error) {
|
||||
response.status(500).send({ error: `Failed to fetch versions: ${error.message}` })
|
||||
return apiError(response, 500, 'Failed to fetch available versions', error)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -158,7 +160,7 @@ export default class SystemController {
|
|||
if (result.success) {
|
||||
response.send({ success: true, message: result.message })
|
||||
} else {
|
||||
response.status(400).send({ error: result.message })
|
||||
response.status(400).send({ success: false, error: result.message })
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
31
admin/app/helpers/api_response.ts
Normal file
31
admin/app/helpers/api_response.ts
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
import type { HttpContext } from '@adonisjs/core/http'
|
||||
import logger from '@adonisjs/core/services/logger'
|
||||
|
||||
/**
|
||||
* Standardized API response helpers.
|
||||
*
|
||||
* Success responses follow the shape `{ success: true, data?: ..., message?: ... }`.
|
||||
* Error responses follow the shape `{ success: false, error: '...' }`.
|
||||
*
|
||||
* Internal error details are logged server-side and never leaked to the client.
|
||||
*/
|
||||
|
||||
export function apiSuccess(data?: Record<string, unknown> | unknown[] | string) {
|
||||
if (typeof data === 'string') {
|
||||
return { success: true, message: data }
|
||||
}
|
||||
return { success: true, ...(data && typeof data === 'object' ? (Array.isArray(data) ? { data } : data) : {}) }
|
||||
}
|
||||
|
||||
export function apiError(
|
||||
response: HttpContext['response'],
|
||||
status: number,
|
||||
clientMessage: string,
|
||||
internalError?: unknown
|
||||
) {
|
||||
if (internalError) {
|
||||
const detail = internalError instanceof Error ? internalError.message : String(internalError)
|
||||
logger.error(`[API] ${clientMessage}: ${detail}`)
|
||||
}
|
||||
return response.status(status).send({ success: false, error: clientMessage })
|
||||
}
|
||||
|
|
@ -623,7 +623,7 @@ export class BenchmarkService {
|
|||
return {
|
||||
events_per_second: eventsMatch ? parseFloat(eventsMatch[1]) : 0,
|
||||
total_time: totalTimeMatch ? parseFloat(totalTimeMatch[1]) : 30,
|
||||
total_events: totalEventsMatch ? parseInt(totalEventsMatch[1]) : 0,
|
||||
total_events: totalEventsMatch ? parseInt(totalEventsMatch[1], 10) : 0,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -194,16 +194,19 @@ export class DockerService {
|
|||
}
|
||||
}
|
||||
|
||||
// Check if installation is already in progress (database-level)
|
||||
if (service.installation_status === 'installing') {
|
||||
// Atomic in-memory guard: check and set in a single operation to prevent
|
||||
// race conditions where two requests pass the check before either sets the flag.
|
||||
if (this.activeInstallations.has(serviceName)) {
|
||||
return {
|
||||
success: false,
|
||||
message: `Service ${serviceName} installation is already in progress`,
|
||||
}
|
||||
}
|
||||
this.activeInstallations.add(serviceName)
|
||||
|
||||
// Double-check with in-memory tracking (race condition protection)
|
||||
if (this.activeInstallations.has(serviceName)) {
|
||||
// Now check database-level status (safe because the in-memory flag blocks concurrent callers)
|
||||
if (service.installation_status === 'installing') {
|
||||
this.activeInstallations.delete(serviceName)
|
||||
return {
|
||||
success: false,
|
||||
message: `Service ${serviceName} installation is already in progress`,
|
||||
|
|
@ -211,7 +214,6 @@ export class DockerService {
|
|||
}
|
||||
|
||||
// Mark installation as in progress
|
||||
this.activeInstallations.add(serviceName)
|
||||
service.installation_status = 'installing'
|
||||
await service.save()
|
||||
|
||||
|
|
@ -513,7 +515,7 @@ export class DockerService {
|
|||
...(containerConfig?.WorkingDir && { WorkingDir: containerConfig.WorkingDir }),
|
||||
...(containerConfig?.ExposedPorts && { ExposedPorts: containerConfig.ExposedPorts }),
|
||||
...(containerConfig?.Env && { Env: containerConfig.Env }),
|
||||
...(service.container_command ? { Cmd: service.container_command.split(' ') } : {}),
|
||||
...(service.container_command ? { Cmd: service.container_command.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g)?.map((arg: string) => arg.replace(/^["']|["']$/g, '')) ?? [] } : {}),
|
||||
// Ensure container is attached to the Nomad docker network in production
|
||||
...(process.env.NODE_ENV === 'production' && {
|
||||
NetworkingConfig: {
|
||||
|
|
|
|||
|
|
@ -223,7 +223,7 @@ export class ZIMExtractionService {
|
|||
});
|
||||
}
|
||||
// Start new section
|
||||
const level = parseInt(tagName.substring(1)); // Extract number from h2, h3, h4
|
||||
const level = parseInt(tagName.substring(1), 10); // Extract number from h2, h3, h4
|
||||
currentSection = {
|
||||
heading: $el.text().replace(/\[edit\]/gi, '').trim(),
|
||||
content: [],
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ export async function doResumableDownload({
|
|||
})
|
||||
|
||||
const contentType = headResponse.headers['content-type'] || ''
|
||||
const totalBytes = parseInt(headResponse.headers['content-length'] || '0')
|
||||
const totalBytes = parseInt(headResponse.headers['content-length'] || '0', 10)
|
||||
const supportsRangeRequests = headResponse.headers['accept-ranges'] === 'bytes'
|
||||
|
||||
// If allowedMimeTypes is provided, check content type
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user