From 8cfd69c7c92518cbc2eeb05ec26e1f9ebc94c555 Mon Sep 17 00:00:00 2001 From: zipg Date: Thu, 3 Sep 2026 13:43:55 +0800 Subject: [PATCH] fix(query): preserve original backend errors with fallback detail --- .../__tests__/useSqlExecution.spec.ts | 27 ++++++++++- .../src/composables/useSqlExecution.ts | 5 ++- .../src/i18n/__tests__/backendErrors.spec.ts | 45 +++++++++++++++++++ apps/desktop/src/i18n/backend-errors.ts | 23 +++++++--- apps/desktop/src/lib/backend/errorUtils.ts | 4 +- .../queryStore.multiStatementError.spec.ts | 37 +++++++++++++++ apps/desktop/src/stores/queryStore.ts | 6 +-- 7 files changed, 134 insertions(+), 13 deletions(-) diff --git a/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts b/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts index c445847160..243577accc 100644 --- a/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts +++ b/apps/desktop/src/composables/__tests__/useSqlExecution.spec.ts @@ -1023,7 +1023,24 @@ SELECT @value AS Message;`; if (!tab) return; const successfulResult = { columns: ["value"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }; tab.result = successfulResult; - tab.results = [successfulResult, { columns: ["Error"], execution_error: true, rows: [["Duplicate entry '1'"]], affected_rows: 0, execution_time_ms: 1 }]; + tab.results = [ + successfulResult, + { + columns: ["Error"], + execution_error: true, + error: { + version: 1, + code: "DBX-LEGACY-0001", + messageKey: "backendErrors.legacy", + messageParams: {}, + source: "legacyBackend", + operationOutcome: "unknown", + }, + rows: [["Duplicate entry '1'"]], + affected_rows: 0, + execution_time_ms: 1, + }, + ]; tab.activeResultIndex = 0; }); const addHistory = vi.spyOn(historyStore, "add").mockResolvedValue(undefined); @@ -1038,7 +1055,13 @@ SELECT @value AS Message;`; await execution.tryExecute(); - expect(addHistory).toHaveBeenCalledWith(expect.objectContaining({ success: false, error: "Duplicate entry '1'", affected_rows: undefined })); + expect(addHistory).toHaveBeenCalledWith( + expect.objectContaining({ + success: false, + error: "backendErrors.unknown\n\nDuplicate entry '1'", + affected_rows: undefined, + }), + ); expect(refreshObjects).not.toHaveBeenCalled(); }); diff --git a/apps/desktop/src/composables/useSqlExecution.ts b/apps/desktop/src/composables/useSqlExecution.ts index c2a16ae067..a88189510d 100644 --- a/apps/desktop/src/composables/useSqlExecution.ts +++ b/apps/desktop/src/composables/useSqlExecution.ts @@ -27,6 +27,7 @@ import type { ConnectionConfig, DatabaseType, QueryTab } from "@/types/database" import type { MultiDbExecutionTarget, MultiDbResultRunExecution, MultiDbTargetExecutionResult } from "@/types/sqlExecution"; import { effectiveDatabaseTypeForConnection } from "@/lib/database/jdbcDialect"; import type { SqlExecutionTargetContext } from "@/lib/database/sqlExecutionTargetRegistry"; +import { translateBackendError } from "@/i18n/backend-errors"; const DANGER_RE = /^\s*(DROP|DELETE|TRUNCATE|ALTER|UPDATE|MERGE|REPLACE)\b/i; @@ -348,7 +349,7 @@ export function useSqlExecution(deps: { sql, execution_time_ms: elapsed, success, - error: failure ? String(failure.rows?.[0]?.[0] ?? "") : undefined, + error: failure ? (failure.error ? translateBackendError(t, failure.error, failure.rows?.[0]?.[0]) : String(failure.rows?.[0]?.[0] ?? "")) : undefined, activity_kind: classifySqlActivityKind(sql), operation: primarySqlOperation(sql), affected_rows: success ? tab.result?.affected_rows : undefined, @@ -484,7 +485,7 @@ export function useSqlExecution(deps: { } focusSqlServerDataResult(executionTabId, connection.db_type, latest); const failure = firstQueryExecutionError(latest); - const errorMessage = failure ? String(failure.rows?.[0]?.[0] ?? t("common.failed")) : undefined; + const errorMessage = failure ? (failure.error ? translateBackendError(t, failure.error, failure.rows?.[0]?.[0]) : String(failure.rows?.[0]?.[0] ?? t("common.failed"))) : undefined; const success = !failure; const resultStatus = success ? "success" : "failed"; captureWorkerResult(resultStatus, errorMessage); diff --git a/apps/desktop/src/i18n/__tests__/backendErrors.spec.ts b/apps/desktop/src/i18n/__tests__/backendErrors.spec.ts index e0262a330a..60ae578f45 100644 --- a/apps/desktop/src/i18n/__tests__/backendErrors.spec.ts +++ b/apps/desktop/src/i18n/__tests__/backendErrors.spec.ts @@ -345,6 +345,51 @@ describe("backend error translation", () => { expect(translateBackendError(t, error)).toBe(`${t("backendErrors.legacy")}\n\nlegacy backend failure`); }); + test("keeps an explicit original detail when a structured legacy error omits detail", () => { + const t = translatorFor("zh-CN"); + const error = { + version: 1 as const, + code: "DBX-LEGACY-0001", + messageKey: "backendErrors.legacy", + messageParams: {}, + source: "legacyBackend", + operationOutcome: "unknown" as const, + }; + + expect(translateBackendError(t, error, "ClickHouse error: table analytics.events does not exist")).toBe(`${t("backendErrors.legacy")}\n\nClickHouse error: table analytics.events does not exist`); + }); + + test("does not append the generic transport fallback to a structured error", () => { + const t = translatorFor("zh-CN"); + const error = { + version: 1 as const, + code: "DBX-LEGACY-0001", + messageKey: "backendErrors.legacy", + messageParams: {}, + source: "legacyBackend", + operationOutcome: "unknown" as const, + }; + + expect(translateBackendError(t, error, "Backend request failed")).toBe(t("backendErrors.legacy")); + }); + + test("does not duplicate the summary when the fallback already carries it", () => { + const t = translatorFor("zh-CN"); + const error = { + version: 1 as const, + code: "DBX-LEGACY-0001", + messageKey: "backendErrors.legacy", + messageParams: {}, + source: "legacyBackend", + operationOutcome: "unknown" as const, + }; + const summary = t("backendErrors.legacy"); + + expect(translateBackendError(t, error, `${summary}\n\nrelation missing_table does not exist`)).toBe( + `${summary}\n\nrelation missing_table does not exist`, + ); + }); + test("preserves JSON envelopes carried by strings and Error messages", () => { const envelope = { version: 1, diff --git a/apps/desktop/src/i18n/backend-errors.ts b/apps/desktop/src/i18n/backend-errors.ts index 78d9ff8936..71a5681508 100644 --- a/apps/desktop/src/i18n/backend-errors.ts +++ b/apps/desktop/src/i18n/backend-errors.ts @@ -1,4 +1,4 @@ -import { normalizeBackendError, sanitizeBackendErrorMessage, type BackendError } from "@/lib/backend/errorUtils"; +import { GENERIC_TRANSPORT_FAILURE_MESSAGE, normalizeBackendError, sanitizeBackendErrorMessage, type BackendError } from "@/lib/backend/errorUtils"; import { PHOENIX_DRIVER_NOT_INSTALLED_ERROR, PHOENIX_JDBC_PLUGIN_NOT_INSTALLED_ERROR } from "@/lib/database/phoenixConnection"; /** @@ -181,19 +181,32 @@ function backendErrorMessage(error: unknown): string { return sanitizeBackendErrorMessage(String(error)); } -function translateStructuredBackendError(t: BackendErrorTranslate, error: BackendError): string { +function usableFallbackDetail(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const detail = sanitizeBackendErrorMessage(value).trim(); + if (!detail || detail === GENERIC_TRANSPORT_FAILURE_MESSAGE) return undefined; + return detail; +} + +function translateStructuredBackendError(t: BackendErrorTranslate, error: BackendError, fallbackDetail?: unknown): string { const translated = t(error.messageKey, error.messageParams); const summary = translated !== error.messageKey ? translated : t("backendErrors.unknown"); - const detail = error.detail ? sanitizeBackendErrorMessage(error.detail).trim() : undefined; + let detail = error.detail ? sanitizeBackendErrorMessage(error.detail).trim() : usableFallbackDetail(fallbackDetail); + // Callers may pass an already-composited "summary\n\noriginal" cell as the + // fallback (query history re-translation of a failed result grid); strip the + // repeated summary so the message stays singular. + if (detail && detail.startsWith(`${summary}\n\n`)) { + detail = detail.slice(summary.length + 2).trim() || undefined; + } const rawAdapterCode = error.diagnostics?.adapterCode; const adapterCode = typeof rawAdapterCode === "string" && /^[A-Za-z][A-Za-z0-9_.-]{0,63}$/.test(rawAdapterCode) ? rawAdapterCode : undefined; const diagnosticDetail = detail && adapterCode ? `[${adapterCode}] ${detail}` : (detail ?? adapterCode); return diagnosticDetail && diagnosticDetail !== summary ? `${summary}\n\n${diagnosticDetail}` : summary; } -export function translateBackendError(t: BackendErrorTranslate, error: unknown): string { +export function translateBackendError(t: BackendErrorTranslate, error: unknown, fallbackDetail?: unknown): string { const structured = normalizeBackendError(error); - if (structured) return translateStructuredBackendError(t, structured); + if (structured) return translateStructuredBackendError(t, structured, fallbackDetail); const message = backendErrorMessage(error); const exactKey = exactMessageKeys[message]; diff --git a/apps/desktop/src/lib/backend/errorUtils.ts b/apps/desktop/src/lib/backend/errorUtils.ts index 84faad60e1..6dbf63e845 100644 --- a/apps/desktop/src/lib/backend/errorUtils.ts +++ b/apps/desktop/src/lib/backend/errorUtils.ts @@ -141,13 +141,15 @@ function normalizeBackendErrorAtDepth(error: unknown, seen: WeakSet, dep return null; } +export const GENERIC_TRANSPORT_FAILURE_MESSAGE = "Backend request failed"; + export class BackendErrorException extends Error { readonly backendError: BackendError; constructor(error: unknown) { const backendError = normalizeRawBackendError(error); const fallbackDetail = boundedFallbackText(error); - const fallbackMessage = sanitizeBackendErrorMessage(fallbackDetail ?? "Backend request failed"); + const fallbackMessage = sanitizeBackendErrorMessage(fallbackDetail ?? GENERIC_TRANSPORT_FAILURE_MESSAGE); super(backendError?.detail ? sanitizeBackendErrorMessage(backendError.detail) : fallbackMessage); this.name = "BackendErrorException"; this.backendError = backendError ?? { diff --git a/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts index 70d57f77fe..67ed9e13eb 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.multiStatementError.spec.ts @@ -1,5 +1,6 @@ import { createPinia, setActivePinia } from "pinia"; import { isActiveResultLoading } from "@/lib/sql/queryExecutionState"; +import { BackendErrorException } from "@/lib/backend/errorUtils"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ @@ -173,6 +174,42 @@ describe("queryStore multi-statement errors", () => { expect(tab.batchSqlExecution?.items[1]?.errorDetails).toEqual(structuredError); expect(tab.batchSqlExecution?.items[1]?.error).not.toBe(structuredError.code); expect(tab.batchSqlExecution?.items[1]?.error).not.toBe("[object Object]"); + expect(tab.batchSqlExecution?.items[1]?.error).toContain("no such table: missing"); + }); + + it("preserves the original message when a top-level structured error omits detail", async () => { + const structuredError = { + version: 1 as const, + code: "DBX-LEGACY-0001", + messageKey: "backendErrors.legacy", + messageParams: {}, + source: "legacyBackend" as const, + operationOutcome: "unknown" as const, + }; + const originalMessage = "ClickHouse error: table iceberg_backend.missing_table does not exist"; + mocks.getConnectionConfig.mockReturnValue({ + id: "clickhouse-1", + name: "ClickHouse", + db_type: "clickhouse", + database: "iceberg_backend", + query_timeout_secs: 30, + }); + mocks.executeMulti.mockRejectedValue( + new BackendErrorException({ + backendError: structuredError, + message: originalMessage, + }), + ); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("clickhouse-1", "iceberg_backend", "Query"); + + await store.executeTabSql(tabId, "SELECT * FROM missing_table"); + + const tab = store.tabs.find((item) => item.id === tabId)!; + expect(tab.result?.execution_error).toBe(true); + expect(tab.result?.rows[0]?.[0]).toContain(originalMessage); + expect(tab.batchSqlExecution?.items[0]?.error).toContain(originalMessage); }); it("updates live per-statement progress before the batch promise resolves", async () => { diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index 97faf7ca30..4401c98c0c 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -411,7 +411,7 @@ function reconcileBatchSqlResults(tab: QueryTab, executionId: string, results: Q item.executionTimeMs = result.execution_time_ms; item.affectedRows = result.affected_rows; item.errorDetails = failed ? result.error : undefined; - item.error = failed ? (result.error ? translateBackendError(i18n.global.t, result.error) : String(result.rows[0]?.[0] ?? "")) : undefined; + item.error = failed ? (result.error ? translateBackendError(i18n.global.t, result.error, result.rows[0]?.[0]) : String(result.rows[0]?.[0] ?? "")) : undefined; } batch.completed = batch.items.filter((item) => item.status === "success" || item.status === "error").length; } @@ -423,7 +423,7 @@ function failBatchSqlExecution(tab: QueryTab, executionId: string, error: unknow if (!item) return; item.status = cancelled ? "cancelled" : "error"; item.errorDetails = cancelled ? undefined : (normalizeBackendError(error) ?? undefined); - item.error = cancelled ? undefined : translateBackendError(i18n.global.t, error); + item.error = cancelled ? undefined : translateBackendError(i18n.global.t, error, error instanceof Error ? error.message : undefined); batch.completed = batch.items.filter((candidate) => candidate.status === "success" || candidate.status === "error").length; } @@ -3856,7 +3856,7 @@ export const useQueryStore = defineStore("query", () => { // Single funnel for every query execution failure, so backend messages DBX // knows about are shown in the active locale rather than as raw English. const error = normalizeBackendError(e) ?? undefined; - const message = translateBackendError(i18n.global.t, e); + const message = translateBackendError(i18n.global.t, e, e instanceof Error ? e.message : undefined); return markQueryResultRowsRaw({ columns: ["Error"], execution_error: true,