From fa64952c080e557bff76d7db12689392b1747425 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Sat, 18 Jul 2026 00:10:58 +0800 Subject: [PATCH] fix(explain): preserve backend error details --- .../backend/__tests__/getExplainInfo.spec.ts | 41 +++++++ apps/desktop/src/lib/backend/http.ts | 8 +- apps/desktop/src/lib/backend/tauri.ts | 9 +- .../queryStore.oracleExplain.spec.ts | 112 ++++++++++++++++++ apps/desktop/src/stores/queryStore.ts | 4 +- 5 files changed, 160 insertions(+), 14 deletions(-) create mode 100644 apps/desktop/src/lib/backend/__tests__/getExplainInfo.spec.ts create mode 100644 apps/desktop/src/stores/__tests__/queryStore.oracleExplain.spec.ts diff --git a/apps/desktop/src/lib/backend/__tests__/getExplainInfo.spec.ts b/apps/desktop/src/lib/backend/__tests__/getExplainInfo.spec.ts new file mode 100644 index 0000000000..79419dddd6 --- /dev/null +++ b/apps/desktop/src/lib/backend/__tests__/getExplainInfo.spec.ts @@ -0,0 +1,41 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + invoke: vi.fn(), +})); + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: mocks.invoke, +})); + +vi.mock("@tauri-apps/api/event", () => ({ + listen: vi.fn(), +})); + +describe("getExplainInfo backend error propagation", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); + }); + + it("preserves Tauri command errors", async () => { + const error = new Error("ORA-01031: insufficient privileges"); + mocks.invoke.mockRejectedValue(error); + const { getExplainInfo } = await import("@/lib/backend/tauri"); + + await expect(getExplainInfo("oracle-1", "ORCL", "APP", "SELECT * FROM DUAL", "explain")).rejects.toBe(error); + }); + + it("preserves HTTP response errors", async () => { + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue({ + ok: false, + text: vi.fn().mockResolvedValue("Agent error: PLAN_TABLE is not accessible"), + }), + ); + const { getExplainInfo } = await import("@/lib/backend/http"); + + await expect(getExplainInfo("oracle-1", "ORCL", "APP", "SELECT * FROM DUAL", "explain")).rejects.toThrow("Agent error: PLAN_TABLE is not accessible"); + }); +}); diff --git a/apps/desktop/src/lib/backend/http.ts b/apps/desktop/src/lib/backend/http.ts index 31d48fbe24..a04f6a97a3 100644 --- a/apps/desktop/src/lib/backend/http.ts +++ b/apps/desktop/src/lib/backend/http.ts @@ -810,12 +810,8 @@ export async function buildCreateUserSql(username: string, password: string, tab } export async function getExplainInfo(connectionId: string, database: string | undefined, schema: string | undefined, sql: string, mode: string): Promise { - try { - const result = await post("/api/query/get-explain-info", { connectionId, database, schema, sql, mode }); - return result; - } catch { - return undefined; - } + // Match the Tauri path: transport and Agent failures must remain distinguishable from an empty plan. + return post("/api/query/get-explain-info", { connectionId, database, schema, sql, mode }); } export async function buildDroppedFilePreviewSql(options: DroppedFilePreviewSqlOptions): Promise { diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index e86d9bb09e..83018f7706 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -901,13 +901,8 @@ export async function buildCreateUserSql(username: string, password: string, tab } export async function getExplainInfo(connectionId: string, database: string | undefined, schema: string | undefined, sql: string, mode: string): Promise { - try { - const result = await invoke("get_explain_info", { connectionId, database, schema, sql, mode }); - return result; - } catch (e: any) { - console.error("[getExplainInfo] invoke failed:", e?.message || e); - return undefined; - } + // Preserve Agent/driver errors so the explain view can show the actionable cause. + return invoke("get_explain_info", { connectionId, database, schema, sql, mode }); } export async function buildDroppedFilePreviewSql(options: DroppedFilePreviewSqlOptions): Promise { diff --git a/apps/desktop/src/stores/__tests__/queryStore.oracleExplain.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.oracleExplain.spec.ts new file mode 100644 index 0000000000..33ae36a03e --- /dev/null +++ b/apps/desktop/src/stores/__tests__/queryStore.oracleExplain.spec.ts @@ -0,0 +1,112 @@ +import { createPinia, setActivePinia } from "pinia"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + buildExplainSql: vi.fn(), + getExplainInfo: vi.fn(), + parseOracleExplainText: vi.fn(), + saveOpenTabsState: vi.fn(), + getConfig: vi.fn(), +})); + +vi.mock("@/lib/diagram/explainPlan", () => ({ + buildExplainSql: mocks.buildExplainSql, + parseExplainResult: vi.fn(), + parseDamengExplainText: vi.fn(), + parseOracleExplainText: mocks.parseOracleExplainText, +})); + +vi.mock("@/lib/backend/api", () => ({ + getExplainInfo: mocks.getExplainInfo, + saveOpenTabsState: mocks.saveOpenTabsState, +})); + +vi.mock("@/stores/connectionStore", () => ({ + useConnectionStore: () => ({ + getConfig: mocks.getConfig, + recordConnectionLostError: vi.fn(), + }), +})); + +vi.mock("@/stores/settingsStore", () => ({ + useSettingsStore: () => ({ + editorSettings: { pageSize: 100, openTabsRestoreMode: "all", confirmUnsavedSqlClose: false }, + }), +})); + +function installLocalStorage() { + const data = new Map(); + vi.stubGlobal("localStorage", { + getItem: vi.fn((key: string) => data.get(key) ?? null), + setItem: vi.fn((key: string, value: string) => data.set(key, value)), + removeItem: vi.fn((key: string) => data.delete(key)), + }); +} + +describe("queryStore Oracle explain errors", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.unstubAllGlobals(); + installLocalStorage(); + setActivePinia(createPinia()); + mocks.getConfig.mockReturnValue({ id: "oracle-1", name: "Oracle", db_type: "oracle" }); + mocks.buildExplainSql.mockResolvedValue({ ok: true, sql: "EXPLAIN PLAN FOR SELECT * FROM DUAL" }); + mocks.saveOpenTabsState.mockResolvedValue(undefined); + }); + + it("shows the real ORA error returned by the backend", async () => { + mocks.getExplainInfo.mockRejectedValue(new Error("ORA-01031: insufficient privileges")); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query", "query", "APP"); + + await store.explainTabSql(tabId, "SELECT * FROM DUAL", "oracle"); + + expect(store.tabs.find((tab) => tab.id === tabId)).toMatchObject({ + isExplaining: false, + explainPlan: undefined, + explainError: "ORA-01031: insufficient privileges", + }); + }); + + it("shows string Agent errors without replacing them", async () => { + mocks.getExplainInfo.mockRejectedValue("Agent error: DBMS_XPLAN execution failed"); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + + await store.explainTabSql(tabId, "SELECT * FROM DUAL", "oracle"); + + expect(store.tabs.find((tab) => tab.id === tabId)?.explainError).toBe("Agent error: DBMS_XPLAN execution failed"); + }); + + it("keeps the empty-plan message for a successful empty response", async () => { + mocks.getExplainInfo.mockResolvedValue(""); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + + await store.explainTabSql(tabId, "SELECT * FROM DUAL", "oracle"); + + expect(store.tabs.find((tab) => tab.id === tabId)?.explainError).toBe("No explain plan returned"); + expect(mocks.parseOracleExplainText).not.toHaveBeenCalled(); + }); + + it("keeps successful Oracle explain behavior unchanged", async () => { + const parsedPlan = { databaseType: "oracle", rawText: "Plan hash value: 123", nodes: [] }; + mocks.getExplainInfo.mockResolvedValue("Plan hash value: 123"); + mocks.parseOracleExplainText.mockReturnValue(parsedPlan); + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query"); + + await store.explainTabSql(tabId, "SELECT * FROM DUAL", "oracle"); + + expect(mocks.parseOracleExplainText).toHaveBeenCalledWith("Plan hash value: 123"); + expect(store.tabs.find((tab) => tab.id === tabId)).toMatchObject({ + explainPlan: parsedPlan, + explainSql: "EXPLAIN PLAN FOR SELECT * FROM DUAL", + explainError: undefined, + }); + }); +}); diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index a9f568fe3e..2bb665b42e 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -57,6 +57,7 @@ import { useSettingsStore } from "@/stores/settingsStore"; import { useSavedSqlStore } from "@/stores/savedSqlStore"; import { recordQueryCancellationLatency, resourceLifecycleDiagnostics } from "@/lib/diagnostics/resourceLifecycleDiagnostics"; import { appendDebugLog } from "@/lib/backend/debugLog"; +import { formatError } from "@/lib/backend/errorUtils"; import { createSavedSqlEditorPosition, initSavedSqlEditorPositions, restoreSavedSqlEditorPosition, saveSavedSqlEditorPosition } from "@/lib/app/savedSqlEditorPosition"; import { ensureSqlExtension } from "@/lib/savedSql/savedSqlFileName"; import { safeLocalStorageGet, safeLocalStorageRemove } from "@/lib/backend/safeStorage"; @@ -3598,7 +3599,8 @@ export const useQueryStore = defineStore("query", () => { const current = tabs.value.find((t) => t.id === id); if (current?.explainExecutionId === executionId) { current.explainPlan = undefined; - current.explainError = String(e?.message || e); + // Backend rejections contain the real ORA/Agent diagnostic; only successful empty responses use the generic empty-plan message. + current.explainError = formatError(e); } } finally { const current = tabs.value.find((t) => t.id === id);