@kaavee315draftchecks…fix/toollog-service-result → master15 files · +601 −210updated 5d agoConverts ToolLogService.getLogs and getSingleLog from throwing raw Error objects to returning ts-results Result<T, Error>, consistent with the repo's typed error-handling convention.
Problem: Both ClickHouseToolLogs and PostgresToolLogs had throw new Error(...) in their failure paths (clickhouse-tool-logs.ts and postgres-tool-logs.ts). These bypassed typed error handling and relied solely on the global Hono onError catcher, making callers unable to distinguish error types (e.g. not-found vs internal error).
Fix:
ToolLog error codes (5700 ToolLog_InternalServerError, 5701 ToolLog_NotFound) and ToolLogError export to apps/apollo/src/common/constants/error.ts (5700-5799 range, follows the existing reserved-range convention; no collisions).ToolLogService interface in both types.ts (clickhouse) and tool-log-types.ts (postgres):
getLogs returns Promise<Result<T, Error_500>> (only ever fails with an internal error).getSingleLog returns Promise<Result<T, Error_404 | Error_500>> — the not-found path returns a typed 404 (ToolLogError.NotFound, built on Error_404), the parse/internal path returns 500. Both ClickHouseToolLogs and PostgresToolLogs getSingleLog signatures use the same Error_404 | Error_500 union (an Error_404 value is not assignable to a bare Error_500 channel, so the union is required for the code to type-check).throw statements with return Err(new ToolLogError.*(...)); wrapped success paths in Ok(...).apps/apollo/src/pages/api/v3/internal/action_execution/logs.ts — getLogsapps/apollo/src/pages/api/v3/internal/action_execution/log/[id].ts — getSingleLog (also added a 404 response to the route's OpenAPI responses; previously a missing log fell through to the global onError as a 500, now it returns a typed 404 consistent with the other log-detail routes)apps/apollo/src/pages/api/internal-dashboard/consumer/logs/poll-action-execution.ts — getLogsapps/apollo/src/pages/api/internal-dashboard/consumer/logs/tool_execution/[id].ts — getSingleLog (removed try-catch; route already declared 404)apps/apollo/src/pages/api/internal-dashboard/consumer/logs/tool_execution/index.ts — getLogsapps/apollo/src/pages/api/v3.1/logs/tool_execution/[id].ts — getSingleLog (removed try-catch; route already declared 404)apps/apollo/src/pages/api/v3.1/logs/tool_execution/index.ts — getLogsAll callers now use the if (result.err) { return errorToHTTPException(c, result.val) } pattern. errorToHTTPException reads the live statusCode off the error instance, so the 404-vs-500 distinction is preserved at runtime. logRequest is unchanged (still throws by design); its callers are unaffected.
apps/apollo/src/lib/logger/tool-logs-service-result.unit.test.ts covering:
ClickHouseToolLogs.getLogs: Ok on success, Err (not throw) on thermos failureClickHouseToolLogs.getSingleLog: Ok on success, Err(NotFound) (not throw) on empty ClickHouse responsePostgresToolLogs.getLogs: Ok on success, Err (not throw) on Prisma failure and thermos failurePostgresToolLogs.getSingleLog: Ok on success, Err(NotFound) (not throw) on null log and Prisma errorpostgres-tool-logs.test.self-hosted.ts) exercises the routes over HTTP; its "missing log" assertion (status >= 400) still holds with the new 404.getSingleLog error channel from Error_500 to Error_404 | Error_500 in both ToolLogService interfaces and both impl signatures: ToolLogError.NotFound is an Error_404, which is not assignable to Error_500 (incompatible statusCode literal), so the original code would have failed check-types.404 response to /api/v3/internal/action_execution/log/{id} so the strict @hono/zod-openapi handler return type admits the now-typed 404 propagated via errorToHTTPException.🤖 Generated with Claude Code