From f949c7a8f982b90d9c33edd8b28a16b2adccf0c9 Mon Sep 17 00:00:00 2001 From: digua Date: Thu, 16 Jul 2026 13:58:29 +0800 Subject: [PATCH] refactor(http): unify API error model --- apps/cli/src/http/server.ts | 4 +- apps/desktop/main/api/auth.ts | 2 +- apps/desktop/main/api/errors.test.ts | 31 ---- apps/desktop/main/api/errors.ts | 146 ------------------ .../main/api/routes/import-helpers.test.ts | 3 +- .../desktop/main/api/routes/import-helpers.ts | 12 +- apps/desktop/main/api/routes/import.ts | 20 ++- apps/desktop/main/api/routes/sessions.ts | 9 +- apps/desktop/main/api/routes/system.ts | 2 +- apps/desktop/main/api/server.ts | 8 +- apps/desktop/main/internal-api/http.ts | 8 +- packages/http-routes/package.json | 5 + packages/http-routes/src/errors.test.ts | 31 +++- packages/http-routes/src/errors.ts | 26 +++- packages/http-routes/src/index.ts | 5 + 15 files changed, 111 insertions(+), 201 deletions(-) delete mode 100644 apps/desktop/main/api/errors.test.ts delete mode 100644 apps/desktop/main/api/errors.ts diff --git a/apps/cli/src/http/server.ts b/apps/cli/src/http/server.ts index bd5c61e7..b8defdae 100644 --- a/apps/cli/src/http/server.ts +++ b/apps/cli/src/http/server.ts @@ -5,14 +5,14 @@ */ import Fastify, { type FastifyInstance, type FastifyError } from 'fastify' +import { authHook } from '@openchatlab/http-routes/auth' import { - authHook, ApiError, ApiErrorCode, apiErrorFromUnknown, errorResponse, serverError, -} from '@openchatlab/http-routes' +} from '@openchatlab/http-routes/errors' import { appLogger } from '@openchatlab/node-runtime' const JSON_BODY_LIMIT = 50 * 1024 * 1024 // 50MB diff --git a/apps/desktop/main/api/auth.ts b/apps/desktop/main/api/auth.ts index a7ba7f95..937d1066 100644 --- a/apps/desktop/main/api/auth.ts +++ b/apps/desktop/main/api/auth.ts @@ -4,8 +4,8 @@ import type { FastifyRequest, FastifyReply } from 'fastify' import { timingSafeEqual } from 'crypto' +import { unauthorized, errorResponse } from '@openchatlab/http-routes/errors' import { getConfig } from './index' -import { unauthorized, errorResponse } from './errors' function safeTokenCompare(a: string, b: string): boolean { const bufA = Buffer.from(a) diff --git a/apps/desktop/main/api/errors.test.ts b/apps/desktop/main/api/errors.test.ts deleted file mode 100644 index e64e9fa4..00000000 --- a/apps/desktop/main/api/errors.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import assert from 'node:assert/strict' -import test from 'node:test' -import { DataDirCompatibilityError } from '@openchatlab/node-runtime/src/data-dir-compat' -import { ApiErrorCode, apiErrorFromUnknown } from './errors' -import { ImportInProgressError } from '@openchatlab/node-runtime/src/import/import-lock' - -test('apiErrorFromUnknown maps the shared import lock error to 409', () => { - const apiError = apiErrorFromUnknown(new ImportInProgressError()) - - assert.equal(apiError?.code, ApiErrorCode.IMPORT_IN_PROGRESS) - assert.equal(apiError?.statusCode, 409) -}) - -test('apiErrorFromUnknown maps wrapped data directory compatibility errors to 409', () => { - const cause = new DataDirCompatibilityError( - 'DATA_DIR_REQUIRES_NEWER_RUNTIME', - 'ChatLab data directory requires runtime version 0.25.1 or newer; current version is 0.25.0.', - { - userDataDir: '/tmp/chatlab-data', - metaPath: '/tmp/chatlab-data/.chatlab-meta.json', - currentVersion: '0.25.0', - minRuntimeVersion: '0.25.1', - } - ) - - const apiError = apiErrorFromUnknown(new Error('Desktop startup formatted message', { cause })) - - assert.equal(apiError?.code, ApiErrorCode.DATA_DIR_INCOMPATIBLE) - assert.equal(apiError?.statusCode, 409) - assert.match(apiError?.message ?? '', /requires runtime version 0\.25\.1/) -}) diff --git a/apps/desktop/main/api/errors.ts b/apps/desktop/main/api/errors.ts deleted file mode 100644 index 26e0eb18..00000000 --- a/apps/desktop/main/api/errors.ts +++ /dev/null @@ -1,146 +0,0 @@ -/** - * ChatLab API — Error codes and factory functions - */ - -import { DataDirCompatibilityError } from '@openchatlab/node-runtime/src/data-dir-compat' -import { ImportInProgressError } from '@openchatlab/node-runtime/src/import/import-lock' - -export enum ApiErrorCode { - UNAUTHORIZED = 'UNAUTHORIZED', - SESSION_NOT_FOUND = 'SESSION_NOT_FOUND', - INVALID_FORMAT = 'INVALID_FORMAT', - INVALID_PAYLOAD = 'INVALID_PAYLOAD', - SQL_READONLY_VIOLATION = 'SQL_READONLY_VIOLATION', - SQL_EXECUTION_ERROR = 'SQL_EXECUTION_ERROR', - EXPORT_TOO_LARGE = 'EXPORT_TOO_LARGE', - BODY_TOO_LARGE = 'BODY_TOO_LARGE', - IMPORT_IN_PROGRESS = 'IMPORT_IN_PROGRESS', - IDEMPOTENCY_CONFLICT = 'IDEMPOTENCY_CONFLICT', - IDEMPOTENCY_PENDING = 'IDEMPOTENCY_PENDING', - IMPORT_FAILED = 'IMPORT_FAILED', - DATA_DIR_INCOMPATIBLE = 'DATA_DIR_INCOMPATIBLE', - SERVER_ERROR = 'SERVER_ERROR', -} - -const HTTP_STATUS: Record = { - [ApiErrorCode.UNAUTHORIZED]: 401, - [ApiErrorCode.SESSION_NOT_FOUND]: 404, - [ApiErrorCode.INVALID_FORMAT]: 400, - [ApiErrorCode.INVALID_PAYLOAD]: 400, - [ApiErrorCode.SQL_READONLY_VIOLATION]: 400, - [ApiErrorCode.SQL_EXECUTION_ERROR]: 400, - [ApiErrorCode.EXPORT_TOO_LARGE]: 400, - [ApiErrorCode.BODY_TOO_LARGE]: 413, - [ApiErrorCode.IMPORT_IN_PROGRESS]: 409, - [ApiErrorCode.IDEMPOTENCY_CONFLICT]: 409, - [ApiErrorCode.IDEMPOTENCY_PENDING]: 409, - [ApiErrorCode.IMPORT_FAILED]: 500, - [ApiErrorCode.DATA_DIR_INCOMPATIBLE]: 409, - [ApiErrorCode.SERVER_ERROR]: 500, -} - -export class ApiError extends Error { - code: ApiErrorCode - statusCode: number - - constructor(code: ApiErrorCode, message: string) { - super(message) - this.name = 'ApiError' - this.code = code - this.statusCode = HTTP_STATUS[code] - } -} - -export function unauthorized(message = 'Invalid or missing token'): ApiError { - return new ApiError(ApiErrorCode.UNAUTHORIZED, message) -} - -export function sessionNotFound(id: string): ApiError { - return new ApiError(ApiErrorCode.SESSION_NOT_FOUND, `Session not found: ${id}`) -} - -export function invalidFormat(message: string): ApiError { - return new ApiError(ApiErrorCode.INVALID_FORMAT, message) -} - -export function invalidPayload(message: string): ApiError { - return new ApiError(ApiErrorCode.INVALID_PAYLOAD, message) -} - -export function idempotencyConflict(): ApiError { - return new ApiError(ApiErrorCode.IDEMPOTENCY_CONFLICT, 'Same Idempotency-Key with different request body hash') -} - -export function idempotencyPending(): ApiError { - return new ApiError( - ApiErrorCode.IDEMPOTENCY_PENDING, - 'A request with this Idempotency-Key is still in progress. Please retry later.' - ) -} - -export function sqlReadonlyViolation(): ApiError { - return new ApiError(ApiErrorCode.SQL_READONLY_VIOLATION, 'Only SELECT queries are allowed') -} - -export function sqlExecutionError(message: string): ApiError { - return new ApiError(ApiErrorCode.SQL_EXECUTION_ERROR, message) -} - -export function exportTooLarge(count: number, limit: number): ApiError { - return new ApiError( - ApiErrorCode.EXPORT_TOO_LARGE, - `Message count ${count} exceeds export limit ${limit}. Use paginated /messages API instead.` - ) -} - -export function importInProgress(): ApiError { - return new ApiError(ApiErrorCode.IMPORT_IN_PROGRESS, 'An import operation is already in progress') -} - -export function importFailed(message: string): ApiError { - return new ApiError(ApiErrorCode.IMPORT_FAILED, message) -} - -export function dataDirIncompatible(message: string): ApiError { - return new ApiError(ApiErrorCode.DATA_DIR_INCOMPATIBLE, message) -} - -export function serverError(message = 'Internal server error'): ApiError { - return new ApiError(ApiErrorCode.SERVER_ERROR, message) -} - -export function apiErrorFromUnknown(error: unknown): ApiError | null { - if (error instanceof ApiError) return error - if (error instanceof ImportInProgressError) return importInProgress() - const compatibilityError = findDataDirCompatibilityError(error) - if (compatibilityError) return dataDirIncompatible(compatibilityError.message) - return null -} - -function findDataDirCompatibilityError(error: unknown): DataDirCompatibilityError | null { - if (error instanceof DataDirCompatibilityError) return error - if (error instanceof Error && error.cause) return findDataDirCompatibilityError(error.cause) - return null -} - -export function successResponse(data: T, meta?: Record) { - return { - success: true as const, - data, - meta: { - timestamp: Math.floor(Date.now() / 1000), - version: '0.0.2', - ...meta, - }, - } -} - -export function errorResponse(error: ApiError) { - return { - success: false as const, - error: { - code: error.code, - message: error.message, - }, - } -} diff --git a/apps/desktop/main/api/routes/import-helpers.test.ts b/apps/desktop/main/api/routes/import-helpers.test.ts index 6485da0b..48fd78ac 100644 --- a/apps/desktop/main/api/routes/import-helpers.test.ts +++ b/apps/desktop/main/api/routes/import-helpers.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict' import test from 'node:test' import { IMPORT_IN_PROGRESS_ERROR_KEY } from '@openchatlab/node-runtime/src/import/import-lock' -import { ApiErrorCode } from '../errors' +import { ApiErrorCode } from '@openchatlab/http-routes/errors' import { analysisFromNewImport, analysisFromPushImport, apiErrorFromImportResult } from './import-helpers' test('maps an import lock result to the documented 409 API error', () => { @@ -9,6 +9,7 @@ test('maps an import lock result to the documented 409 API error', () => { assert.equal(error.code, ApiErrorCode.IMPORT_IN_PROGRESS) assert.equal(error.statusCode, 409) + assert.equal(error.message, 'An import operation is already in progress') }) test('keeps ordinary import failures as IMPORT_FAILED', () => { diff --git a/apps/desktop/main/api/routes/import-helpers.ts b/apps/desktop/main/api/routes/import-helpers.ts index 377e5edc..1d62e73a 100644 --- a/apps/desktop/main/api/routes/import-helpers.ts +++ b/apps/desktop/main/api/routes/import-helpers.ts @@ -1,10 +1,18 @@ import { IMPORT_IN_PROGRESS_ERROR_KEY } from '@openchatlab/node-runtime/src/import/import-lock' import type { PushImportAnalysisResult } from '@openchatlab/node-runtime/src/services/push-importer' import type { AnalyzeNewImportResult } from '@openchatlab/node-runtime/src/import/streaming-importer' -import { importFailed, importInProgress, type ApiError } from '../errors' +import { importFailed, importInProgress, type ApiError } from '@openchatlab/http-routes/errors' + +const DESKTOP_IMPORT_IN_PROGRESS_MESSAGE = 'An import operation is already in progress' + +export function desktopImportInProgressError(): ApiError { + return importInProgress(DESKTOP_IMPORT_IN_PROGRESS_MESSAGE) +} export function apiErrorFromImportResult(error: string | undefined, fallbackMessage: string): ApiError { - return error === IMPORT_IN_PROGRESS_ERROR_KEY ? importInProgress() : importFailed(error || fallbackMessage) + return error === IMPORT_IN_PROGRESS_ERROR_KEY + ? desktopImportInProgressError() + : importFailed(error || fallbackMessage) } export function analysisFromNewImport(result: AnalyzeNewImportResult): { diff --git a/apps/desktop/main/api/routes/import.ts b/apps/desktop/main/api/routes/import.ts index 70d72f5e..d5efa068 100644 --- a/apps/desktop/main/api/routes/import.ts +++ b/apps/desktop/main/api/routes/import.ts @@ -20,23 +20,27 @@ import * as crypto from 'crypto' import { pipeline } from 'stream/promises' import { hashImportBody, ImportIdempotencyCache, isValidImportSessionId } from '@openchatlab/node-runtime' import type { PushImportPayload } from '@openchatlab/node-runtime' -import { getTempDir } from '../../paths/locations' -import * as worker from '../../worker/workerManager' import { ApiError, ApiErrorCode, successResponse, apiErrorFromUnknown, - importInProgress, importFailed, invalidFormat, invalidPayload, idempotencyConflict, idempotencyPending, errorResponse, -} from '../errors' +} from '@openchatlab/http-routes/errors' +import { getTempDir } from '../../paths/locations' +import * as worker from '../../worker/workerManager' import { apiLogger } from '../logger' -import { analysisFromNewImport, analysisFromPushImport, apiErrorFromImportResult } from './import-helpers' +import { + analysisFromNewImport, + analysisFromPushImport, + apiErrorFromImportResult, + desktopImportInProgressError, +} from './import-helpers' // Tracks active External API requests; the shared data-directory lock enforces writer exclusion. const isImporting = new Set() @@ -188,7 +192,7 @@ async function handleUnifiedImport(request: FastifyRequest, reply: FastifyReply, // IDEMPOTENCY_PENDING；只有本次新占位的 key 才会在锁冲突时释放。 if (isImporting.has(sessionId)) { idempotencyFail(cacheKey) - const err = importInProgress() + const err = desktopImportInProgressError() reply.code(err.statusCode).send(errorResponse(err)) return } @@ -266,7 +270,7 @@ async function handleUnifiedImport(request: FastifyRequest, reply: FastifyReply, idempotencyFail(cacheKey) const err = outcome.reason === 'import_in_progress' - ? importInProgress() + ? desktopImportInProgressError() : outcome.reason === 'invalid_payload' ? invalidPayload(outcome.message) : importFailed(outcome.message) @@ -341,7 +345,7 @@ async function handleUnifiedImport(request: FastifyRequest, reply: FastifyReply, async function handleLegacyImport(request: FastifyRequest, reply: FastifyReply, sessionId?: string): Promise { const lockKey = sessionId ?? '__legacy__' if (isImporting.has(lockKey)) { - const err = importInProgress() + const err = desktopImportInProgressError() reply.code(err.statusCode).send(errorResponse(err)) return } diff --git a/apps/desktop/main/api/routes/sessions.ts b/apps/desktop/main/api/routes/sessions.ts index 582749df..09e0c507 100644 --- a/apps/desktop/main/api/routes/sessions.ts +++ b/apps/desktop/main/api/routes/sessions.ts @@ -3,8 +3,15 @@ */ import type { FastifyInstance } from 'fastify' +import { + successResponse, + sessionNotFound, + exportTooLarge, + sqlExecutionError, + ApiError, + errorResponse, +} from '@openchatlab/http-routes/errors' import * as worker from '../../worker/workerManager' -import { successResponse, sessionNotFound, exportTooLarge, sqlExecutionError, ApiError, errorResponse } from '../errors' const EXPORT_MESSAGE_LIMIT = 100_000 diff --git a/apps/desktop/main/api/routes/system.ts b/apps/desktop/main/api/routes/system.ts index 236c0870..f4cd37e1 100644 --- a/apps/desktop/main/api/routes/system.ts +++ b/apps/desktop/main/api/routes/system.ts @@ -6,7 +6,7 @@ import type { FastifyInstance } from 'fastify' import { app } from 'electron' -import { successResponse } from '../errors' +import { successResponse } from '@openchatlab/http-routes/errors' import * as worker from '../../worker/workerManager' import { getDesktopAppVersion } from '../../runtime/compat' diff --git a/apps/desktop/main/api/server.ts b/apps/desktop/main/api/server.ts index 8670170d..4c3d458c 100644 --- a/apps/desktop/main/api/server.ts +++ b/apps/desktop/main/api/server.ts @@ -3,8 +3,14 @@ */ import Fastify, { type FastifyInstance, type FastifyError } from 'fastify' +import { + ApiError, + ApiErrorCode, + apiErrorFromUnknown, + errorResponse, + serverError, +} from '@openchatlab/http-routes/errors' import { authHook } from './auth' -import { ApiError, ApiErrorCode, apiErrorFromUnknown, errorResponse, serverError } from './errors' import { apiLogger } from './logger' const JSON_BODY_LIMIT = 50 * 1024 * 1024 // 50MB diff --git a/apps/desktop/main/internal-api/http.ts b/apps/desktop/main/internal-api/http.ts index 8bacac2c..b344bb64 100644 --- a/apps/desktop/main/internal-api/http.ts +++ b/apps/desktop/main/internal-api/http.ts @@ -1,5 +1,11 @@ import type { FastifyError, FastifyInstance, FastifyReply } from 'fastify' -import { ApiError, ApiErrorCode, apiErrorFromUnknown, errorResponse, serverError } from '@openchatlab/http-routes' +import { + ApiError, + ApiErrorCode, + apiErrorFromUnknown, + errorResponse, + serverError, +} from '@openchatlab/http-routes/errors' import { appLogger } from '@openchatlab/node-runtime' import { createInternalAuthHook } from './auth' diff --git a/packages/http-routes/package.json b/packages/http-routes/package.json index cd726b16..7f0916f9 100644 --- a/packages/http-routes/package.json +++ b/packages/http-routes/package.json @@ -6,6 +6,11 @@ "type": "module", "main": "./src/index.ts", "types": "./src/index.ts", + "exports": { + ".": "./src/index.ts", + "./auth": "./src/auth.ts", + "./errors": "./src/errors.ts" + }, "dependencies": { "@openchatlab/config": "workspace:*", "@openchatlab/core": "workspace:*", diff --git a/packages/http-routes/src/errors.test.ts b/packages/http-routes/src/errors.test.ts index 9d54538d..49014991 100644 --- a/packages/http-routes/src/errors.test.ts +++ b/packages/http-routes/src/errors.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict' import test from 'node:test' -import { DataDirCompatibilityError } from '@openchatlab/node-runtime/src/data-dir-compat' -import { ApiErrorCode, apiErrorFromUnknown } from './errors' +import { DataDirCompatibilityError } from '@openchatlab/node-runtime/data-dir-compat' +import { ApiErrorCode, apiErrorFromUnknown, invalidFormat } from './errors' import { ImportInProgressError } from '@openchatlab/node-runtime/src/import/import-lock' test('apiErrorFromUnknown maps the shared import lock error to 409', () => { @@ -29,3 +29,30 @@ test('apiErrorFromUnknown maps data directory compatibility errors to 409', () = assert.equal(apiError?.statusCode, 409) assert.match(apiError?.message ?? '', /requires runtime version 0\.25\.1/) }) + +test('apiErrorFromUnknown maps wrapped data directory compatibility errors to 409', () => { + const cause = new DataDirCompatibilityError( + 'DATA_DIR_REQUIRES_NEWER_RUNTIME', + 'ChatLab data directory requires runtime version 0.25.1 or newer; current version is 0.25.0.', + { + userDataDir: '/tmp/chatlab-data', + metaPath: '/tmp/chatlab-data/.chatlab-meta.json', + currentVersion: '0.25.0', + minRuntimeVersion: '0.25.1', + } + ) + + const apiError = apiErrorFromUnknown(new Error('Platform-formatted message', { cause })) + + assert.equal(apiError?.code, ApiErrorCode.DATA_DIR_INCOMPATIBLE) + assert.equal(apiError?.statusCode, 409) + assert.match(apiError?.message ?? '', /requires runtime version 0\.25\.1/) +}) + +test('invalidFormat creates the canonical INVALID_FORMAT response error', () => { + const error = invalidFormat('Content-Type must be application/json') + + assert.equal(error.code, ApiErrorCode.INVALID_FORMAT) + assert.equal(error.statusCode, 400) + assert.equal(error.message, 'Content-Type must be application/json') +}) diff --git a/packages/http-routes/src/errors.ts b/packages/http-routes/src/errors.ts index 81dda167..6201054f 100644 --- a/packages/http-routes/src/errors.ts +++ b/packages/http-routes/src/errors.ts @@ -4,7 +4,7 @@ * Platform-agnostic error handling shared by CLI Server and Electron Internal Server. */ -import { DataDirCompatibilityError } from '@openchatlab/node-runtime/src/data-dir-compat' +import { DataDirCompatibilityError } from '@openchatlab/node-runtime/data-dir-compat' import { ImportInProgressError } from '@openchatlab/node-runtime/src/import/import-lock' export enum ApiErrorCode { @@ -61,6 +61,10 @@ export function sessionNotFound(id: string): ApiError { return new ApiError(ApiErrorCode.SESSION_NOT_FOUND, `Session not found: ${id}`) } +export function invalidFormat(message: string): ApiError { + return new ApiError(ApiErrorCode.INVALID_FORMAT, message) +} + export function invalidPayload(message: string): ApiError { return new ApiError(ApiErrorCode.INVALID_PAYLOAD, message) } @@ -84,8 +88,8 @@ export function serverError(message = 'Internal server error'): ApiError { return new ApiError(ApiErrorCode.SERVER_ERROR, message) } -export function importInProgress(): ApiError { - return new ApiError(ApiErrorCode.IMPORT_IN_PROGRESS, 'Another import is already in progress') +export function importInProgress(message = 'Another import is already in progress'): ApiError { + return new ApiError(ApiErrorCode.IMPORT_IN_PROGRESS, message) } export function idempotencyConflict(): ApiError { @@ -110,7 +114,21 @@ export function dataDirIncompatible(message: string): ApiError { export function apiErrorFromUnknown(error: unknown): ApiError | null { if (error instanceof ApiError) return error if (error instanceof ImportInProgressError) return importInProgress() - if (error instanceof DataDirCompatibilityError) return dataDirIncompatible(error.message) + const compatibilityError = findDataDirCompatibilityError(error) + if (compatibilityError) return dataDirIncompatible(compatibilityError.message) + return null +} + +function findDataDirCompatibilityError(error: unknown): DataDirCompatibilityError | null { + const seen = new Set() + let current = error + + while (current instanceof Error && !seen.has(current)) { + if (current instanceof DataDirCompatibilityError) return current + seen.add(current) + current = current.cause + } + return null } diff --git a/packages/http-routes/src/index.ts b/packages/http-routes/src/index.ts index 35d0aab9..fc0a7094 100644 --- a/packages/http-routes/src/index.ts +++ b/packages/http-routes/src/index.ts @@ -14,11 +14,16 @@ export { ApiErrorCode, unauthorized, sessionNotFound, + invalidFormat, invalidPayload, sqlReadonlyViolation, sqlExecutionError, exportTooLarge, serverError, + importInProgress, + idempotencyConflict, + idempotencyPending, + importFailed, dataDirIncompatible, apiErrorFromUnknown, successResponse,