From 716d2eafd2a19179f17632a3131c471faefd2b0d Mon Sep 17 00:00:00 2001 From: zipg Date: Wed, 2 Sep 2026 11:20:20 +0800 Subject: [PATCH] fix(query): fall back to autocommit for legacy oracle agents --- apps/desktop/src/lib/backend/errorUtils.ts | 7 + ...queryStore.manualTransactionExpiry.spec.ts | 39 +++++ apps/desktop/src/stores/queryStore.ts | 134 ++++++++++-------- 3 files changed, 124 insertions(+), 56 deletions(-) diff --git a/apps/desktop/src/lib/backend/errorUtils.ts b/apps/desktop/src/lib/backend/errorUtils.ts index 409a1adfd7..84faad60e1 100644 --- a/apps/desktop/src/lib/backend/errorUtils.ts +++ b/apps/desktop/src/lib/backend/errorUtils.ts @@ -92,6 +92,13 @@ export function isManualTransactionSessionExpired(error: unknown): boolean { return message?.startsWith("Transaction session not found or expired;") === true || message === "Transaction was auto-rolled back due to 5 minutes of inactivity"; } +export function isUnsupportedManualTransactionMethod(error: unknown): boolean { + const message = error instanceof Error ? error.message : typeof error === "string" ? error : undefined; + if (!message) return false; + const normalized = message.toLowerCase(); + return normalized.includes("begin_manual_transaction") && (normalized.includes("unknown method") || normalized.includes("method not found")); +} + function normalizeBackendErrorAtDepth(error: unknown, seen: WeakSet, depth: number): BackendError | null { if (depth > MAX_ERROR_PARSE_DEPTH) return null; diff --git a/apps/desktop/src/stores/__tests__/queryStore.manualTransactionExpiry.spec.ts b/apps/desktop/src/stores/__tests__/queryStore.manualTransactionExpiry.spec.ts index 76ed58e01c..438aded0b0 100644 --- a/apps/desktop/src/stores/__tests__/queryStore.manualTransactionExpiry.spec.ts +++ b/apps/desktop/src/stores/__tests__/queryStore.manualTransactionExpiry.spec.ts @@ -172,6 +172,45 @@ describe("queryStore manual transaction expiry recovery", () => { expect(tab.result?.execution_error).toBe(true); }); + it("falls back to auto-commit for a read query when an old Oracle Agent lacks manual transactions", async () => { + mocks.beginManualTransaction.mockRejectedValue(new Error("BEGIN manual transaction failed: Agent RPC error (-1): unknown method: begin_manual_transaction")); + mocks.executeMulti.mockResolvedValue(successfulSelect()); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query", "query", "APP"); + store.setAutoCommit(tabId, false); + + await store.executeTabSql(tabId, "SELECT 1 FROM DUAL"); + + const tab = store.tabs.find((item) => item.id === tabId)!; + expect(tab.autoCommit).toBe(true); + expect(tab.txnSessionId).toBeUndefined(); + expect(mocks.beginManualTransaction).toHaveBeenCalledOnce(); + expect(mocks.executeInManualTransaction).not.toHaveBeenCalled(); + expect(mocks.executeMulti).toHaveBeenCalledOnce(); + expect(tab.result?.rows).toEqual([[1]]); + }); + + it("does not replay a write when an old Oracle Agent lacks manual transactions", async () => { + mocks.beginManualTransaction.mockRejectedValue(new Error("BEGIN manual transaction failed: Agent RPC error (-1): Method not found: begin_manual_transaction")); + mocks.executeMulti.mockResolvedValue(successfulUpdate()); + + const { useQueryStore } = await import("@/stores/queryStore"); + const store = useQueryStore(); + const tabId = store.createTab("oracle-1", "ORCL", "Query", "query", "APP"); + store.setAutoCommit(tabId, false); + + await store.executeTabSql(tabId, "UPDATE USERS SET ACTIVE = 1"); + + const tab = store.tabs.find((item) => item.id === tabId)!; + expect(tab.autoCommit).toBe(false); + expect(mocks.beginManualTransaction).toHaveBeenCalledOnce(); + expect(mocks.executeInManualTransaction).not.toHaveBeenCalled(); + expect(mocks.executeMulti).not.toHaveBeenCalled(); + expect(tab.result?.execution_error).toBe(true); + }); + it("keeps raw JDBC connections in manual mode when their effective dialect is unsupported", async () => { mocks.getConnectionConfig.mockReturnValue({ id: "jdbc-1", diff --git a/apps/desktop/src/stores/queryStore.ts b/apps/desktop/src/stores/queryStore.ts index a286bd5761..74e0f214b6 100644 --- a/apps/desktop/src/stores/queryStore.ts +++ b/apps/desktop/src/stores/queryStore.ts @@ -56,6 +56,7 @@ import { normalizeResultPageSize } from "@/lib/dataGrid/paginationPageSize"; import { agentProtocolQueryResultMaxRows, capQueryResultTotal, effectiveQueryResultMaxRows, limitQueryPagination, queryResultLimitReached } from "@/lib/dataGrid/queryResultRowLimit"; import { elasticsearchRestRequestRanges, executableStatementRanges, splitSqlStatementRanges } from "@/lib/sql/sqlStatementRanges"; import { replaceSqlServerLeadingUseQuery, sqlServerLeadingUseScript, sqlServerUseDatabaseFromStatement } from "@/lib/sql/sqlCompletionLookupTarget"; +import { classifySqlRisk } from "@/lib/sql/sqlRisk"; import { externalSqlFileDisplayTitles, normalizeExternalSqlPath } from "@/lib/sql/sqlFileOpen"; import { clearDataGridPendingSnapshot, clearDataGridPendingSnapshotsForTab } from "@/composables/useDataGridEditor"; import { clearDataGridStructuredFilterStatesForTab } from "@/lib/dataGrid/dataGridFilterBuilderPersistence"; @@ -72,7 +73,7 @@ import { useSavedSqlStore } from "@/stores/savedSqlStore"; import { useExportTracker } from "@/composables/useExportTracker"; import { recordQueryCancellationLatency, resourceLifecycleDiagnostics } from "@/lib/diagnostics/resourceLifecycleDiagnostics"; import { appendDebugLog } from "@/lib/backend/debugLog"; -import { BackendErrorException, formatError, isManualTransactionSessionExpired, normalizeBackendError, type BackendError } from "@/lib/backend/errorUtils"; +import { BackendErrorException, formatError, isManualTransactionSessionExpired, isUnsupportedManualTransactionMethod, normalizeBackendError, type BackendError } from "@/lib/backend/errorUtils"; import { createSavedSqlEditorPosition, initSavedSqlEditorPositions, restoreSavedSqlEditorPosition, saveSavedSqlEditorPosition } from "@/lib/app/savedSqlEditorPosition"; import { ensureSqlExtension } from "@/lib/savedSql/savedSqlFileName"; import { resolveSavedSqlExecutionTarget, savedSqlExecutionTargetFromTab, type SavedSqlExecutionTarget, type SavedSqlOpenTargetMode } from "@/lib/savedSql/savedSqlExecutionTarget"; @@ -5408,45 +5409,7 @@ export const useQueryStore = defineStore("query", () => { return false; } - let executionPromise: Promise; - if (tab.autoCommit === false) { - if (!tab.txnSessionId) { - queryExecutionLog("info", "begin-manual-txn:start", { traceId, elapsed: elapsed() }); - tab.txnSessionId = await api.beginManualTransaction(executionConnectionId, executionDatabase, executionSchema, executionCatalog); - queryExecutionLog("info", "begin-manual-txn:done", { traceId, txnSessionId: tab.txnSessionId, elapsed: elapsed() }); - } - queryExecutionLog("info", "execute-in-txn:invoke", { traceId, txnSessionId: tab.txnSessionId, elapsed: elapsed() }); - executionDispatched = true; - // Only an initial manual execution classifies the user SQL (Oracle-only). - // A later cursor-page fetch must neither set nor clear the sticky bit. - const isInitialOracleManualExecution = effectiveDbType === "oracle" && !options?.pagination?.sessionId; - const classificationSql = isInitialOracleManualExecution ? queryBaseSql : undefined; - let manualTransactionRecoveryAttempted = false; - executionPromise = (async () => { - const txnSessionId = tab.txnSessionId; - if (!txnSessionId) throw new Error("Manual transaction session was not initialized"); - const executeInTransaction = (sessionId: string) => - useAgentResultSession - ? api.executeInManualTransaction(sessionId, sqlToExecute, executionDatabase, executionSchema, agentProtocolQueryResultMaxRows(queryResultMaxRows), useOracleLobPreview, pageLimit, options?.pagination?.sessionId, classificationSql) - : api.executeInManualTransaction(sessionId, sqlToExecute, executionDatabase, executionSchema, pageLimit ?? agentProtocolQueryResultMaxRows(queryResultMaxRows), useOracleLobPreview, undefined, undefined, classificationSql); - try { - return await executeInTransaction(txnSessionId); - } catch (error) { - if (options?.pagination?.sessionId || manualTransactionRecoveryAttempted || !isManualTransactionSessionExpired(error)) throw error; - manualTransactionRecoveryAttempted = true; - // The expired session was discarded by the backend; the replacement - // session starts fresh, so the old sticky state resets with it. - clearOracleTxnPossiblyDirty(tab); - tab.txnSessionId = undefined; - tab.txnAutoRolledBack = true; - queryExecutionLog("info", "manual-txn:expired-recover", { traceId, elapsed: elapsed() }); - const refreshedSessionId = await api.beginManualTransaction(executionConnectionId, executionDatabase, executionSchema, executionCatalog); - tab.txnSessionId = refreshedSessionId; - queryExecutionLog("info", "manual-txn:restarted", { traceId, txnSessionId: refreshedSessionId, elapsed: elapsed() }); - return executeInTransaction(refreshedSessionId); - } - })(); - } else { + const executeWithoutManualTransaction = (): Promise => { queryExecutionLog("info", "execute-multi:start", { traceId, elapsed: elapsed() }); // Query and data tabs use a tab-scoped pool so repeated executions keep // connection-local state and avoid MySQL pool resets on every refresh. @@ -5487,22 +5450,81 @@ export const useQueryStore = defineStore("query", () => { clientSession: Boolean(executionClientSessionId), }); executionDispatched = true; - executionPromise = - tab.batchSqlExecution && tab.batchSqlExecution.total > 1 - ? api.executeMultiWithProgress( - executionConnectionId, - executionDatabase, - sqlToExecute, - (progress) => { - const current = findExecutionTab(id); - if (current?.executionId === executionId) { - applyBatchSqlProgress(current, progress, continueOnBatchError, batchResume?.startStatementIndex ?? 0); - } - }, - executionSchema, - { ...executionOptions, executionId }, - ) - : api.executeMulti(executionConnectionId, executionDatabase, sqlToExecute, executionSchema, executionId, executionOptions); + return tab.batchSqlExecution && tab.batchSqlExecution.total > 1 + ? api.executeMultiWithProgress( + executionConnectionId, + executionDatabase, + sqlToExecute, + (progress) => { + const current = findExecutionTab(id); + if (current?.executionId === executionId) { + applyBatchSqlProgress(current, progress, continueOnBatchError, batchResume?.startStatementIndex ?? 0); + } + }, + executionSchema, + { ...executionOptions, executionId }, + ) + : api.executeMulti(executionConnectionId, executionDatabase, sqlToExecute, executionSchema, executionId, executionOptions); + }; + + let executionPromise: Promise; + if (tab.autoCommit === false) { + let useLegacyReadFallback = false; + if (!tab.txnSessionId) { + queryExecutionLog("info", "begin-manual-txn:start", { traceId, elapsed: elapsed() }); + try { + tab.txnSessionId = await api.beginManualTransaction(executionConnectionId, executionDatabase, executionSchema, executionCatalog); + queryExecutionLog("info", "begin-manual-txn:done", { traceId, txnSessionId: tab.txnSessionId, elapsed: elapsed() }); + } catch (error) { + const risk = classifySqlRisk(sqlToExecute, { dialect: effectiveDbType }).risk; + if (!isUnsupportedManualTransactionMethod(error) || risk !== "read") throw error; + tab.autoCommit = true; + clearManualTransactionSession(tab); + useLegacyReadFallback = true; + queryExecutionLog("warn", "begin-manual-txn:legacy-read-fallback", { + traceId, + elapsed: elapsed(), + databaseType: effectiveDbType, + }); + } + } + if (useLegacyReadFallback) { + executionPromise = executeWithoutManualTransaction(); + } else { + queryExecutionLog("info", "execute-in-txn:invoke", { traceId, txnSessionId: tab.txnSessionId, elapsed: elapsed() }); + executionDispatched = true; + // Only an initial manual execution classifies the user SQL (Oracle-only). + // A later cursor-page fetch must neither set nor clear the sticky bit. + const isInitialOracleManualExecution = effectiveDbType === "oracle" && !options?.pagination?.sessionId; + const classificationSql = isInitialOracleManualExecution ? queryBaseSql : undefined; + let manualTransactionRecoveryAttempted = false; + executionPromise = (async () => { + const txnSessionId = tab.txnSessionId; + if (!txnSessionId) throw new Error("Manual transaction session was not initialized"); + const executeInTransaction = (sessionId: string) => + useAgentResultSession + ? api.executeInManualTransaction(sessionId, sqlToExecute, executionDatabase, executionSchema, agentProtocolQueryResultMaxRows(queryResultMaxRows), useOracleLobPreview, pageLimit, options?.pagination?.sessionId, classificationSql) + : api.executeInManualTransaction(sessionId, sqlToExecute, executionDatabase, executionSchema, pageLimit ?? agentProtocolQueryResultMaxRows(queryResultMaxRows), useOracleLobPreview, undefined, undefined, classificationSql); + try { + return await executeInTransaction(txnSessionId); + } catch (error) { + if (options?.pagination?.sessionId || manualTransactionRecoveryAttempted || !isManualTransactionSessionExpired(error)) throw error; + manualTransactionRecoveryAttempted = true; + // The expired session was discarded by the backend; the replacement + // session starts fresh, so the old sticky state resets with it. + clearOracleTxnPossiblyDirty(tab); + tab.txnSessionId = undefined; + tab.txnAutoRolledBack = true; + queryExecutionLog("info", "manual-txn:expired-recover", { traceId, elapsed: elapsed() }); + const refreshedSessionId = await api.beginManualTransaction(executionConnectionId, executionDatabase, executionSchema, executionCatalog); + tab.txnSessionId = refreshedSessionId; + queryExecutionLog("info", "manual-txn:restarted", { traceId, txnSessionId: refreshedSessionId, elapsed: elapsed() }); + return executeInTransaction(refreshedSessionId); + } + })(); + } + } else { + executionPromise = executeWithoutManualTransaction(); } const annotatedResults = annotateQueryResultSources(markQueryResultsRowsRaw(await withFrontendQueryTimeout(executionPromise, frontendTimeoutSecs, t("editor.queryTimeoutError", { seconds: frontendTimeoutSecs }))), queryBaseSql, sourceLabelDatabase, effectiveDbType, options?.sourceOffset); const results = offsetBatchQueryResultIndexes(annotatedResults.results, batchResume?.startStatementIndex ?? 0);