From b4015a439317621fb1a844c229141d0d8cf9a6cf Mon Sep 17 00:00:00 2001 From: Melody Ruch <396921921@qq.com> Date: Thu, 20 Aug 2026 22:23:37 +0800 Subject: [PATCH] fix(sidebar): make destructive table operation cancellation reliable --- .../src/components/sidebar/ConnectionTree.vue | 4 + .../sidebar/SidebarTreeRuntimeHost.vue | 17 +- .../ConnectionTree.dangerDialogGuard.spec.ts | 10 + ...arTreeRuntimeHost.executionIdOptIn.spec.ts | 11 + .../__tests__/sidebarTreeDialogState.spec.ts | 14 + .../sidebar/sidebarTreeDialogState.ts | 2 + ...idebarTableMutationRuntime.saphana.spec.ts | 87 +++++++ .../useSidebarTableMutationRuntime.ts | 244 ++++++++++++------ apps/desktop/src/i18n/locales/en.ts | 2 + apps/desktop/src/i18n/locales/es.ts | 2 + apps/desktop/src/i18n/locales/it.ts | 2 + apps/desktop/src/i18n/locales/ja.ts | 2 + apps/desktop/src/i18n/locales/ko.ts | 2 + apps/desktop/src/i18n/locales/pt-BR.ts | 2 + apps/desktop/src/i18n/locales/zh-CN.ts | 2 + apps/desktop/src/i18n/locales/zh-TW.ts | 2 + crates/dbx-core/src/query_cancel.rs | 170 +++++++++--- crates/dbx-web/src/routes/query.rs | 22 +- .../app-tests/sidebarMutationRuntime.test.ts | 8 +- src-tauri/src/commands/query.rs | 16 +- 20 files changed, 481 insertions(+), 140 deletions(-) create mode 100644 apps/desktop/src/components/sidebar/__tests__/ConnectionTree.dangerDialogGuard.spec.ts create mode 100644 apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.executionIdOptIn.spec.ts create mode 100644 apps/desktop/src/components/sidebar/__tests__/sidebarTreeDialogState.spec.ts diff --git a/apps/desktop/src/components/sidebar/ConnectionTree.vue b/apps/desktop/src/components/sidebar/ConnectionTree.vue index 6a2de7f653..25cf0a9007 100644 --- a/apps/desktop/src/components/sidebar/ConnectionTree.vue +++ b/apps/desktop/src/components/sidebar/ConnectionTree.vue @@ -1594,6 +1594,10 @@ function openSidebarContextMenu(event: MouseEvent, node: TreeNode, openContextMe } function openSidebarDangerDialog(request: SidebarDangerDialogRequest) { + if (sidebarDangerRunningExecutionId.value) { + toast(t("contextMenu.dangerOperationAlreadyRunning"), 4000); + return; + } sidebarDangerDialogRequest.value = request; sidebarDangerDialogConfirming.value = false; // Defense in depth: sidebarDangerDialogCancelling is a singleton shared diff --git a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue index 4a34f7de90..e73a81577a 100644 --- a/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue +++ b/apps/desktop/src/components/sidebar/SidebarTreeRuntimeHost.vue @@ -70,7 +70,6 @@ import { useDatabaseOptions } from "@/composables/useDatabaseOptions"; import type { ColumnInfo, DatabaseType, TreeNode, TreeNodeType } from "@/types/database"; import * as api from "@/lib/backend/api"; import { queryTimeoutSecsForConnection } from "@/lib/sql/queryTimeout"; -import { uuid } from "@/lib/common/utils"; import { resolveDefaultDatabase } from "@/lib/database/defaultDatabase"; import { connectionUsesVisibleSchemaFilter } from "@/lib/database/visibleDatabases"; import { canTreeNodePin, canTreeNodeShowExpander } from "@/lib/sidebar/sidebarTreeItemLayout"; @@ -2524,13 +2523,25 @@ function openRenameObjectDialog() { async function executeTreeNodeSqlWithProductionGuard( node: Pick, sql: string, - options: { database?: string; schema?: string; executeAsScript?: boolean; executionId?: string; isCancelledBeforeDispatch?: () => boolean; markDispatched?: () => void } = {}, + options: { + database?: string; + schema?: string; + executeAsScript?: boolean; + executionId?: string; + isCancelledBeforeDispatch?: () => boolean; + markDispatched?: () => void; + } = {}, ) { if (!node.connectionId) return undefined; const database = options.database ?? node.database ?? ""; const config = connectionStore.getConfig(node.connectionId); + // Always resolve the connection's configured timeout the same way the + // main editor does — independent of whether this call also wires up + // Cancel-button UI. Gating this behind an opt-in flag previously left + // every non-danger-dialog caller falling back to the backend's hardcoded + // 30s default instead of the user's configured (or unlimited) timeout. const timeoutSecs = queryTimeoutSecsForConnection(config, settingsStore.editorSettings.globalQueryTimeoutSecs); - const executionId = options.executionId ?? uuid(); + const executionId = options.executionId; return executeWithProductionSqlGuard({ connection: config, database, diff --git a/apps/desktop/src/components/sidebar/__tests__/ConnectionTree.dangerDialogGuard.spec.ts b/apps/desktop/src/components/sidebar/__tests__/ConnectionTree.dangerDialogGuard.spec.ts new file mode 100644 index 0000000000..5149f1d89d --- /dev/null +++ b/apps/desktop/src/components/sidebar/__tests__/ConnectionTree.dangerDialogGuard.spec.ts @@ -0,0 +1,10 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const connectionTreeSource = readFileSync(new URL("../ConnectionTree.vue", import.meta.url), "utf8"); + +describe("ConnectionTree danger dialog guard", () => { + it("refuses to open a new danger dialog while a previous danger operation is still running", () => { + expect(connectionTreeSource).toMatch(/function openSidebarDangerDialog\(request: SidebarDangerDialogRequest\) \{\s*if \(sidebarDangerRunningExecutionId\.value\) \{\s*toast\(t\("contextMenu\.dangerOperationAlreadyRunning"\), \d+\);\s*return;\s*\}/); + }); +}); diff --git a/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.executionIdOptIn.spec.ts b/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.executionIdOptIn.spec.ts new file mode 100644 index 0000000000..e5b53d51a6 --- /dev/null +++ b/apps/desktop/src/components/sidebar/__tests__/SidebarTreeRuntimeHost.executionIdOptIn.spec.ts @@ -0,0 +1,11 @@ +import { readFileSync } from "node:fs"; +import { describe, expect, it } from "vitest"; + +const source = readFileSync(new URL("../SidebarTreeRuntimeHost.vue", import.meta.url), "utf8"); + +describe("executeTreeNodeSqlWithProductionGuard executionId", () => { + it("only registers a backend execution id for callers that opt in, instead of generating one for every caller", () => { + expect(source).toContain("const executionId = options.executionId;"); + expect(source).not.toMatch(/executionId\s*=\s*options\.executionId\s*\?\?\s*uuid\(\)/); + }); +}); diff --git a/apps/desktop/src/components/sidebar/__tests__/sidebarTreeDialogState.spec.ts b/apps/desktop/src/components/sidebar/__tests__/sidebarTreeDialogState.spec.ts new file mode 100644 index 0000000000..fe86ec73d8 --- /dev/null +++ b/apps/desktop/src/components/sidebar/__tests__/sidebarTreeDialogState.spec.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from "vitest"; +import { resetSidebarTreeDialogState, sidebarDangerRunningCancel, sidebarDangerRunningExecutionId } from "../sidebarTreeDialogState"; + +describe("resetSidebarTreeDialogState", () => { + it("clears the danger-running singleton", () => { + sidebarDangerRunningExecutionId.value = "exec-1"; + sidebarDangerRunningCancel.value = () => {}; + + resetSidebarTreeDialogState(); + + expect(sidebarDangerRunningExecutionId.value).toBe(""); + expect(sidebarDangerRunningCancel.value).toBeNull(); + }); +}); diff --git a/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts b/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts index 774a23c5d5..99d0e70b98 100644 --- a/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts +++ b/apps/desktop/src/components/sidebar/sidebarTreeDialogState.ts @@ -241,6 +241,8 @@ export function resetSidebarTreeDialogState() { sidebarTreeDialogOwner.value = null; sidebarDangerTarget.value = null; sidebarFormTarget.value = null; + sidebarDangerRunningExecutionId.value = ""; + sidebarDangerRunningCancel.value = null; connectionDeleteTargetSnapshot.value = []; connectionGroupDeleteTargetSnapshot.value = []; deleteConnectionsWithGroup.value = false; diff --git a/apps/desktop/src/composables/__tests__/useSidebarTableMutationRuntime.saphana.spec.ts b/apps/desktop/src/composables/__tests__/useSidebarTableMutationRuntime.saphana.spec.ts index 7cde3cf1e3..22cb947110 100644 --- a/apps/desktop/src/composables/__tests__/useSidebarTableMutationRuntime.saphana.spec.ts +++ b/apps/desktop/src/composables/__tests__/useSidebarTableMutationRuntime.saphana.spec.ts @@ -259,6 +259,40 @@ describe("useSidebarTableMutationRuntime SAP HANA schema-scoped actions", () => expect(sidebarDangerRunningCancel.value).toBeNull(); }); + it("waits for an in-flight cancel confirmation before classifying a cancelled execution", async () => { + const { feature } = runtime(""); + let signalCancelStarted: () => void = () => {}; + let resolveCancel: (confirmed: boolean) => void = () => {}; + const cancelStarted = new Promise((resolve) => { + signalCancelStarted = resolve; + }); + const cancelResult = new Promise((resolve) => { + resolveCancel = resolve; + }); + mocks.cancelQuery.mockImplementationOnce(() => { + signalCancelStarted(); + return cancelResult; + }); + mocks.executeWithProductionGuard.mockImplementationOnce(async (_node: unknown, _sql: unknown, options: { markDispatched: () => void }) => { + options.markDispatched(); + void sidebarDangerRunningCancel.value?.(); + throw new Error("Query cancelled"); + }); + + const operationPromise = feature.confirmEmptyTable(); + await cancelStarted; + + expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationCancelUnconfirmed", 8000); + resolveCancel(true); + await operationPromise; + + expect(mocks.toast).toHaveBeenCalledWith("contextMenu.tableOperationCancelled", 3000); + expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationCancelUnconfirmed", 8000); + expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationCancelPending", 6000); + expect(sidebarDangerRunningExecutionId.value).toBe(""); + expect(sidebarDangerRunningCancel.value).toBeNull(); + }); + it("never dispatches the SQL when the user cancels before the operation is sent to the database (registration race)", async () => { const { feature } = runtime(""); mocks.ensureConnected.mockImplementationOnce(async () => { @@ -291,6 +325,10 @@ describe("useSidebarTableMutationRuntime SAP HANA schema-scoped actions", () => // Bounded retry, not a single silently-ignored attempt. expect(mocks.cancelQuery.mock.calls.length).toBeGreaterThan(1); + // The cancel handler itself surfaces feedback the moment its own + // confirmation window gives up (before the underlying operation later + // settles and reports tableOperationCancelUnconfirmed). + expect(mocks.toast).toHaveBeenCalledWith("contextMenu.tableOperationCancelPending", 6000); expect(mocks.toast).toHaveBeenCalledWith("contextMenu.tableOperationCancelUnconfirmed", 8000); expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationCancelled", 3000); expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationFailed", 5000); @@ -340,10 +378,59 @@ describe("useSidebarTableMutationRuntime SAP HANA schema-scoped actions", () => vi.useRealTimers(); } + // A single hung attempt must not block the remaining retries — each + // attempt gets its own bounded slice of the overall retry budget. + expect(mocks.cancelQuery.mock.calls.length).toBeGreaterThan(1); // The closure above must have settled instead of hanging — a stuck // cancel handle would otherwise permanently disable the shared // sidebarDangerDialogCancelling state for every future danger dialog. expect(mocks.toast).not.toHaveBeenCalledWith("contextMenu.tableOperationCancelled", 3000); + // Settling silently is not enough on its own: the user must be told the + // cancel could not be confirmed instead of the button just going quiet. + expect(mocks.toast).toHaveBeenCalledWith("contextMenu.tableOperationCancelPending", 6000); + // The operation may genuinely still be running server-side, so the + // running-execution state must not be torn down just because the + // confirmation window elapsed. + expect(sidebarDangerRunningExecutionId.value).not.toBe(""); + }); + + it("confirms the cancel when the first backend response arrives after its own per-attempt timeout", async () => { + const { feature } = runtime(""); + mocks.executeWithProductionGuard.mockImplementationOnce(async (_node: unknown, _sql: unknown, options: { markDispatched: () => void }) => { + options.markDispatched(); + throw new Error("Query timed out after 60 seconds"); + }); + + await feature.confirmEmptyTable(); + mocks.toast.mockClear(); + + vi.useFakeTimers(); + try { + let resolveFirstAttempt: (confirmed: boolean) => void = () => {}; + const firstAttempt = new Promise((resolve) => { + resolveFirstAttempt = resolve; + }); + mocks.cancelQuery.mockImplementationOnce(() => firstAttempt); + // By the time a retry is fired, the backend has already removed the + // execution id (the first attempt is the one that actually cancelled + // it), so any further attempt can only ever answer false. + mocks.cancelQuery.mockResolvedValue(false); + + const cancelPromise = sidebarDangerRunningCancel.value?.(); + // Let the first attempt's own 2s per-attempt timeout elapse without an + // answer, so the loop moves on to firing a retry. + await vi.advanceTimersByTimeAsync(2_000); + // The first attempt now succeeds, 0.5s after its own timeout fired — + // it must still be observed instead of being discarded. + resolveFirstAttempt(true); + await vi.advanceTimersByTimeAsync(500); + await cancelPromise; + } finally { + vi.useRealTimers(); + } + + expect(mocks.toast).toHaveBeenCalledWith("contextMenu.tableOperationCancelled", 3000); + expect(sidebarDangerRunningExecutionId.value).toBe(""); }); it.each(actions)("does not report a false success for $action when the user declines the production-safety confirmation", async ({ action }) => { diff --git a/apps/desktop/src/composables/useSidebarTableMutationRuntime.ts b/apps/desktop/src/composables/useSidebarTableMutationRuntime.ts index 95eed6806f..739fa575fe 100644 --- a/apps/desktop/src/composables/useSidebarTableMutationRuntime.ts +++ b/apps/desktop/src/composables/useSidebarTableMutationRuntime.ts @@ -258,25 +258,74 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt // danger dialog's Cancel Query button (sidebarDangerDialogCancelling is a // shared singleton — see ConnectionTree.vue). const CANCEL_QUERY_OVERALL_TIMEOUT_MS = 10_000; + // Each attempt gets an even share of the overall budget, so a single hung + // api.cancelQuery call can't block the remaining retries — it can only + // ever stop *waiting* on that attempt, not actually abort it (neither + // backend transport plumbs an AbortSignal through cancelQuery). + const CANCEL_QUERY_PER_ATTEMPT_TIMEOUT_MS = Math.floor(CANCEL_QUERY_OVERALL_TIMEOUT_MS / CANCEL_QUERY_MAX_ATTEMPTS); async function confirmCancelWithRetry(executionId: string): Promise { - for (let attempt = 0; attempt < CANCEL_QUERY_MAX_ATTEMPTS; attempt++) { - let confirmed = false; - try { - confirmed = await api.cancelQuery(executionId); - } catch { - confirmed = false; - } - if (confirmed) return true; + // Once an attempt's own per-attempt timeout elapses, the loop below + // moves on to firing the next attempt without waiting for it — but it + // must keep observing that attempt's eventual result instead of just + // dropping the promise. A retry fired after the timeout hits the + // backend *after* it already removed the execution id on a first + // attempt that merely answered late, so every subsequent retry can only + // ever return false; discarding the first attempt's own (eventually + // `true`) result would make the whole call report an unconfirmed + // cancel even though the backend did cancel it. + let confirmed = false; + let settledAttempts = 0; + let totalAttempts = 0; + let notifyNextSettle: (() => void) | undefined; + + function trackAttempt(attemptPromise: Promise) { + totalAttempts += 1; + void attemptPromise.then((result) => { + settledAttempts += 1; + if (result) confirmed = true; + notifyNextSettle?.(); + }); } - return false; + + for (let attempt = 0; attempt < CANCEL_QUERY_MAX_ATTEMPTS && !confirmed; attempt++) { + trackAttempt(api.cancelQuery(executionId).catch(() => false)); + await new Promise((resolve) => { + const timeoutId = setTimeout(resolve, CANCEL_QUERY_PER_ATTEMPT_TIMEOUT_MS); + notifyNextSettle = () => { + clearTimeout(timeoutId); + resolve(); + }; + }); + } + + // All attempts have been fired; wait for any still-outstanding ones so + // a slow-but-successful cancel is not lost just because it settled + // after its own attempt's pacing timeout. + while (!confirmed && settledAttempts < totalAttempts) { + await new Promise((resolve) => { + notifyNextSettle = resolve; + }); + } + + return confirmed; } - function confirmCancelWithRetryAndTimeout(executionId: string): Promise { + // Races the retry loop against a UI-facing deadline, but also returns the + // retry loop's own promise so a caller can keep observing it after the + // race settles — otherwise a retry that is still running when the timeout + // wins would have its eventual result silently discarded. + function confirmCancelWithRetryAndTimeout(executionId: string): { confirmedWithinTimeout: Promise; retryPromise: Promise } { + const retryPromise = confirmCancelWithRetry(executionId); + let timeoutId: ReturnType; const timeout = new Promise((resolve) => { - setTimeout(() => resolve(false), CANCEL_QUERY_OVERALL_TIMEOUT_MS); + timeoutId = setTimeout(() => resolve(false), CANCEL_QUERY_OVERALL_TIMEOUT_MS); }); - return Promise.race([confirmCancelWithRetry(executionId), timeout]); + // Once the retry loop itself settles, the timeout has nothing left to + // race against — clear it instead of leaving it to fire (and resolve an + // already-settled promise) up to CANCEL_QUERY_OVERALL_TIMEOUT_MS later. + void retryPromise.finally(() => clearTimeout(timeoutId)); + return { confirmedWithinTimeout: Promise.race([retryPromise, timeout]), retryPromise }; } const DANGER_OPERATION_CANCELLED_BEFORE_DISPATCH_MESSAGE = "Operation cancelled before it was sent to the database."; @@ -287,6 +336,7 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt markDispatched: () => void; wasCancelled: () => boolean; cancelConfirmed: () => boolean; + waitForCancelConfirmation: () => Promise; markHandedOff: () => void; } @@ -295,22 +345,70 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt let dispatched = false; let cancelledByUser = false; let cancelConfirmedFlag = false; + let cancelConfirmationPromise: Promise | null = null; // Set once the owning confirm*Table() call has already given up waiting // (a client-observed timeout) and deferred final cleanup to a later // confirmed cancel — see markHandedOff below. let handedOff = false; + // Shared by both the immediate (raced) outcome and a later-arriving + // retry result so a confirmed cancel is finalized exactly once, however + // late it arrives. + function finalizeConfirmedCancel() { + cancelConfirmedFlag = true; + if (handedOff && sidebarDangerRunningExecutionId.value === executionId) { + toast(t("contextMenu.tableOperationCancelled", { name: nodeLabel }), 3000); + endDangerRunningExecution(); + } + } + sidebarDangerRunningExecutionId.value = executionId; sidebarDangerRunningCancel.value = async () => { cancelledByUser = true; // Nothing has reached the backend yet: the pending dispatch will see // isCancelledBeforeDispatch() and skip the network call entirely, so // this is already a confirmed cancellation. - const confirmed = dispatched ? await confirmCancelWithRetryAndTimeout(executionId) : true; - if (confirmed) cancelConfirmedFlag = true; - if (handedOff && confirmed && sidebarDangerRunningExecutionId.value === executionId) { - toast(t("contextMenu.tableOperationCancelled", { name: nodeLabel }), 3000); - endDangerRunningExecution(); + if (!dispatched) { + finalizeConfirmedCancel(); + return; + } + if (!cancelConfirmationPromise) { + cancelConfirmationPromise = (async () => { + const { confirmedWithinTimeout, retryPromise } = confirmCancelWithRetryAndTimeout(executionId); + // Do not discard the losing side of the race: if the retry loop is + // still going when the UI timeout wins below and it later succeeds, + // still finalize the confirmed cancel instead of dropping the result. + void retryPromise.then((eventuallyConfirmed) => { + if (eventuallyConfirmed) finalizeConfirmedCancel(); + }); + const confirmed = await confirmedWithinTimeout; + if (confirmed) { + finalizeConfirmedCancel(); + } else if (sidebarDangerRunningExecutionId.value === executionId) { + // The confirmation window elapsed with no answer from the database. + // The operation may genuinely still be running server-side, so leave + // sidebarDangerRunningExecutionId / the running-execution state + // intact — the user can retry Cancel or just wait for it to settle + // — but don't leave the Cancel button silently usable again with + // zero explanation of what happened. + // + // Only surface this if the execution is still the one being tracked: + // by the time this arrives, confirmDropTable/confirmEmptyTable/ + // confirmTruncateTable may have already resolved (success or an + // unrelated failure) and moved on, in which case this stale warning + // would contradict the outcome the user already saw. + toast(t("contextMenu.tableOperationCancelPending", { name: nodeLabel }), 6000); + } + return confirmed; + })(); + } + const currentConfirmation = cancelConfirmationPromise; + try { + await currentConfirmation; + } finally { + if (cancelConfirmationPromise === currentConfirmation) { + cancelConfirmationPromise = null; + } } }; @@ -322,6 +420,7 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt }, wasCancelled: () => cancelledByUser, cancelConfirmed: () => cancelConfirmedFlag, + waitForCancelConfirmation: () => cancelConfirmationPromise ?? Promise.resolve(cancelConfirmedFlag), markHandedOff: () => { handedOff = true; }, @@ -345,31 +444,36 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt } } - async function confirmDropTable() { - const node = sidebarDangerTarget.value ?? activeNode.value; + interface DangerOperationConfig { + node: TreeNode; + buildSql: () => string | Promise; + onSuccess: (node: TreeNode) => void | Promise; + } + + async function runDangerOperation(config: DangerOperationConfig) { + const { node, buildSql, onSuccess } = config; if (!node.connectionId || node.database == null) return; - const { executionId, isCancelledBeforeDispatch, markDispatched, wasCancelled, cancelConfirmed, markHandedOff } = beginDangerRunningExecution(node.label); + const { executionId, isCancelledBeforeDispatch, markDispatched, wasCancelled, cancelConfirmed, waitForCancelConfirmation, markHandedOff } = beginDangerRunningExecution(node.label); try { await connectionStore.ensureConnected(node.connectionId); - const sql = dropTablePreviewSql.value || (await buildDropTableSql(tableAdminSqlOptionsForNode(node, { cascade: dropTableCascade.value && supportsDropTableCascade(databaseTypeForNode(node)) }))); + const sql = await buildSql(); if (isCancelledBeforeDispatch()) throw new Error(DANGER_OPERATION_CANCELLED_BEFORE_DISPATCH_MESSAGE); const executed = await options.executeWithProductionGuard(node, sql, { database: node.database, schema: node.schema, executionId, isCancelledBeforeDispatch, markDispatched }); if (executed === undefined) { // The user declined the production-safety confirmation: the SQL was - // never sent, so this must not be reported as a successful drop. + // never sent, so this must not be reported as a successful operation. endDangerRunningExecution(); return; } - toast(t("contextMenu.dropTableSuccess", { name: node.label }), 3000); - options.closeDroppedTableObjectTabsForNode(node); - connectionStore.removeTreeNode(node.id); - options.releaseActiveNodeReference([node.id]); + await onSuccess(node); endDangerRunningExecution(); } catch (error: any) { const message = error?.message || String(error); const backendError = normalizeBackendError(error) ?? undefined; - toastDangerOperationError(node.label, message, wasCancelled(), cancelConfirmed(), backendError); - if (cancelConfirmed() || !isQueryTimeoutErrorMessage(message, backendError)) { + const cancellationWasRequested = wasCancelled(); + const cancellationWasConfirmed = cancelConfirmed() || (cancellationWasRequested && (await waitForCancelConfirmation())); + toastDangerOperationError(node.label, message, cancellationWasRequested, cancellationWasConfirmed, backendError); + if (cancellationWasConfirmed || !isQueryTimeoutErrorMessage(message, backendError)) { endDangerRunningExecution(); } else { markHandedOff(); @@ -377,6 +481,20 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt } } + async function confirmDropTable() { + const node = sidebarDangerTarget.value ?? activeNode.value; + await runDangerOperation({ + node, + buildSql: () => dropTablePreviewSql.value || buildDropTableSql(tableAdminSqlOptionsForNode(node, { cascade: dropTableCascade.value && supportsDropTableCascade(databaseTypeForNode(node)) })), + onSuccess: (node) => { + toast(t("contextMenu.dropTableSuccess", { name: node.label }), 3000); + options.closeDroppedTableObjectTabsForNode(node); + connectionStore.removeTreeNode(node.id); + options.releaseActiveNodeReference([node.id]); + }, + }); + } + function emptyTable() { void refreshEmptyTablePreviewSql(); showEmptyTableConfirm.value = true; @@ -384,33 +502,15 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt async function confirmEmptyTable() { const node = sidebarDangerTarget.value ?? activeNode.value; - if (!node.connectionId || node.database == null) return; - const { executionId, isCancelledBeforeDispatch, markDispatched, wasCancelled, cancelConfirmed, markHandedOff } = beginDangerRunningExecution(node.label); - try { - await connectionStore.ensureConnected(node.connectionId); - const sql = emptyTablePreviewSql.value || (await buildEmptyTableSql(tableAdminSqlOptionsForNode(node))); - if (isCancelledBeforeDispatch()) throw new Error(DANGER_OPERATION_CANCELLED_BEFORE_DISPATCH_MESSAGE); - const executed = await options.executeWithProductionGuard(node, sql, { database: node.database, schema: node.schema, executionId, isCancelledBeforeDispatch, markDispatched }); - if (executed === undefined) { - // The user declined the production-safety confirmation: the SQL was - // never sent, so this must not be reported as a successful empty. - endDangerRunningExecution(); - return; - } - const messageKey = databaseTypeForNode(node) === "clickhouse" ? "contextMenu.emptyTableSubmitted" : "contextMenu.emptyTableSuccess"; - toast(t(messageKey, { name: node.label }), 3000); - await options.refreshMutatedTableDataTabsForNode(node); - endDangerRunningExecution(); - } catch (error: any) { - const message = error?.message || String(error); - const backendError = normalizeBackendError(error) ?? undefined; - toastDangerOperationError(node.label, message, wasCancelled(), cancelConfirmed(), backendError); - if (cancelConfirmed() || !isQueryTimeoutErrorMessage(message, backendError)) { - endDangerRunningExecution(); - } else { - markHandedOff(); - } - } + await runDangerOperation({ + node, + buildSql: () => emptyTablePreviewSql.value || buildEmptyTableSql(tableAdminSqlOptionsForNode(node)), + onSuccess: async (node) => { + const messageKey = databaseTypeForNode(node) === "clickhouse" ? "contextMenu.emptyTableSubmitted" : "contextMenu.emptyTableSuccess"; + toast(t(messageKey, { name: node.label }), 3000); + await options.refreshMutatedTableDataTabsForNode(node); + }, + }); } function truncateTable() { @@ -421,32 +521,14 @@ export function useSidebarTableMutationRuntime(options: SidebarTableMutationRunt async function confirmTruncateTable() { const node = sidebarDangerTarget.value ?? activeNode.value; - if (!node.connectionId || node.database == null) return; - const { executionId, isCancelledBeforeDispatch, markDispatched, wasCancelled, cancelConfirmed, markHandedOff } = beginDangerRunningExecution(node.label); - try { - await connectionStore.ensureConnected(node.connectionId); - const sql = truncateTablePreviewSql.value || (await buildTruncateTableSql(tableAdminSqlOptionsForNode(node, { cascade: truncateTableCascade.value && supportsTruncateTableCascade(databaseTypeForNode(node)) }))); - if (isCancelledBeforeDispatch()) throw new Error(DANGER_OPERATION_CANCELLED_BEFORE_DISPATCH_MESSAGE); - const executed = await options.executeWithProductionGuard(node, sql, { database: node.database, schema: node.schema, executionId, isCancelledBeforeDispatch, markDispatched }); - if (executed === undefined) { - // The user declined the production-safety confirmation: the SQL was - // never sent, so this must not be reported as a successful truncate. - endDangerRunningExecution(); - return; - } - toast(t("contextMenu.truncateTableSuccess", { name: node.label }), 3000); - await options.refreshMutatedTableDataTabsForNode(node); - endDangerRunningExecution(); - } catch (error: any) { - const message = error?.message || String(error); - const backendError = normalizeBackendError(error) ?? undefined; - toastDangerOperationError(node.label, message, wasCancelled(), cancelConfirmed(), backendError); - if (cancelConfirmed() || !isQueryTimeoutErrorMessage(message, backendError)) { - endDangerRunningExecution(); - } else { - markHandedOff(); - } - } + await runDangerOperation({ + node, + buildSql: () => truncateTablePreviewSql.value || buildTruncateTableSql(tableAdminSqlOptionsForNode(node, { cascade: truncateTableCascade.value && supportsTruncateTableCascade(databaseTypeForNode(node)) })), + onSuccess: async (node) => { + toast(t("contextMenu.truncateTableSuccess", { name: node.label }), 3000); + await options.refreshMutatedTableDataTabsForNode(node); + }, + }); } return { diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index 943aac79a3..840f2c5986 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -2958,6 +2958,8 @@ export default { tableOperationTimedOut: 'The operation on "{name}" is taking longer than expected and has not been cancelled — it may still be running on the database. Check the result later, or click Cancel to stop it. ({message})', tableOperationCancelled: 'Operation on "{name}" cancelled', tableOperationCancelUnconfirmed: 'The cancel request for "{name}" was not confirmed by the database — the operation may have already finished or may still be running. Check the result to be sure. ({message})', + tableOperationCancelPending: 'The cancel request for "{name}" has not been confirmed yet — it may still be running on the database. You can try Cancel again, or check back shortly.', + dangerOperationAlreadyRunning: "Another dangerous operation is still running. Wait for it to finish or cancel it before starting a new one.", objectDropRefreshFailed: "Objects were deleted, but refreshing the sidebar failed: {message}", duplicateNameTitle: "Clone as New Table", duplicateNamePlaceholder: "New table name", diff --git a/apps/desktop/src/i18n/locales/es.ts b/apps/desktop/src/i18n/locales/es.ts index 568d05476f..3c17de8de7 100644 --- a/apps/desktop/src/i18n/locales/es.ts +++ b/apps/desktop/src/i18n/locales/es.ts @@ -2861,6 +2861,8 @@ export default withEnglishFallback({ tableOperationTimedOut: 'La operación en "{name}" está tardando más de lo esperado y no se ha cancelado; es posible que siga ejecutándose en la base de datos. Compruebe el resultado más tarde o haga clic en Cancelar para detenerla. ({message})', tableOperationCancelled: 'Operación en "{name}" cancelada', tableOperationCancelUnconfirmed: 'La solicitud de cancelación de "{name}" no fue confirmada por la base de datos; la operación puede haber terminado o seguir en ejecución. Compruebe el resultado real. ({message})', + tableOperationCancelPending: 'La solicitud de cancelación de "{name}" aún no ha sido confirmada; puede que la operación siga en ejecución en la base de datos. Puedes intentar cancelar de nuevo o comprobarlo en unos instantes.', + dangerOperationAlreadyRunning: "Otra operación peligrosa sigue en ejecución. Espera a que finalice o cancélala antes de iniciar una nueva.", objectDropRefreshFailed: "Los objetos se eliminaron, pero no se pudo actualizar la barra lateral: {message}", duplicateNameTitle: "Clonar como tabla nueva", duplicateNamePlaceholder: "Nombre de la nueva tabla", diff --git a/apps/desktop/src/i18n/locales/it.ts b/apps/desktop/src/i18n/locales/it.ts index 9dddc9009a..dea1730e22 100644 --- a/apps/desktop/src/i18n/locales/it.ts +++ b/apps/desktop/src/i18n/locales/it.ts @@ -2859,6 +2859,8 @@ export default withEnglishFallback({ tableOperationTimedOut: 'L\'operazione su "{name}" sta impiegando più tempo del previsto e non è stata annullata: potrebbe essere ancora in esecuzione sul database. Controlla il risultato più tardi oppure fai clic su Annulla per interromperla. ({message})', tableOperationCancelled: 'Operazione su "{name}" annullata', tableOperationCancelUnconfirmed: 'La richiesta di annullamento per "{name}" non è stata confermata dal database: l\'operazione potrebbe essere già terminata o essere ancora in esecuzione. Verifica il risultato effettivo. ({message})', + tableOperationCancelPending: 'La richiesta di annullamento per "{name}" non è stata ancora confermata: l\'operazione potrebbe essere ancora in esecuzione sul database. Puoi riprovare ad annullare oppure controllare tra poco.', + dangerOperationAlreadyRunning: "Un'altra operazione pericolosa è ancora in esecuzione. Attendi che finisca o annullala prima di iniziarne una nuova.", objectDropRefreshFailed: "Gli oggetti sono stati eliminati, ma l'aggiornamento della barra laterale non è riuscito: {message}", duplicateNameTitle: "Clona as Nuova Tabella", duplicateNamePlaceholder: "Nuovo nome tabella", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index 218090159c..1cbe38a1ff 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -2882,6 +2882,8 @@ export default withEnglishFallback({ tableOperationTimedOut: "「{name}」の操作に予想より時間がかかっており、まだキャンセルされていません。データベース側では実行が続いている可能性があります。後で結果を確認するか、キャンセルをクリックして停止してください。（{message}）", tableOperationCancelled: "「{name}」の操作をキャンセルしました", tableOperationCancelUnconfirmed: "「{name}」のキャンセル要求はデータベース側で確認されませんでした。操作はすでに完了しているか、まだ実行中の可能性があります。実際の結果をご確認ください。（{message}）", + tableOperationCancelPending: "「{name}」のキャンセル要求はまだ確認されていません。データベース側でまだ実行中の可能性があります。もう一度キャンセルを試すか、しばらくしてから確認してください。", + dangerOperationAlreadyRunning: "別の危険な操作がまだ実行中です。完了を待つか、キャンセルしてから新しい操作を開始してください。", objectDropRefreshFailed: "オブジェクトは削除されましたが、サイドバーの更新に失敗しました: {message}", duplicateNameTitle: "新しいテーブルとして複製", duplicateNamePlaceholder: "新しいテーブル名", diff --git a/apps/desktop/src/i18n/locales/ko.ts b/apps/desktop/src/i18n/locales/ko.ts index 4d4ed39634..4f1dbd5b87 100644 --- a/apps/desktop/src/i18n/locales/ko.ts +++ b/apps/desktop/src/i18n/locales/ko.ts @@ -2778,6 +2778,8 @@ export default withEnglishFallback({ tableOperationTimedOut: '"{name}"에 대한 작업이 예상보다 오래 걸리고 있으며 취소되지 않았습니다. 데이터베이스에서 계속 실행 중일 수 있습니다. 나중에 결과를 확인하거나 취소를 클릭하여 중지하세요. ({message})', tableOperationCancelled: '"{name}" 작업이 취소되었습니다', tableOperationCancelUnconfirmed: '"{name}"에 대한 취소 요청이 데이터베이스에서 확인되지 않았습니다. 작업이 이미 완료되었거나 계속 실행 중일 수 있습니다. 실제 결과를 확인하세요. ({message})', + tableOperationCancelPending: '"{name}"에 대한 취소 요청이 아직 확인되지 않았습니다. 데이터베이스에서 계속 실행 중일 수 있습니다. 다시 취소를 시도하거나 잠시 후 확인해 보세요.', + dangerOperationAlreadyRunning: "다른 위험한 작업이 아직 실행 중입니다. 완료되기를 기다리거나 취소한 후 새 작업을 시작하세요.", objectDropRefreshFailed: "개체는 삭제되었지만 사이드바 새로 고침에 실패했습니다: {message}", duplicateNameTitle: "새 테이블로 복제", duplicateNamePlaceholder: "새 테이블 이름", diff --git a/apps/desktop/src/i18n/locales/pt-BR.ts b/apps/desktop/src/i18n/locales/pt-BR.ts index 5b19eba75b..348ed37d4a 100644 --- a/apps/desktop/src/i18n/locales/pt-BR.ts +++ b/apps/desktop/src/i18n/locales/pt-BR.ts @@ -2861,6 +2861,8 @@ export default withEnglishFallback({ tableOperationTimedOut: 'A operação em "{name}" está demorando mais do que o esperado e não foi cancelada — ela pode continuar em execução no banco de dados. Verifique o resultado mais tarde ou clique em Cancelar para interrompê-la. ({message})', tableOperationCancelled: 'Operação em "{name}" cancelada', tableOperationCancelUnconfirmed: 'A solicitação de cancelamento de "{name}" não foi confirmada pelo banco de dados — a operação pode já ter terminado ou ainda estar em execução. Verifique o resultado real. ({message})', + tableOperationCancelPending: 'A solicitação de cancelamento de "{name}" ainda não foi confirmada — a operação pode continuar em execução no banco de dados. Você pode tentar cancelar novamente ou verificar em instantes.', + dangerOperationAlreadyRunning: "Outra operação perigosa ainda está em execução. Aguarde a conclusão ou cancele-a antes de iniciar uma nova.", objectDropRefreshFailed: "Os objetos foram excluídos, mas não foi possível atualizar a barra lateral: {message}", duplicateNameTitle: "Clonar como Nova Tabela", duplicateNamePlaceholder: "Nome da nova tabela", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index 8a526674fa..885e312c4f 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -2882,6 +2882,8 @@ export default withEnglishFallback({ tableOperationTimedOut: "「{name}」的操作耗时较长，尚未被取消，可能仍在数据库端继续执行。请稍后自行确认执行结果，或点击「取消」手动终止。（{message}）", tableOperationCancelled: "「{name}」的操作已取消", tableOperationCancelUnconfirmed: "「{name}」的取消请求未被数据库确认，该操作可能已经完成，也可能仍在继续执行，请自行核实实际结果。（{message}）", + tableOperationCancelPending: "「{name}」的取消请求尚未得到确认，该操作可能仍在数据库中执行。可以再次点击取消，或稍后再查看结果。", + dangerOperationAlreadyRunning: "还有一个危险操作正在执行，请等待其完成或取消后再开始新的操作。", objectDropRefreshFailed: "对象已删除，但侧边栏刷新失败：{message}", duplicateNameTitle: "克隆为新表", duplicateNamePlaceholder: "新表名", diff --git a/apps/desktop/src/i18n/locales/zh-TW.ts b/apps/desktop/src/i18n/locales/zh-TW.ts index cddb84d24b..2e1c08c718 100644 --- a/apps/desktop/src/i18n/locales/zh-TW.ts +++ b/apps/desktop/src/i18n/locales/zh-TW.ts @@ -2860,6 +2860,8 @@ export default withEnglishFallback({ tableOperationTimedOut: "「{name}」的操作耗時較長，尚未被取消，可能仍在資料庫端繼續執行。請稍後自行確認執行結果，或點擊「取消」手動終止。（{message}）", tableOperationCancelled: "「{name}」的操作已取消", tableOperationCancelUnconfirmed: "「{name}」的取消請求未被資料庫確認，該操作可能已經完成，也可能仍在繼續執行，請自行核實實際結果。（{message}）", + tableOperationCancelPending: "「{name}」的取消請求尚未得到確認，該操作可能仍在資料庫中執行。可以再次點擊取消，或稍後再查看結果。", + dangerOperationAlreadyRunning: "還有一個危險操作正在執行，請等待其完成或取消後再開始新的操作。", objectDropRefreshFailed: "物件已刪除，但側邊欄重新整理失敗：{message}", duplicateNameTitle: "克隆為新資料表", duplicateNamePlaceholder: "新資料表名稱", diff --git a/crates/dbx-core/src/query_cancel.rs b/crates/dbx-core/src/query_cancel.rs index bd70afe80d..59c800f324 100644 --- a/crates/dbx-core/src/query_cancel.rs +++ b/crates/dbx-core/src/query_cancel.rs @@ -62,18 +62,24 @@ impl RunningTaskMetadata { } } -#[derive(Clone)] struct RunningTask { registration_id: u64, token: CancellationToken, metadata: RunningTaskMetadata, pool_key: Option, + interrupt: Option, } #[derive(Clone, Default)] pub struct RunningQueries { + // Interrupt closures live inside `RunningTask` rather than a second map + // so that a task's presence and its interrupt handle always change + // together under one lock acquisition. Splitting them across two + // mutexes previously left a window where `cancel()` could remove the + // task while a driver's `register_interrupt()` — running concurrently, + // before its interrupt handle was ready — inserted a closure keyed to + // an execution id nothing would ever look at again, leaking it forever. inner: Arc>>, - interrupts: Arc>>, next_registration_id: Arc, } @@ -87,11 +93,11 @@ impl RunningQueries { let registration_id = self.next_registration_id.fetch_add(1, Ordering::Relaxed) + 1; let previous = self.inner.lock().unwrap_or_else(|e| e.into_inner()).insert( execution_id.clone(), - RunningTask { registration_id, token: token.clone(), metadata, pool_key: None }, + RunningTask { registration_id, token: token.clone(), metadata, pool_key: None, interrupt: None }, ); if let Some(previous) = previous { previous.token.cancel(); - if let Some(interrupt) = self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).remove(&execution_id) { + if let Some(interrupt) = previous.interrupt { interrupt(); } } @@ -99,23 +105,39 @@ impl RunningQueries { RegisteredQuery { execution_id, registration_id, token, running_queries: self.clone(), detached: false } } + /// Hands over the driver-specific interrupt handle (e.g. a MySQL + /// `KILL QUERY` or a DuckDB worker cancel) once it becomes available, + /// which is necessarily *after* the task itself was registered. + /// + /// If a `cancel()` already ran for this execution id by the time this + /// arrives, the task is already gone and nothing will ever call + /// `cancel()` again for it — so the interrupt runs immediately instead + /// of being stored, which would otherwise leak it forever (nothing ever + /// visits an interrupt for a task that no longer exists). pub fn register_interrupt(&self, execution_id: &str, interrupt: impl Fn() + Send + 'static) { - self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).insert(execution_id.to_string(), Box::new(interrupt)); + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(task) = inner.get_mut(execution_id) { + task.interrupt = Some(Box::new(interrupt)); + return; + } + drop(inner); + interrupt(); } pub fn cancel(&self, execution_id: &str) -> bool { - let token = - self.inner.lock().unwrap_or_else(|e| e.into_inner()).get(execution_id).map(|task| task.token.clone()); - let interrupt = self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).remove(execution_id); - - if let Some(interrupt) = interrupt { - interrupt(); - } - if let Some(token) = token { - token.cancel(); - true - } else { - false + let task = { + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + inner.remove(execution_id) + }; + match task { + Some(task) => { + if let Some(interrupt) = task.interrupt { + interrupt(); + } + task.token.cancel(); + true + } + None => false, } } @@ -140,17 +162,16 @@ impl RunningQueries { let mut active_execution_ids = inner.keys().cloned().collect::>(); active_execution_ids.sort(); let mut active_by_connection = HashMap::new(); + let mut interrupt_registrations = 0; for task in inner.values() { if let Some(connection_id) = &task.metadata.connection_id { *active_by_connection.entry(connection_id.clone()).or_insert(0) += 1; } + if task.interrupt.is_some() { + interrupt_registrations += 1; + } } - drop(inner); - RunningQueryDiagnostics { - active_execution_ids, - active_by_connection, - interrupt_registrations: self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).len(), - } + RunningQueryDiagnostics { active_execution_ids, active_by_connection, interrupt_registrations } } fn cancel_matching(&self, predicate: impl Fn(&RunningTask) -> bool) -> usize { @@ -190,24 +211,16 @@ impl RunningQueries { #[cfg(test)] pub fn registration_counts(&self) -> (usize, usize) { - ( - self.inner.lock().unwrap_or_else(|e| e.into_inner()).len(), - self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).len(), - ) + let inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + let tasks = inner.len(); + let interrupts = inner.values().filter(|task| task.interrupt.is_some()).count(); + (tasks, interrupts) } fn remove(&self, execution_id: &str, registration_id: u64) { - let removed = { - let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); - if inner.get(execution_id).is_some_and(|task| task.registration_id == registration_id) { - inner.remove(execution_id); - true - } else { - false - } - }; - if removed { - self.interrupts.lock().unwrap_or_else(|e| e.into_inner()).remove(execution_id); + let mut inner = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + if inner.get(execution_id).is_some_and(|task| task.registration_id == registration_id) { + inner.remove(execution_id); } } } @@ -267,6 +280,27 @@ impl RegisteredQuery { running_queries.remove(&execution_id, registration_id); }); } + + /// Detaches on a client-observed timeout (server-side execution may + /// still be running and reachable for a later explicit cancel); + /// otherwise drops normally. Centralizes the branch duplicated across + /// every HTTP/Tauri query-execution entry point. + pub fn finish(self, result: &Result) { + self.finish_with_late_cancel(result, true); + } + + /// Finishes a registration while preserving timed-out work only when the + /// caller has an execution id it can use for a later explicit cancel. + pub fn finish_with_late_cancel( + self, + result: &Result, + keep_timeout_reachable: bool, + ) { + if keep_timeout_reachable && matches!(result, Err(crate::query::QueryExecutionError::Timeout(_))) { + self.detach(); + } + // else: falls out of scope here and Drop removes the registration. + } } impl Drop for RegisteredQuery { @@ -322,6 +356,7 @@ mod tests { let interrupted = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); let flag = interrupted.clone(); let registered = running.register("exec-timeout".to_string()); + running.set_pool_key("exec-timeout", "pool-1"); running.register_interrupt("exec-timeout", move || { flag.store(true, std::sync::atomic::Ordering::SeqCst); }); @@ -330,11 +365,17 @@ mod tests { // timeout) while the statement may still be running server-side. registered.detach(); assert!(running.has("exec-timeout")); + assert!(running.is_pool_active("pool-1")); // A later explicit cancel must still reach the (still registered) // KILL-QUERY-style interrupt. assert!(running.cancel("exec-timeout")); assert!(interrupted.load(std::sync::atomic::Ordering::SeqCst)); + + // Cancelling a detached, timed-out query must free it immediately — + // not after the 30-minute detach grace period. + assert!(!running.has("exec-timeout")); + assert!(!running.is_pool_active("pool-1")); } #[tokio::test(start_paused = true)] @@ -356,6 +397,34 @@ mod tests { assert!(!running.has("exec-timeout")); } + #[tokio::test(start_paused = true)] + async fn finish_detaches_on_timeout_and_drops_otherwise() { + let running = RunningQueries::default(); + + let registered = running.register("exec-timeout".to_string()); + registered.finish(&Result::<(), _>::Err(crate::query::QueryExecutionError::Timeout("t".into()))); + assert!(running.has("exec-timeout")); + + let registered = running.register("exec-ok".to_string()); + registered.finish(&Result::<(), _>::Ok(())); + assert!(!running.has("exec-ok")); + } + + #[tokio::test(start_paused = true)] + async fn finish_only_keeps_timed_out_registration_when_late_cancel_is_reachable() { + let running = RunningQueries::default(); + let timeout = Result::<(), _>::Err(crate::query::QueryExecutionError::Timeout("t".into())); + + let registered = running.register("exec-internal".to_string()); + registered.finish_with_late_cancel(&timeout, false); + assert!(!running.has("exec-internal")); + + let registered = running.register("exec-client".to_string()); + registered.finish_with_late_cancel(&timeout, true); + assert!(running.has("exec-client")); + assert!(running.cancel("exec-client")); + } + #[test] fn stale_registration_drop_does_not_remove_replacement() { let running = RunningQueries::default(); @@ -441,6 +510,31 @@ mod tests { assert_eq!(running.registration_counts(), (0, 0)); } + #[test] + fn late_interrupt_registration_after_cancel_runs_immediately_and_does_not_leak() { + let running = RunningQueries::default(); + let registered = running.register("exec-1".to_string()); + + // The user cancels before the driver has a chance to hand over its + // interrupt handle (e.g. it is still establishing the connection it + // would issue a KILL QUERY against). + assert!(running.cancel("exec-1")); + + let interrupted = std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)); + let flag = interrupted.clone(); + running.register_interrupt("exec-1", move || { + flag.store(true, std::sync::atomic::Ordering::SeqCst); + }); + + // Nobody will ever call cancel() again for this execution id, so the + // late registration must run immediately instead of being stored. + assert!(interrupted.load(std::sync::atomic::Ordering::SeqCst)); + + drop(registered); + + assert_eq!(running.registration_counts(), (0, 0)); + } + #[test] fn completion_after_cancel_removes_all_registrations() { let running = RunningQueries::default(); diff --git a/crates/dbx-web/src/routes/query.rs b/crates/dbx-web/src/routes/query.rs index ce07cc263e..c5c17abc03 100644 --- a/crates/dbx-web/src/routes/query.rs +++ b/crates/dbx-web/src/routes/query.rs @@ -356,7 +356,9 @@ pub async fn execute_query( let allow_database_switch = req.client_session_id.as_deref().is_some_and(|id| !id.trim().is_empty()); super::mcp_policy::ensure_sql(&state, &headers, &req.connection_id, &req.database, &req.sql, allow_database_switch) .await?; - let execution_id = req.execution_id.unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); + let requested_execution_id = req.execution_id.filter(|id| !id.trim().is_empty()); + let keep_timeout_reachable = requested_execution_id.is_some(); + let execution_id = requested_execution_id.unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); let registered = state.app.running_queries.register_task( execution_id.clone(), @@ -391,10 +393,11 @@ pub async fn execute_query( ..Default::default() }, ) - .await - .map_err(|error| AppError::from(error.into_backend_error()))?; + .await; - drop(registered); + registered.finish_with_late_cancel(&result, keep_timeout_reachable); + + let result = result.map_err(|error| AppError::from(error.into_backend_error()))?; Ok(Json(result)) } @@ -406,7 +409,9 @@ pub async fn execute_multi( let allow_database_switch = req.client_session_id.as_deref().is_some_and(|id| !id.trim().is_empty()); super::mcp_policy::ensure_sql(&state, &headers, &req.connection_id, &req.database, &req.sql, allow_database_switch) .await?; - let execution_id = req.execution_id.unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); + let requested_execution_id = req.execution_id.filter(|id| !id.trim().is_empty()); + let keep_timeout_reachable = requested_execution_id.is_some(); + let execution_id = requested_execution_id.unwrap_or_else(|| uuid::Uuid::new_v4().to_string()); let registered = state.app.running_queries.register_task( execution_id.clone(), @@ -442,11 +447,12 @@ pub async fn execute_multi( execution_mode: req.execution_mode.unwrap_or_default(), }, ) - .await - .map_err(|error| AppError::from(error.into_backend_error()))?; + .await; let core_ms = core_started_at.elapsed().as_millis(); - drop(registered); + registered.finish_with_late_cancel(&result, keep_timeout_reachable); + + let result = result.map_err(|error| AppError::from(error.into_backend_error()))?; execute_multi_response(result, core_ms) } diff --git a/packages/app-tests/sidebarMutationRuntime.test.ts b/packages/app-tests/sidebarMutationRuntime.test.ts index 378607d265..fdea85cd7c 100644 --- a/packages/app-tests/sidebarMutationRuntime.test.ts +++ b/packages/app-tests/sidebarMutationRuntime.test.ts @@ -32,9 +32,15 @@ test("mutation families retain accepted targets, failures, and refresh work", () const body = functionBody(name); assert.match(body, /sidebarDangerTarget\.value \?\? activeNode\.value/); assert.match(body, /refreshMutatedTableDataTabsForNode\(node\)/); - assert.match(body, /toastDangerOperationError\(node\.label/); + // Failure reporting (including the tableOperationFailed toast) lives in + // the shared runDangerOperation helper, not inlined in each confirm* + // function. + assert.match(body, /runDangerOperation\(/); } + const runDangerOperation = functionBody("runDangerOperation"); + assert.match(runDangerOperation, /toastDangerOperationError\(/); + for (const name of ["confirmCreateNacosNamespace", "confirmEditNacosNamespace", "confirmDropDatabase"]) { const body = functionBody(name); assert.match(body, /try \{/); diff --git a/src-tauri/src/commands/query.rs b/src-tauri/src/commands/query.rs index f89260e614..a7b48186a7 100644 --- a/src-tauri/src/commands/query.rs +++ b/src-tauri/src/commands/query.rs @@ -74,15 +74,8 @@ pub async fn execute_query( ) .await; - if matches!(result, Err(dbx_core::query::QueryExecutionError::Timeout(_))) { - // We're giving up on waiting, not cancelling: the statement may - // still be executing server-side. Keep the registration (and any - // KILL-QUERY-style interrupt registered against it) reachable so a - // later explicit cancel_query call can still reach it, instead of - // losing that capability the instant this command returns. - if let Some(registered_query) = registered_query { - registered_query.detach(); - } + if let Some(registered_query) = registered_query { + registered_query.finish(&result); } result.map_err(dbx_core::query::QueryExecutionError::into_backend_error) @@ -194,6 +187,11 @@ pub async fn execute_multi( error ), } + + if let Some(registered_query) = registered_query { + registered_query.finish(&result); + } + result.map_err(dbx_core::query::QueryExecutionError::into_backend_error) }