From 00c4d3045382bd89baa1ad45db630861cc3d8cfb Mon Sep 17 00:00:00 2001 From: zipg Date: Sat, 19 Sep 2026 00:22:18 +0800 Subject: [PATCH] fix(agent): keep offline import resilient per item with os error detail --- .../components/config/DriverStoreDialog.vue | 26 +- .../agentOfflineImportParity.spec.ts | 55 ++ apps/desktop/src/i18n/locales/az.ts | 1 + apps/desktop/src/i18n/locales/en.ts | 1 + apps/desktop/src/i18n/locales/es.ts | 1 + apps/desktop/src/i18n/locales/it.ts | 1 + apps/desktop/src/i18n/locales/ja.ts | 1 + apps/desktop/src/i18n/locales/ko.ts | 1 + apps/desktop/src/i18n/locales/pt-BR.ts | 1 + apps/desktop/src/i18n/locales/tr.ts | 1 + apps/desktop/src/i18n/locales/zh-CN.ts | 1 + apps/desktop/src/i18n/locales/zh-TW.ts | 1 + .../__tests__/agentOfflineImport.spec.ts | 12 +- apps/desktop/src/lib/backend/api.ts | 2 + apps/desktop/src/lib/backend/http.ts | 2 +- apps/desktop/src/lib/backend/tauri.ts | 11 + crates/dbx-core/tests/agent_service.rs | 120 ++- crates/dbx-drivers/src/agent_service.rs | 766 +++++++++++++++--- crates/dbx-web/src/routes/agents.rs | 1 + src-tauri/src/commands/agents.rs | 6 +- 20 files changed, 858 insertions(+), 153 deletions(-) create mode 100644 apps/desktop/src/i18n/__tests__/agentOfflineImportParity.spec.ts diff --git a/apps/desktop/src/components/config/DriverStoreDialog.vue b/apps/desktop/src/components/config/DriverStoreDialog.vue index 2c2e9d19a8..3e7d87835d 100644 --- a/apps/desktop/src/components/config/DriverStoreDialog.vue +++ b/apps/desktop/src/components/config/DriverStoreDialog.vue @@ -18,7 +18,7 @@ import { uuid } from "@/lib/common/utils"; import { countAvailableDriverUpdates } from "@/lib/connection/agentDriverUpdateBadge"; import type { JdbcDriverInfo, JdbcLocalBundleInfo, JdbcMavenBundleInfo, JdbcPluginStatus } from "@/types/database"; import * as api from "@/lib/backend/api"; -import type { AgentDriverInfo, AgentOfflineExportPreview, DriverRuntimeInfo, DriverRuntimeSummary, DriverStoreUsage, JavaRuntimeConfig } from "@/lib/backend/api"; +import type { AgentDriverInfo, AgentOfflineExportPreview, AgentOfflineImportResult, DriverRuntimeInfo, DriverRuntimeSummary, DriverStoreUsage, JavaRuntimeConfig } from "@/lib/backend/api"; import { formatRuntimeBytes, formatRuntimeCpu, formatRuntimeUptime, runtimeHealthClass, runtimeStatusClass, runtimeStatusDotClass } from "@/lib/connection/driverRuntimePresentation"; import { addDriverInstallQueue, @@ -745,7 +745,7 @@ async function importOfflineZip() { try { const result = await api.importAgentsFromZip(selected, activeAgentOperationId.value); await Promise.all([refreshAgents(), loadJdbcDrivers(), loadJdbcPluginStatus()]); - toast(t(result.jreCount > 0 ? (result.count > 0 ? "driverStore.offlineImportWithJreSuccess" : "driverStore.offlineJreImportSuccess") : "driverStore.offlineImportSuccess", { count: result.count, jreCount: result.jreCount })); + toastOfflineImportResult(result); } catch (e: any) { toast(t("driverStore.offlineImportFailed", { error: backendError(e) })); } finally { @@ -755,6 +755,26 @@ async function importOfflineZip() { } } +// An offline package keeps importing after one item fails (a blocked JRE +// archive, a corrupt driver JAR), so surface what did install alongside the +// per-item failure instead of reporting the whole import as failed. +function toastOfflineImportResult(result: AgentOfflineImportResult) { + if (result.failures.length > 0) { + const [failure] = result.failures; + toast( + t("driverStore.offlineImportPartial", { + count: result.count, + jreCount: result.jreCount, + failed: result.failures.length, + item: failure.is_jre ? `JRE ${failure.key}` : driverLabel(failure.key), + error: failure.error, + }), + ); + return; + } + toast(t(result.jreCount > 0 ? (result.count > 0 ? "driverStore.offlineImportWithJreSuccess" : "driverStore.offlineJreImportSuccess") : "driverStore.offlineImportSuccess", { count: result.count, jreCount: result.jreCount })); +} + async function importDriverFile(driver: AgentDriverInfo) { if (agentPackageBusy.value) return; const dbType = driver.db_type; @@ -777,7 +797,7 @@ async function importDriverFile(driver: AgentDriverInfo) { try { const result = await api.importAgentsFromZip(selected, activeAgentOperationId.value); await Promise.all([refreshAgents(), loadJdbcDrivers(), loadJdbcPluginStatus()]); - toast(t(result.jreCount > 0 ? (result.count > 0 ? "driverStore.offlineImportWithJreSuccess" : "driverStore.offlineJreImportSuccess") : "driverStore.offlineImportSuccess", { count: result.count, jreCount: result.jreCount })); + toastOfflineImportResult(result); } finally { activeAgentOperationId.value = null; resetAgentInstallProgress(); diff --git a/apps/desktop/src/i18n/__tests__/agentOfflineImportParity.spec.ts b/apps/desktop/src/i18n/__tests__/agentOfflineImportParity.spec.ts new file mode 100644 index 0000000000..52f30e669f --- /dev/null +++ b/apps/desktop/src/i18n/__tests__/agentOfflineImportParity.spec.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from "vitest"; +import az from "../locales/az"; +import en from "../locales/en"; +import es from "../locales/es"; +import itLocale from "../locales/it"; +import ja from "../locales/ja"; +import ko from "../locales/ko"; +import ptBR from "../locales/pt-BR"; +import tr from "../locales/tr"; +import zhCN from "../locales/zh-CN"; +import zhTW from "../locales/zh-TW"; + +type Messages = Record; + +const locales: Array<[string, Messages]> = [ + ["az", az], + ["es", es], + ["it", itLocale], + ["ja", ja], + ["ko", ko], + ["pt-BR", ptBR], + ["tr", tr], + ["zh-CN", zhCN], + ["zh-TW", zhTW], +]; + +function offlineImportEntries(locale: Messages): Map { + const driverStore = locale.driverStore as Record; + return new Map( + Object.entries(driverStore) + .filter(([key, value]) => (key.startsWith("offlineImport") || key.startsWith("offlineJreImport")) && typeof value === "string") + .map(([key, value]) => [key, value as string]), + ); +} + +function placeholders(message: string): string[] { + return [...new Set([...message.matchAll(/\{([^{}]+)\}/g)].map((match) => match[1]))].sort(); +} + +describe("offline Agent import i18n parity", () => { + const englishEntries = offlineImportEntries(en); + + it("declares the complete English message set", () => { + expect([...englishEntries.keys()].sort()).toEqual(["offlineImportFailed", "offlineImportPartial", "offlineImportSuccess", "offlineImportWithJreSuccess", "offlineJreImportSuccess"]); + }); + + it.each(locales)("%s declares matching keys and placeholders", (_name, locale) => { + const localeEntries = offlineImportEntries(locale); + expect([...localeEntries.keys()].sort()).toEqual([...englishEntries.keys()].sort()); + + for (const [key, englishMessage] of englishEntries) { + expect(placeholders(localeEntries.get(key) ?? ""), key).toEqual(placeholders(englishMessage)); + } + }); +}); diff --git a/apps/desktop/src/i18n/locales/az.ts b/apps/desktop/src/i18n/locales/az.ts index 84092d50fe..a18bc45471 100644 --- a/apps/desktop/src/i18n/locales/az.ts +++ b/apps/desktop/src/i18n/locales/az.ts @@ -8054,6 +8054,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "Oflayn idxal tamamlandı. {jreCount} JRE quraşdırıldı.", offlineImportWithJreSuccess: "Oflayn idxal tamamlandı. {jreCount} JRE və {count} drayver quraşdırıldı.", offlineImportFailed: "Oflayn idxal uğursuz oldu: {error}", + offlineImportPartial: "Oflayn idxal qismən tamamlandı: {failed} element uğursuz oldu, {jreCount} JRE və {count} drayver quraşdırıldı. {item}: {error}", driverImportSuccess: "{label} drayveri idxal edildi", driverImportFailed: "{label} drayverini idxal etmək mümkün olmadı: {error}", chooseDriverJar: "Drayver faylını seç", diff --git a/apps/desktop/src/i18n/locales/en.ts b/apps/desktop/src/i18n/locales/en.ts index 84b1b31b8c..dc430dbbb9 100644 --- a/apps/desktop/src/i18n/locales/en.ts +++ b/apps/desktop/src/i18n/locales/en.ts @@ -8289,6 +8289,7 @@ export default { offlineJreImportSuccess: "Offline import complete. Installed {jreCount} JRE(s).", offlineImportWithJreSuccess: "Offline import complete. Installed {jreCount} JRE(s) and {count} driver(s).", offlineImportFailed: "Offline import failed: {error}", + offlineImportPartial: "Offline import finished with problems: {failed} item(s) failed, installed {jreCount} JRE(s) and {count} driver(s). {item}: {error}", driverImportSuccess: "{label} driver imported", driverImportFailed: "Failed to import {label} driver: {error}", chooseDriverJar: "Choose driver file", diff --git a/apps/desktop/src/i18n/locales/es.ts b/apps/desktop/src/i18n/locales/es.ts index 544b8647f1..9d2e57ac5a 100644 --- a/apps/desktop/src/i18n/locales/es.ts +++ b/apps/desktop/src/i18n/locales/es.ts @@ -7824,6 +7824,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "Importación offline completada. Se instalaron {jreCount} JRE(s).", offlineImportWithJreSuccess: "Importación offline completada. Se instalaron {jreCount} JRE(s) y {count} driver(s).", offlineImportFailed: "Error en la importación offline: {error}", + offlineImportPartial: "Importación offline completada con incidencias: {failed} elemento(s) fallidos, se instalaron {jreCount} JRE(s) y {count} driver(s). {item}: {error}", driverImportSuccess: "Driver {label} importado", driverImportFailed: "Error al importar el driver {label}: {error}", chooseDriverJar: "Seleccionar archivo del driver", diff --git a/apps/desktop/src/i18n/locales/it.ts b/apps/desktop/src/i18n/locales/it.ts index f8d5b5ae27..bc8585f810 100644 --- a/apps/desktop/src/i18n/locales/it.ts +++ b/apps/desktop/src/i18n/locales/it.ts @@ -7824,6 +7824,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "Importazione offline completata. Installato/i {jreCount} JRE.", offlineImportWithJreSuccess: "Importazione offline completata. Installato/i {jreCount} JRE e {count} driver.", offlineImportFailed: "Importazione offline non riuscita: {error}", + offlineImportPartial: "Importazione offline completata con problemi: {failed} elemento/i non riusciti, installati {jreCount} JRE e {count} driver. {item}: {error}", driverImportSuccess: "Driver {label} importato", driverImportFailed: "Importazione del driver {label} non riuscita: {error}", chooseDriverJar: "Scegli il file del driver", diff --git a/apps/desktop/src/i18n/locales/ja.ts b/apps/desktop/src/i18n/locales/ja.ts index 3dcaf5b5b5..1a62792d72 100644 --- a/apps/desktop/src/i18n/locales/ja.ts +++ b/apps/desktop/src/i18n/locales/ja.ts @@ -7876,6 +7876,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "オフラインインポートが完了しました。{jreCount}件のJREをインストールしました。", offlineImportWithJreSuccess: "オフラインインポートが完了しました。{jreCount}件のJREと{count}件のドライバーをインストールしました。", offlineImportFailed: "オフラインインポートに失敗しました: {error}", + offlineImportPartial: "オフラインインポートが一部完了しました：{failed}件が失敗し、{jreCount}件のJREと{count}件のドライバーをインストールしました。{item}: {error}", driverImportSuccess: "{label} ドライバーをインポートしました", driverImportFailed: "{label} ドライバーのインポートに失敗しました: {error}", chooseDriverJar: "ドライバーファイルを選択", diff --git a/apps/desktop/src/i18n/locales/ko.ts b/apps/desktop/src/i18n/locales/ko.ts index afbec7deb6..862ef32f93 100644 --- a/apps/desktop/src/i18n/locales/ko.ts +++ b/apps/desktop/src/i18n/locales/ko.ts @@ -7556,6 +7556,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "오프라인 가져오기 완료. JRE {jreCount}개 설치.", offlineImportWithJreSuccess: "오프라인 가져오기 완료. JRE {jreCount}개와 드라이버 {count}개 설치.", offlineImportFailed: "오프라인 가져오기 실패: {error}", + offlineImportPartial: "오프라인 가져오기가 일부 완료되었습니다: {failed}개 실패, JRE {jreCount}개와 드라이버 {count}개 설치됨. {item}: {error}", driverImportSuccess: "{label} 드라이버를 가져왔습니다", driverImportFailed: "{label} 드라이버 가져오기 실패: {error}", chooseDriverJar: "드라이버 파일 선택", diff --git a/apps/desktop/src/i18n/locales/pt-BR.ts b/apps/desktop/src/i18n/locales/pt-BR.ts index 2d40a569f1..3125482a92 100644 --- a/apps/desktop/src/i18n/locales/pt-BR.ts +++ b/apps/desktop/src/i18n/locales/pt-BR.ts @@ -7826,6 +7826,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "Importação offline concluída. {jreCount} JRE(s) instalado(s).", offlineImportWithJreSuccess: "Importação offline concluída. {jreCount} JRE(s) e {count} driver(s) instalado(s).", offlineImportFailed: "Falha na importação offline: {error}", + offlineImportPartial: "Importação offline concluída com problemas: {failed} item(ns) com falha, {jreCount} JRE(s) e {count} driver(s) instalado(s). {item}: {error}", driverImportSuccess: "Driver {label} importado", driverImportFailed: "Falha ao importar o driver {label}: {error}", chooseDriverJar: "Escolher arquivo do driver", diff --git a/apps/desktop/src/i18n/locales/tr.ts b/apps/desktop/src/i18n/locales/tr.ts index 0b3eb1d71f..bb9b616b88 100644 --- a/apps/desktop/src/i18n/locales/tr.ts +++ b/apps/desktop/src/i18n/locales/tr.ts @@ -7909,6 +7909,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "Çevrimdışı içe aktarma tamamlandı. {jreCount} JRE kuruldu.", offlineImportWithJreSuccess: "Çevrimdışı içe aktarma tamamlandı. {jreCount} JRE ve {count} sürücü kuruldu.", offlineImportFailed: "Çevrimdışı içe aktarma başarısız: {error}", + offlineImportPartial: "Çevrimdışı içe aktarma sorunlarla tamamlandı: {failed} öğe başarısız, {jreCount} JRE ve {count} sürücü kuruldu. {item}: {error}", driverImportSuccess: "{label} sürücüsü içe aktarıldı", driverImportFailed: "{label} sürücüsü içe aktarılamadı: {error}", chooseDriverJar: "Sürücü dosyasını seçin", diff --git a/apps/desktop/src/i18n/locales/zh-CN.ts b/apps/desktop/src/i18n/locales/zh-CN.ts index e8a2353fc3..98d238bc88 100644 --- a/apps/desktop/src/i18n/locales/zh-CN.ts +++ b/apps/desktop/src/i18n/locales/zh-CN.ts @@ -8261,6 +8261,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "离线导入完成，已安装 {jreCount} 个 JRE", offlineImportWithJreSuccess: "离线导入完成，已安装 {jreCount} 个 JRE 和 {count} 个驱动", offlineImportFailed: "离线导入失败: {error}", + offlineImportPartial: "离线导入部分完成：{failed} 项失败，已安装 {jreCount} 个 JRE 和 {count} 个驱动。{item}：{error}", driverImportSuccess: "{label} 驱动导入成功", driverImportFailed: "{label} 驱动导入失败: {error}", chooseDriverJar: "选择驱动文件", diff --git a/apps/desktop/src/i18n/locales/zh-TW.ts b/apps/desktop/src/i18n/locales/zh-TW.ts index 88b3edb413..999849fdb4 100644 --- a/apps/desktop/src/i18n/locales/zh-TW.ts +++ b/apps/desktop/src/i18n/locales/zh-TW.ts @@ -7131,6 +7131,7 @@ export default withEnglishFallback({ offlineJreImportSuccess: "離線匯入完成，已安裝 {jreCount} 個 JRE", offlineImportWithJreSuccess: "離線匯入完成，已安裝 {jreCount} 個 JRE 和 {count} 個驅動程式", offlineImportFailed: "離線匯入失敗： {error}", + offlineImportPartial: "離線匯入部分完成：{failed} 項失敗，已安裝 {jreCount} 個 JRE 和 {count} 個驅動程式。{item}：{error}", driverImportSuccess: "{label} 驅動程式匯入成功", driverImportFailed: "{label} 驅動程式匯入失敗： {error}", chooseDriverJar: "選擇驅動程式檔案", diff --git a/apps/desktop/src/lib/backend/__tests__/agentOfflineImport.spec.ts b/apps/desktop/src/lib/backend/__tests__/agentOfflineImport.spec.ts index 4cd6c163a4..c92ad16156 100644 --- a/apps/desktop/src/lib/backend/__tests__/agentOfflineImport.spec.ts +++ b/apps/desktop/src/lib/backend/__tests__/agentOfflineImport.spec.ts @@ -17,11 +17,12 @@ describe("offline Agent and standalone JRE import", () => { }); it("uploads standalone JRE packages and preserves both result counts", async () => { - const fetchMock = vi.fn().mockResolvedValue(new Response(JSON.stringify({ count: 0, jreCount: 1 }))); + const failures = [{ key: "21", is_jre: true, error: "Failed to extract JRE archive: Access is denied. (os error 5)" }]; + const fetchMock = vi.fn().mockResolvedValue(new Response(JSON.stringify({ count: 0, jreCount: 1, failures }))); vi.stubGlobal("fetch", fetchMock); const { importAgentsFromZip } = await import("../http"); const file = new File(["archive"], "renamed.tar.zst"); - expect(await importAgentsFromZip(file, "import-2")).toEqual({ count: 0, jreCount: 1 }); + expect(await importAgentsFromZip(file, "import-2")).toEqual({ count: 0, jreCount: 1, failures }); const [url, request] = fetchMock.mock.calls[0]; expect(url).toContain("/api/agents/import-offline"); expect(request.method).toBe("POST"); @@ -32,6 +33,11 @@ describe("offline Agent and standalone JRE import", () => { it("remains compatible with web backends that only return a driver count", async () => { vi.stubGlobal("fetch", vi.fn().mockResolvedValue(new Response(JSON.stringify({ count: 2 })))); const { importAgentsFromZip } = await import("../http"); - expect(await importAgentsFromZip(new File(["archive"], "drivers.zip"))).toEqual({ count: 2, jreCount: 0 }); + // Older web backends report no per-item failures; the caller must still see an empty list. + expect(await importAgentsFromZip(new File(["archive"], "drivers.zip"))).toEqual({ + count: 2, + jreCount: 0, + failures: [], + }); }); }); diff --git a/apps/desktop/src/lib/backend/api.ts b/apps/desktop/src/lib/backend/api.ts index c5e20c6716..fb09b096ce 100644 --- a/apps/desktop/src/lib/backend/api.ts +++ b/apps/desktop/src/lib/backend/api.ts @@ -921,6 +921,8 @@ export type { AgentOfflineExportCandidate, AgentOfflineExportPreview, AgentOfflineExportResult, + AgentOfflineImportFailure, + AgentOfflineImportResult, DriverStoreUsage, DriverStoreUsageItem, DriverRuntimeHealth, diff --git a/apps/desktop/src/lib/backend/http.ts b/apps/desktop/src/lib/backend/http.ts index 08f823d080..34179b3689 100644 --- a/apps/desktop/src/lib/backend/http.ts +++ b/apps/desktop/src/lib/backend/http.ts @@ -879,7 +879,7 @@ export async function importAgentsFromZip(fileOrPath: string | File, operationId }); if (!res.ok) throw await backendResponseError(res); const result: AgentOfflineImportResult = await res.json(); - return { count: result.count, jreCount: result.jreCount ?? 0 }; + return { count: result.count, jreCount: result.jreCount ?? 0, failures: result.failures ?? [] }; } export async function previewAgentOfflineExport(): Promise { diff --git a/apps/desktop/src/lib/backend/tauri.ts b/apps/desktop/src/lib/backend/tauri.ts index b0c42b2e07..3be872f669 100644 --- a/apps/desktop/src/lib/backend/tauri.ts +++ b/apps/desktop/src/lib/backend/tauri.ts @@ -206,6 +206,17 @@ export interface AgentOfflineExportResult { export interface AgentOfflineImportResult { count: number; jreCount: number; + /** Items the package could not install; the rest of the import still ran. */ + failures: AgentOfflineImportFailure[]; +} + +export interface AgentOfflineImportFailure { + /** Managed JRE key (e.g. "21") or driver key (e.g. "oracle"). */ + key: string; + /** True when the failed item is a managed JRE runtime rather than a driver. */ + is_jre: boolean; + /** Failure text, including the underlying OS error when there is one. */ + error: string; } export type JavaRuntimeMode = "managed" | "system" | "custom"; diff --git a/crates/dbx-core/tests/agent_service.rs b/crates/dbx-core/tests/agent_service.rs index c953710cc3..f1e03ff38b 100644 --- a/crates/dbx-core/tests/agent_service.rs +++ b/crates/dbx-core/tests/agent_service.rs @@ -703,24 +703,33 @@ async fn offline_zip_import_preserves_existing_jre_when_archive_is_corrupt() { let original_java = std::fs::read(&java_path).unwrap(); write_offline_driver_zip_with_jre_bytes(&corrupt_zip, "h2", "0.3.0", "21.0.13", b"not-a-tar-gz".to_vec()); - let err = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap_err(); + let result = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap(); - assert!(err.contains("Failed to extract JRE archive")); + assert!(result.jre_installed.is_empty()); + // The unusable JRE must not stop the drivers that do work. + assert_eq!(result.drivers_installed, vec!["h2"]); + assert_eq!(result.failures.len(), 1); + assert!(result.failures[0].is_jre); + assert_eq!(result.failures[0].key, DEFAULT_JRE_KEY); + assert!( + result.failures[0].error.contains("Failed to extract JRE archive"), + "unexpected error: {}", + result.failures[0].error + ); assert_eq!(std::fs::read(java_path).unwrap(), original_java); assert_eq!(manager.load_state().jre_versions.get(DEFAULT_JRE_KEY).map(String::as_str), Some("21.0.12")); } #[tokio::test] -async fn offline_zip_import_preserves_existing_jre_when_driver_is_corrupt() { - let manager = test_manager("offline-corrupt-driver-preserves-jre"); - let root = test_path("offline-corrupt-driver-preserves-jre-zip"); +async fn offline_zip_import_installs_jre_and_reports_corrupt_driver() { + let manager = test_manager("offline-corrupt-driver-reports-failure"); + let root = test_path("offline-corrupt-driver-reports-failure-zip"); let valid_zip = root.join("valid.zip"); let corrupt_zip = root.join("corrupt.zip"); std::fs::create_dir_all(&root).unwrap(); write_offline_driver_zip_with_jre(&valid_zip, "h2", "0.2.0", "21.0.12"); import_agents_from_zip(&manager, &valid_zip, |_| {}).await.unwrap(); - let java_path = manager.jre_java_path(DEFAULT_JRE_KEY); - let original_java = std::fs::read(&java_path).unwrap(); + let original_jar = std::fs::read(manager.driver_jar_path("h2")).unwrap(); write_offline_driver_zip_with_jre_and_jar_bytes( &corrupt_zip, @@ -730,23 +739,33 @@ async fn offline_zip_import_preserves_existing_jre_when_driver_is_corrupt() { test_jre_archive_bytes(), b"jar".to_vec(), ); - let err = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap_err(); + let result = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap(); - assert!(err.contains("invalid or corrupt")); - assert_eq!(std::fs::read(java_path).unwrap(), original_java); - assert_eq!(manager.load_state().jre_versions.get(DEFAULT_JRE_KEY).map(String::as_str), Some("21.0.12")); + assert_eq!(result.jre_installed, vec![DEFAULT_JRE_KEY]); + assert!(result.drivers_installed.is_empty()); + assert_eq!(result.failures.len(), 1); + assert!(!result.failures[0].is_jre); + assert_eq!(result.failures[0].key, "h2"); + assert!(result.failures[0].error.contains("invalid or corrupt"), "unexpected error: {}", result.failures[0].error); + // A corrupt driver artifact must not replace the working installation. + assert_eq!(std::fs::read(manager.driver_jar_path("h2")).unwrap(), original_jar); + assert_eq!(manager.load_state().installed_drivers["h2"].version, "0.2.0"); + assert_eq!(manager.load_state().jre_versions.get(DEFAULT_JRE_KEY).map(String::as_str), Some("21.0.13")); } #[tokio::test] -async fn offline_zip_import_rejects_corrupt_jar() { +async fn offline_zip_import_reports_corrupt_jar_without_installing_it() { let manager = test_manager("offline-corrupt-driver"); let zip_path = test_path("offline-corrupt-driver-zip").join("agents.zip"); std::fs::create_dir_all(zip_path.parent().unwrap()).unwrap(); write_offline_driver_zip_with_jar(&zip_path, "h2", "0.2.0", b"jar".to_vec()); - let err = import_agents_from_zip(&manager, &zip_path, |_| {}).await.unwrap_err(); + let result = import_agents_from_zip(&manager, &zip_path, |_| {}).await.unwrap(); - assert!(err.contains("invalid or corrupt")); + assert!(result.drivers_installed.is_empty()); + assert_eq!(result.failures.len(), 1); + assert_eq!(result.failures[0].key, "h2"); + assert!(result.failures[0].error.contains("invalid or corrupt"), "unexpected error: {}", result.failures[0].error); assert!(!manager.driver_jar_path("h2").exists()); assert!(!manager.load_state().installed_drivers.contains_key("h2")); } @@ -763,13 +782,34 @@ async fn offline_zip_import_preserves_existing_driver_when_jar_is_corrupt() { let original = std::fs::read(manager.driver_jar_path("h2")).unwrap(); write_offline_driver_zip_with_jar(&corrupt_zip, "h2", "0.3.0", b"jar".to_vec()); - let err = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap_err(); + let result = import_agents_from_zip(&manager, &corrupt_zip, |_| {}).await.unwrap(); - assert!(err.contains("invalid or corrupt")); + assert_eq!(result.failures.len(), 1); + assert_eq!(result.failures[0].key, "h2"); + assert!(result.failures[0].error.contains("invalid or corrupt"), "unexpected error: {}", result.failures[0].error); assert_eq!(std::fs::read(manager.driver_jar_path("h2")).unwrap(), original); assert_eq!(manager.load_state().installed_drivers["h2"].version, "0.2.0"); } +#[tokio::test] +async fn offline_zip_import_installs_the_remaining_drivers_when_one_item_is_broken() { + let manager = test_manager("offline-partial-driver-package"); + let zip_path = test_path("offline-partial-driver-package-zip").join("agents.zip"); + std::fs::create_dir_all(zip_path.parent().unwrap()).unwrap(); + let valid_jar = test_agent_jar_bytes(); + write_offline_java_driver_zip(&zip_path, &[("h2", "0.2.0", &valid_jar), ("duckdb", "0.1.0", b"not-a-jar")]); + + let result = import_agents_from_zip(&manager, &zip_path, |_| {}).await.unwrap(); + + assert_eq!(result.drivers_installed, vec!["h2"]); + assert_eq!(manager.load_state().installed_drivers["h2"].version, "0.2.0"); + assert_eq!(result.failures.len(), 1); + assert!(!result.failures[0].is_jre); + assert_eq!(result.failures[0].key, "duckdb"); + assert!(result.failures[0].error.contains("invalid or corrupt"), "unexpected error: {}", result.failures[0].error); + assert!(!manager.is_driver_installed("duckdb")); +} + #[tokio::test] async fn offline_zip_import_keeps_legacy_unversioned_jar_compatibility() { let manager = test_manager("offline-legacy-jar-zip"); @@ -884,9 +924,11 @@ async fn offline_zip_import_preserves_existing_native_driver_when_binary_is_inva AgentManager::current_platform(), b"not-a-native-agent".to_vec(), ); - let err = import_agents_from_zip(&manager, &invalid_zip, |_| {}).await.unwrap_err(); + let result = import_agents_from_zip(&manager, &invalid_zip, |_| {}).await.unwrap(); - assert!(err.contains("not a")); + assert_eq!(result.failures.len(), 1); + assert_eq!(result.failures[0].key, "kingbase"); + assert!(result.failures[0].error.contains("not a"), "unexpected error: {}", result.failures[0].error); assert_eq!(std::fs::read(manager.driver_native_path("kingbase")).unwrap(), original); assert_eq!(manager.load_state().installed_drivers["kingbase"].version, "0.1.34"); } @@ -1289,25 +1331,41 @@ fn test_jre_archive_zstd_bytes() -> Vec { } fn write_offline_driver_zip_with_jar(path: &std::path::Path, db_type: &str, version: &str, jar: Vec) { + write_offline_java_driver_zip(path, &[(db_type, version, &jar)]); +} + +/// Build a package from `(db_type, version, jar bytes)` tuples so a single +/// package can mix working and broken drivers. +fn write_offline_java_driver_zip(path: &std::path::Path, drivers: &[(&str, &str, &[u8])]) { let file = std::fs::File::create(path).unwrap(); let mut zip = zip::ZipWriter::new(file); let options = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); - let registry = serde_json::json!({ - "drivers": { - db_type: { - "version": version, - "label": db_type, - "min_app_version": "0.1.0", - "jre": DEFAULT_JRE_KEY, - "jar": { "url": format!("https://example.com/dbx-agent-{db_type}-{version}.jar"), "size": jar.len() } - } - } - }); + let registry_drivers = drivers + .iter() + .map(|(db_type, version, jar)| { + ( + (*db_type).to_string(), + serde_json::json!({ + "version": version, + "label": db_type, + "min_app_version": "0.1.0", + "jre": DEFAULT_JRE_KEY, + "jar": { + "url": format!("https://example.com/dbx-agent-{db_type}-{version}.jar"), + "size": jar.len() + } + }), + ) + }) + .collect::>(); + let registry = serde_json::json!({ "drivers": registry_drivers }); zip.start_file("agent-registry.json", options).unwrap(); std::io::Write::write_all(&mut zip, registry.to_string().as_bytes()).unwrap(); - zip.start_file(format!("drivers/dbx-agent-{db_type}-{version}.jar"), options).unwrap(); - std::io::Write::write_all(&mut zip, &jar).unwrap(); + for (db_type, version, jar) in drivers { + zip.start_file(format!("drivers/dbx-agent-{db_type}-{version}.jar"), options).unwrap(); + std::io::Write::write_all(&mut zip, jar).unwrap(); + } zip.finish().unwrap(); } diff --git a/crates/dbx-drivers/src/agent_service.rs b/crates/dbx-drivers/src/agent_service.rs index 3075a34869..d2cd45f6cb 100644 --- a/crates/dbx-drivers/src/agent_service.rs +++ b/crates/dbx-drivers/src/agent_service.rs @@ -29,6 +29,17 @@ const JRE_REMOVE_ATTEMPTS: usize = if cfg!(windows) { 6 } else { 1 }; /// Exponential-ish backoff between retries. Total wait ≈ 1.55s on Windows. const JRE_REMOVE_BACKOFF_MS: &[u64] = &[50, 100, 200, 400, 400, 400]; +/// Attempts to unpack an archive before reporting a failure. On Windows an +/// anti-virus/EDR scanner briefly holds a file it has just seen created, which +/// surfaces as `Access is denied`/`sharing violation` while `tar` writes the +/// archive contents. POSIX returns 1 — a failure there is not transient. +const ARCHIVE_EXTRACT_ATTEMPTS: usize = if cfg!(windows) { 4 } else { 1 }; + +/// Exponential-ish backoff between archive extraction retries. Total wait +/// ≈ 0.85s on Windows: long enough to cover the AV scan window, short enough +/// not to stall the import. +const ARCHIVE_EXTRACT_BACKOFF_MS: &[u64] = &[100, 250, 500]; + /// Keep batch updates concurrent without allowing a large registry to exhaust /// the download server, local disk, or the application's file descriptors. const MAX_CONCURRENT_AGENT_UPDATES: usize = 4; @@ -82,6 +93,46 @@ fn format_jre_dir_remove_error(path: &Path, os_err: &std::io::Error) -> String { } } +/// Render an error together with the I/O failure it wrapped. +/// +/// `tar` reports every failed entry as `failed to unpack \`\`` and +/// `zip` reports archive-level failures as `invalid Zip archive: …`; neither +/// prints the OS error in `Display`. On Windows that hides whether the +/// extraction was blocked by anti-virus (`Access is denied. (os error 5)`), +/// by a scanner still holding the file (`The process cannot access the file … +/// (os error 32)`), or by a full/quota-limited disk (`There is not enough +/// space … (os error 112)`), which made offline-import failures impossible to +/// diagnose from the UI. Append the deepest I/O cause — preferring one that +/// carries an OS error code — so the OS error survives, without repeating the +/// wrapper text `tar` already printed. +fn describe_error(error: &dyn std::error::Error) -> String { + let mut message = error.to_string(); + if let Some(io_error) = deepest_io_error(error) { + let text = io_error.to_string(); + if !text.is_empty() && !message.contains(&text) { + message.push_str(": "); + message.push_str(&text); + } + } + message +} + +/// Deepest `std::io::Error` in an error chain, preferring one that carries an +/// OS error code over a generic wrapper. +fn deepest_io_error(error: &dyn std::error::Error) -> Option<&std::io::Error> { + let mut detail: Option<&std::io::Error> = None; + let mut cause = error.source(); + while let Some(current) = cause { + if let Some(io_error) = current.downcast_ref::() { + if io_error.raw_os_error().is_some() || detail.is_none() { + detail = Some(io_error); + } + } + cause = current.source(); + } + detail +} + /// Windows-only: rename the old JRE dir to a unique sibling so the install /// can continue even when files inside are still mapped. Returns the stash /// path so the caller can record it for later cleanup. On POSIX this is @@ -2468,11 +2519,26 @@ pub struct OfflineImportProgress { pub db_type: Option, } +/// An offline-import item (managed JRE runtime or driver) that could not be +/// installed. The import continues past it so a single blocked or corrupt +/// item cannot abort the whole package. +#[derive(Debug, Clone, serde::Serialize)] +pub struct OfflineImportFailure { + /// Managed JRE key (e.g. `21`) or driver key (e.g. `oracle`). + pub key: String, + /// True when the failed item is a managed JRE runtime rather than a driver. + pub is_jre: bool, + /// Failure text, including the underlying OS error when there is one. + pub error: String, +} + #[derive(Debug, Clone)] pub struct OfflineImportResult { pub jre_installed: Vec, pub drivers_installed: Vec, pub drivers_skipped: Vec, + /// Items that failed; reported to the user instead of failing the import. + pub failures: Vec, } #[derive(Debug)] @@ -2591,6 +2657,7 @@ async fn import_tar_zstd_jre_package( jre_installed: vec![info.key.clone()], drivers_installed: Vec::new(), drivers_skipped: Vec::new(), + failures: Vec::new(), }) } @@ -2642,8 +2709,12 @@ async fn import_tar_zstd_driver_package( ) -> Result { let _installation_guard = am.installation_operation_lock.write().await; let info = tar_zstd_driver_package_info(package_path)?; - let mut result = - OfflineImportResult { jre_installed: Vec::new(), drivers_installed: Vec::new(), drivers_skipped: Vec::new() }; + let mut result = OfflineImportResult { + jre_installed: Vec::new(), + drivers_installed: Vec::new(), + drivers_skipped: Vec::new(), + failures: Vec::new(), + }; if let Some(installed) = am.load_state().installed_drivers.get(&info.db_type) { if installed.version != "0.1.0-local" && installed.version != "local" @@ -2824,8 +2895,8 @@ pub async fn import_offline_zip( // JRE installs, Upgrade All, and uninstall operations are serialised. let _installation_guard = am.installation_operation_lock.write().await; - let file = std::fs::File::open(zip_path).map_err(|e| format!("Failed to open ZIP file: {e}"))?; - let mut archive = zip::ZipArchive::new(file).map_err(|e| format!("Invalid ZIP file: {e}"))?; + let file = std::fs::File::open(zip_path).map_err(|e| format!("Failed to open ZIP file: {}", describe_error(&e)))?; + let mut archive = zip::ZipArchive::new(file).map_err(|e| format!("Invalid ZIP file: {}", describe_error(&e)))?; let registry = read_registry_from_zip(&mut archive)?; @@ -2837,11 +2908,15 @@ pub async fn import_offline_zip( return Err(format!("Offline package contains no drivers compatible with platform: {platform}")); } validate_offline_zip_preflight(&mut archive, ®istry, &jre_entries, &driver_entries)?; - std::fs::create_dir_all(am.base_dir()).map_err(|e| format!("Failed to create agent directory: {e}"))?; - validate_offline_driver_entries(am, &mut archive, &driver_entries)?; + std::fs::create_dir_all(am.base_dir()) + .map_err(|e| format!("Failed to create agent directory: {}", describe_error(&e)))?; let mut local_state = am.load_state(); - let mut result = - OfflineImportResult { jre_installed: Vec::new(), drivers_installed: Vec::new(), drivers_skipped: Vec::new() }; + let mut result = OfflineImportResult { + jre_installed: Vec::new(), + drivers_installed: Vec::new(), + drivers_skipped: Vec::new(), + failures: Vec::new(), + }; let mut current: u32 = 0; for (jre_key, entry_name, format) in &jre_entries { @@ -2860,36 +2935,51 @@ pub async fn import_offline_zip( db_type: None, }); - let mut entry = archive.by_name(entry_name).map_err(|e| format!("Failed to read {entry_name}: {e}"))?; - let tmp_archive = am.base_dir().join(format!("jre-offline-{jre_key}{}", jre_archive_suffix(*format))); - { - let mut out = - std::fs::File::create(&tmp_archive).map_err(|e| format!("Failed to create temp file: {e}"))?; - std::io::copy(&mut entry, &mut out).map_err(|e| format!("Failed to extract JRE archive: {e}"))?; - } + // A blocked write (anti-virus, disk quota) or an invalid archive must + // not abort the rest of the package: record the failure and continue so + // the drivers still install. + let outcome = (|| -> Result<(), String> { + let mut entry = archive + .by_name(entry_name) + .map_err(|e| format!("Failed to read {entry_name}: {}", describe_error(&e)))?; + let tmp_archive = am.base_dir().join(format!("jre-offline-{jre_key}{}", jre_archive_suffix(*format))); + { + let mut out = std::fs::File::create(&tmp_archive) + .map_err(|e| format!("Failed to create temp file: {}", describe_error(&e)))?; + std::io::copy(&mut entry, &mut out) + .map_err(|e| format!("Failed to extract JRE archive: {}", describe_error(&e)))?; + } - let jre_dir = am.jre_dir(jre_key); - let staging_dir = am.base_dir().join(format!(".jre-offline-import-{}", uuid::Uuid::new_v4())); - if let Err(error) = extract_jre_archive(&tmp_archive, &staging_dir, *format) { - std::fs::remove_dir_all(&staging_dir).ok(); + let jre_dir = am.jre_dir(jre_key); + let staging_dir = am.base_dir().join(format!(".jre-offline-import-{}", uuid::Uuid::new_v4())); + if let Err(error) = extract_jre_archive(&tmp_archive, &staging_dir, *format) { + std::fs::remove_dir_all(&staging_dir).ok(); + std::fs::remove_file(&tmp_archive).ok(); + return Err(error); + } + if !jre_dir_contains_java(&staging_dir) { + std::fs::remove_dir_all(&staging_dir).ok(); + std::fs::remove_file(&tmp_archive).ok(); + return Err(format!("Offline JRE archive does not contain a Java executable: {entry_name}")); + } + let pending_cleanup = replace_imported_jre_dir(&staging_dir, &jre_dir)?; std::fs::remove_file(&tmp_archive).ok(); - return Err(error); - } - if !jre_dir_contains_java(&staging_dir) { - std::fs::remove_dir_all(&staging_dir).ok(); - std::fs::remove_file(&tmp_archive).ok(); - return Err(format!("Offline JRE archive does not contain a Java executable: {entry_name}")); - } - let pending_cleanup = replace_imported_jre_dir(&staging_dir, &jre_dir)?; - std::fs::remove_file(&tmp_archive).ok(); - if let Some(path) = pending_cleanup { - local_state.pending_jre_cleanup.push(path); - } + if let Some(path) = pending_cleanup { + local_state.pending_jre_cleanup.push(path); + } - if let Some(ver) = jre_version { - local_state.jre_versions.insert(jre_key.clone(), ver); + if let Some(ver) = jre_version { + local_state.jre_versions.insert(jre_key.clone(), ver); + } + Ok(()) + })(); + match outcome { + Ok(()) => result.jre_installed.push(jre_key.clone()), + Err(error) => { + log::warn!("Offline JRE import failed for {jre_key}: {error}"); + result.failures.push(OfflineImportFailure { key: jre_key.clone(), is_jre: true, error }); + } } - result.jre_installed.push(jre_key.clone()); } for (db_type, entry_name, is_native) in &driver_entries { @@ -2916,45 +3006,68 @@ pub async fn import_offline_zip( }); let driver_path = if *is_native { am.driver_native_path(db_type) } else { am.driver_jar_path(db_type) }; - if let Some(parent) = driver_path.parent() { - std::fs::create_dir_all(parent).map_err(|e| e.to_string())?; - } - let mut entry = archive.by_name(entry_name).map_err(|e| format!("Failed to read {entry_name}: {e}"))?; - let parent = driver_path.parent().ok_or_else(|| format!("Invalid driver path: {}", driver_path.display()))?; - let staging_path = parent.join(format!(".offline-agent-import-{}", uuid::Uuid::new_v4())); - let mut out = std::fs::File::create(&staging_path).map_err(|e| format!("Failed to write driver: {e}"))?; - std::io::copy(&mut entry, &mut out).map_err(|e| format!("Failed to copy driver: {e}"))?; - drop(out); - if *is_native { - if let Err(error) = validate_native_agent_binary(&staging_path) { - std::fs::remove_file(&staging_path).ok(); - return Err(error); + // Same per-item isolation as the JRE loop: one unreadable or blocked + // driver must not stop the remaining drivers from installing. + let outcome = (|| -> Result<(), String> { + if let Some(parent) = driver_path.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| format!("Failed to create driver directory: {}", describe_error(&e)))?; } - mark_executable(&staging_path)?; - } else { - // Validate the staged JAR before replacing a working driver so a - // corrupt offline package cannot destroy the previous installation. - if !is_valid_agent_jar(&staging_path) { - std::fs::remove_file(&staging_path).ok(); - return Err(format!("Offline agent jar is invalid or corrupt: {entry_name}")); + let mut entry = archive + .by_name(entry_name) + .map_err(|e| format!("Failed to read {entry_name}: {}", describe_error(&e)))?; + let parent = + driver_path.parent().ok_or_else(|| format!("Invalid driver path: {}", driver_path.display()))?; + let staging_path = parent.join(format!(".offline-agent-import-{}", uuid::Uuid::new_v4())); + let mut out = std::fs::File::create(&staging_path) + .map_err(|e| format!("Failed to write driver: {}", describe_error(&e)))?; + match std::io::copy(&mut entry, &mut out) { + Ok(_) => {} + Err(e) => { + std::fs::remove_file(&staging_path).ok(); + return Err(format!("Failed to copy driver: {}", describe_error(&e))); + } + } + drop(out); + if *is_native { + if let Err(error) = validate_native_agent_binary(&staging_path) { + std::fs::remove_file(&staging_path).ok(); + return Err(error); + } + mark_executable(&staging_path)?; + } else { + // Validate the staged JAR before replacing a working driver so a + // corrupt offline package cannot destroy the previous installation. + if !is_valid_agent_jar(&staging_path) { + std::fs::remove_file(&staging_path).ok(); + return Err(format!("Offline agent jar is invalid or corrupt: {entry_name}")); + } + } + replace_imported_agent_file(&staging_path, &driver_path)?; + if *is_native { + std::fs::remove_file(am.driver_jar_path(db_type)).ok(); + } else { + std::fs::remove_file(am.driver_native_path(db_type)).ok(); + } + + let version = + registry.drivers.get(db_type).map(|d| d.version.clone()).unwrap_or_else(|| "local".to_string()); + let jre_key = + registry.drivers.get(db_type).map(|d| d.jre.clone()).unwrap_or_else(|| DEFAULT_JRE_KEY.to_string()); + + local_state.installed_drivers.insert( + db_type.clone(), + InstalledDriver { version, installed_at: chrono::Utc::now().to_rfc3339(), jre: jre_key }, + ); + Ok(()) + })(); + match outcome { + Ok(()) => result.drivers_installed.push(db_type.clone()), + Err(error) => { + log::warn!("Offline driver import failed for {db_type}: {error}"); + result.failures.push(OfflineImportFailure { key: db_type.clone(), is_jre: false, error }); } } - replace_imported_agent_file(&staging_path, &driver_path)?; - if *is_native { - std::fs::remove_file(am.driver_jar_path(db_type)).ok(); - } else { - std::fs::remove_file(am.driver_native_path(db_type)).ok(); - } - - let version = registry.drivers.get(db_type).map(|d| d.version.clone()).unwrap_or_else(|| "local".to_string()); - let jre_key = - registry.drivers.get(db_type).map(|d| d.jre.clone()).unwrap_or_else(|| DEFAULT_JRE_KEY.to_string()); - - local_state.installed_drivers.insert( - db_type.clone(), - InstalledDriver { version, installed_at: chrono::Utc::now().to_rfc3339(), jre: jre_key }, - ); - result.drivers_installed.push(db_type.clone()); } am.mutate_state(|state| { @@ -3130,32 +3243,6 @@ fn validate_offline_zip_artifact( } } -fn validate_offline_driver_entries( - am: &AgentManager, - archive: &mut zip::ZipArchive, - driver_entries: &[OfflineDriverEntry], -) -> Result<(), String> { - for (_, entry_name, is_native) in driver_entries { - let staging_path = am.base_dir().join(format!(".offline-agent-validation-{}", uuid::Uuid::new_v4())); - let result = (|| { - let mut entry = archive.by_name(entry_name).map_err(|e| format!("Failed to read {entry_name}: {e}"))?; - let mut out = std::fs::File::create(&staging_path).map_err(|e| format!("Failed to write driver: {e}"))?; - std::io::copy(&mut entry, &mut out).map_err(|e| format!("Failed to copy driver: {e}"))?; - drop(out); - if *is_native { - validate_native_agent_binary(&staging_path) - } else if is_valid_agent_jar(&staging_path) { - Ok(()) - } else { - Err(format!("Offline agent jar is invalid or corrupt: {entry_name}")) - } - })(); - std::fs::remove_file(&staging_path).ok(); - result?; - } - Ok(()) -} - fn validate_offline_driver_key(db_type: &str) -> Result<(), String> { validate_offline_identifier(db_type, "driver")?; if agent_catalog::label_for_key(db_type).is_none() { @@ -3251,55 +3338,346 @@ fn jre_archive_download_path(am: &AgentManager, jre_key: &str, format: Option) -> Result<(), String> { + let mut last_error: Option = None; + for attempt in 0..ARCHIVE_EXTRACT_ATTEMPTS { + if attempt > 0 { + // Windows rename cannot replace an existing directory, so a failed + // attempt may leave partial entries in dest and every retry would + // then fail forever. Clear dest first; errors are ignored because + // a failed attempt may also leave no dest at all, and extract recreates it. + let _ = std::fs::remove_dir_all(dest); + let delay_ms = ARCHIVE_EXTRACT_BACKOFF_MS.get(attempt - 1).copied().unwrap_or(500); + std::thread::sleep(Duration::from_millis(delay_ms)); + } + match extract_jre_archive_once(archive, dest, format) { + Ok(()) => return Ok(()), + Err(error) => { + log::warn!( + "JRE archive extraction ({}) attempt {}/{} failed: {error}", + archive.display(), + attempt + 1, + ARCHIVE_EXTRACT_ATTEMPTS + ); + last_error = Some(error); + } + } + } + Err(last_error.unwrap_or_else(|| "Failed to extract JRE archive".to_string())) +} + +fn extract_jre_archive_once(archive: &Path, dest: &Path, format: Option) -> Result<(), String> { match format { Some(ArtifactFormat::TarZstd) => { - let file = std::fs::File::open(archive).map_err(|e| format!("Failed to open JRE archive: {e}"))?; - let decoder = - zstd::stream::read::Decoder::new(file).map_err(|e| format!("Failed to open zstd JRE archive: {e}"))?; - extract_jre_tar(tar::Archive::new(decoder), dest) + let source = JreArchiveSource::TarZstd(archive); + extract_jre_tar(source.open()?, dest, source) } None => extract_tar_gz(archive, dest), } } fn extract_tar_gz(archive: &Path, dest: &Path) -> Result<(), String> { - let file = std::fs::File::open(archive).map_err(|e| format!("Failed to open JRE archive: {e}"))?; - let decoder = flate2::read::GzDecoder::new(file); - extract_jre_tar(tar::Archive::new(decoder), dest) + let source = JreArchiveSource::TarGzip(archive); + extract_jre_tar(source.open()?, dest, source) } -fn extract_jre_tar(mut archive: tar::Archive, dest: &Path) -> Result<(), String> { +/// A JRE archive on disk and the codec needed to read it. +/// +/// The path is kept next to the open stream because a failed extraction replays +/// the archive once to report the entries that never reached the disk; see +/// `diagnose_jre_extract_failure`. +#[derive(Clone, Copy)] +enum JreArchiveSource<'a> { + TarZstd(&'a Path), + TarGzip(&'a Path), +} + +impl JreArchiveSource<'_> { + fn open(&self) -> Result>, String> { + let path = match self { + Self::TarZstd(path) | Self::TarGzip(path) => *path, + }; + let file = std::fs::File::open(path) + .map_err(|error| format!("Failed to open JRE archive: {}", describe_error(&error)))?; + let reader: Box = match self { + Self::TarZstd(_) => Box::new( + zstd::stream::read::Decoder::new(file) + .map_err(|error| format!("Failed to open zstd JRE archive: {}", describe_error(&error)))?, + ), + Self::TarGzip(_) => Box::new(flate2::read::GzDecoder::new(file)), + }; + Ok(tar::Archive::new(reader)) + } +} + +fn extract_jre_tar( + mut archive: tar::Archive>, + dest: &Path, + source: JreArchiveSource<'_>, +) -> Result<(), String> { let parent = dest.parent().ok_or_else(|| format!("Invalid JRE destination: {}", dest.display()))?; - std::fs::create_dir_all(parent).map_err(|e| format!("Failed to create JRE directory: {e}"))?; + std::fs::create_dir_all(parent).map_err(|e| format!("Failed to create JRE directory: {}", describe_error(&e)))?; let staging = tempfile::Builder::new() .prefix(".jre-extract-") .tempdir_in(parent) - .map_err(|e| format!("Failed to create JRE extraction directory: {e}"))?; - archive.unpack(staging.path()).map_err(|e| format!("Failed to extract JRE archive: {e}"))?; + .map_err(|e| format!("Failed to create JRE extraction directory: {}", describe_error(&e)))?; + + // Restoring POSIX permissions and mtimes has no Windows equivalent: the + // emulation cannot express a real ACL and only adds ways for the extraction + // to fail (a file-system filter refusing attribute changes), while the + // read-only attribute it sets for 0444 entries later blocks removing a JRE + // that is being replaced. POSIX keeps the archive modes, which is what makes + // `bin/java` executable there. + if cfg!(windows) { + archive.set_preserve_permissions(false); + archive.set_preserve_mtime(false); + } + + if let Err(error) = archive.unpack(staging.path()) { + return Err(format!( + "Failed to extract JRE archive: {}{}", + describe_error(&error), + diagnose_jre_extract_failure(source, staging.path(), &error) + )); + } let mut roots = std::fs::read_dir(staging.path()) - .map_err(|e| format!("Failed to inspect extracted JRE archive: {e}"))? + .map_err(|e| format!("Failed to inspect extracted JRE archive: {}", describe_error(&e)))? .collect::, _>>() - .map_err(|e| format!("Failed to inspect extracted JRE archive: {e}"))?; + .map_err(|e| format!("Failed to inspect extracted JRE archive: {}", describe_error(&e)))?; if roots.len() != 1 { return Err("Invalid JRE archive: expected a single top-level directory".to_string()); } let root = roots.pop().expect("root count checked above"); - if !root.file_type().map_err(|e| format!("Failed to inspect extracted JRE archive: {e}"))?.is_dir() { + if !root + .file_type() + .map_err(|e| format!("Failed to inspect extracted JRE archive: {}", describe_error(&e)))? + .is_dir() + { return Err("Invalid JRE archive: expected a top-level directory".to_string()); } - std::fs::create_dir_all(dest).map_err(|e| format!("Failed to create JRE directory: {e}"))?; - for entry in std::fs::read_dir(root.path()).map_err(|e| format!("Failed to inspect extracted JRE archive: {e}"))? { - let entry = entry.map_err(|e| format!("Failed to inspect extracted JRE archive: {e}"))?; + std::fs::create_dir_all(dest).map_err(|e| format!("Failed to create JRE directory: {}", describe_error(&e)))?; + for entry in std::fs::read_dir(root.path()) + .map_err(|e| format!("Failed to inspect extracted JRE archive: {}", describe_error(&e)))? + { + let entry = entry.map_err(|e| format!("Failed to inspect extracted JRE archive: {}", describe_error(&e)))?; std::fs::rename(entry.path(), dest.join(entry.file_name())) - .map_err(|e| format!("Failed to install extracted JRE: {e}"))?; + .map_err(|e| format!("Failed to install extracted JRE: {}", describe_error(&e)))?; } Ok(()) } +/// Bytes written by the extraction probe that classifies a failed entry: +/// large enough to fail on a full or quota-limited disk, small enough to be +/// harmless. +const JRE_EXTRACT_PROBE_BYTES: usize = 64 * 1024; + +/// Explain a failed extraction in terms a support engineer can act on. +/// +/// `tar` names the entry it failed on and nothing else, so the same message +/// covered a full disk, a permission problem and a security product refusing a +/// single file name. JRE archives ship ~30 `api-ms-win-*.dll` API-set stubs in +/// `bin/`; droppers plant exactly that name for DLL hijacking, so endpoint +/// security products are known to reject it — and closing the anti-virus UI does +/// not remove the file-system filter behind it. Replaying the archive locates +/// the entries that never reached the disk, and probing the failing folder +/// separates "this folder is not writable" from "this file name is not +/// writable". +fn diagnose_jre_extract_failure(source: JreArchiveSource<'_>, staging: &Path, error: &dyn std::error::Error) -> String { + let mut diagnosis = String::new(); + if let Some(hint) = deepest_io_error(error).and_then(|error| describe_extract_os_error(error.raw_os_error())) { + diagnosis.push_str(&format!("\nThe OS error means {hint}.")); + } + + let store_root = staging.parent().unwrap_or(staging); + match locate_unwritten_entries(source, staging) { + Err(replay_error) => { + diagnosis.push_str(&format!( + "\nExtraction check: the archive could not be replayed ({replay_error}), so the archive itself is \ + damaged or incomplete rather than the destination folder." + )); + } + Ok(scan) => match (scan.first_failed, scan.first_failed_path) { + (Some(name), Some(path)) => { + diagnosis.push_str(&format!( + "\nExtraction check: {} file(s) are missing or incomplete; the first is `{name}`. {}", + scan.failed_count, + describe_unwritten_entry(&path, store_root, scan.first_failed_incomplete) + )); + } + _ if scan.entry_count > 0 => { + diagnosis.push_str( + "\nExtraction check: every file entry of the archive is on disk, so the failure came from a \ + directory or link entry, or from moving the extracted JRE into place.", + ); + } + _ => {} + }, + } + diagnosis +} + +/// Which entries a failed extraction left behind, from replaying the archive. +#[derive(Default)] +struct JreArchiveScan { + /// Archive-relative name of the first entry that is missing or incomplete. + first_failed: Option, + /// Destination path of that entry. + first_failed_path: Option, + /// Set when that entry exists but is shorter than the archive says. + first_failed_incomplete: bool, + failed_count: usize, + entry_count: usize, +} + +impl JreArchiveScan { + fn record_failure(&mut self, relative: PathBuf, destination: PathBuf, incomplete: bool) { + self.failed_count += 1; + if self.first_failed.is_none() { + self.first_failed = Some(relative.to_string_lossy().replace('\\', "/")); + self.first_failed_path = Some(destination); + self.first_failed_incomplete = incomplete; + } + } +} + +/// Replay the archive and report the file entries whose destination is missing +/// or shorter than the archive says. Directories, links and metadata entries are +/// skipped: only regular files carry a size that can be compared. +fn locate_unwritten_entries(source: JreArchiveSource<'_>, staging: &Path) -> Result { + let mut archive = source.open()?; + let mut scan = JreArchiveScan::default(); + let mut entries = archive.entries().map_err(|error| describe_error(&error))?; + for entry in &mut entries { + let entry = entry.map_err(|error| describe_error(&error))?; + if !entry.header().entry_type().is_file() { + continue; + } + let mut relative = PathBuf::new(); + for component in entry.path().map_err(|error| describe_error(&error))?.components() { + match component { + std::path::Component::Normal(value) => relative.push(value), + std::path::Component::CurDir => {} + _ => return Err(format!("unsafe entry path: {}", relative.display())), + } + } + if relative.as_os_str().is_empty() { + continue; + } + scan.entry_count += 1; + let destination = staging.join(&relative); + match std::fs::metadata(&destination).map(|metadata| metadata.len()) { + Ok(length) if length == entry.size() => {} + Ok(_) => scan.record_failure(relative, destination, true), + Err(_) => scan.record_failure(relative, destination, false), + } + } + Ok(scan) +} + +fn describe_unwritten_entry(path: &Path, store_root: &Path, incomplete: bool) -> String { + let directory = path.parent().unwrap_or(store_root); + if incomplete { + return format!( + "The entry was created but is shorter than the archive says, so the write itself was interrupted — check \ + the free disk space and any anti-virus, backup or quota filter for `{}`.", + directory.display() + ); + } + classify_extract_blocker(directory, path.file_name(), store_root) +} + +/// Tell apart the reasons a file cannot be created in the export directory by +/// creating probes there: one with a random name, one with the failed name. +fn classify_extract_blocker(directory: &Path, name: Option<&std::ffi::OsStr>, store_root: &Path) -> String { + let random_probe = probe_destination_write(directory, None); + let named_probe = name.map(|name| probe_destination_write(directory, Some(name))); + describe_extract_blocker(directory, store_root, random_probe, named_probe) +} + +fn describe_extract_blocker( + directory: &Path, + store_root: &Path, + random_probe: std::io::Result<()>, + named_probe: Option>, +) -> String { + match (random_probe, named_probe) { + (Err(error), _) => format!( + "Creating a test file in `{}` failed the same way ({error}), so the folder is not writable — check its \ + permissions, the free disk space and any anti-virus, backup or quota filter.", + directory.display() + ), + (Ok(()), Some(Err(error))) if error.kind() == std::io::ErrorKind::AlreadyExists => format!( + "A test file can be created in `{}` and the entry exists now, so the failure looks transient — retry the \ + import.", + directory.display() + ), + (Ok(()), Some(Err(_))) => format!( + "A differently named test file could be created in `{}`, but creating that exact file name failed the \ + same way, so the file name itself is rejected on this machine — usually an endpoint-security file \ + filter or a group policy. Whitelist `{}` or the dbx process and retry; closing the anti-virus UI alone \ + does not remove such a filter.", + directory.display(), + store_root.display() + ), + _ => format!( + "Test files can be created in `{}` now, so the failure looks transient (a scanner, indexer or backup agent \ + holding the file) — retry the import.", + directory.display() + ), + } +} + +fn probe_destination_write(directory: &Path, name: Option<&std::ffi::OsStr>) -> std::io::Result<()> { + match name { + Some(name) => { + let probe = directory.join(name); + let result = std::fs::OpenOptions::new().write(true).create_new(true).open(&probe).map(|_| ()); + if result.is_ok() { + let _ = std::fs::remove_file(&probe); + } + result + } + None => { + let probe = directory.join(format!(".dbx-extract-probe-{}", uuid::Uuid::new_v4())); + let result = std::fs::File::create(&probe) + .and_then(|mut file| std::io::Write::write_all(&mut file, &[0u8; JRE_EXTRACT_PROBE_BYTES])); + let _ = std::fs::remove_file(&probe); + result + } + } +} + +/// Explain the well-known OS errors an extraction can hit, so support does not +/// have to translate `os error 5` by hand. +fn describe_extract_os_error(code: Option) -> Option<&'static str> { + let code = code?; + #[cfg(windows)] + { + match code { + 112 => Some("the destination disk is full or the quota is exhausted"), + 5 => Some("the file or folder is blocked by permissions, a security product or a group policy"), + 32 | 33 => Some("another process is holding the file (a virus scanner, indexer or backup agent)"), + 206 => Some("the path is longer than the file system allows"), + 225 => Some("a security product rejected the file while it was written"), + 226 => Some("a security product deleted the file right after it was written"), + _ => None, + } + } + #[cfg(not(windows))] + { + match code { + 28 => Some("the destination disk is full or the quota is exhausted"), + 13 => Some("the file or folder is blocked by permissions, a security product or a group policy"), + 11 | 16 => Some("another process is holding the file (a virus scanner, indexer or backup agent)"), + 63 => Some("the path is longer than the file system allows"), + _ => None, + } + } +} + #[cfg(test)] mod jre_archive_tests { use super::*; @@ -3361,6 +3739,166 @@ mod jre_archive_tests { let error = extract_tar_gz(&archive_path, &temp.path().join("managed-jre")).unwrap_err(); assert!(error.contains("single top-level directory"), "unexpected error: {error}"); } + + /// `tar` prints only its own description and keeps the OS error one level + /// deeper in the source chain, which is the shape that hid Windows + /// anti-virus and disk failures from the offline-import UI. + #[derive(Debug)] + struct ArchiveLikeError { + description: String, + io: std::io::Error, + } + + impl std::fmt::Display for ArchiveLikeError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str(&self.description) + } + } + + impl std::error::Error for ArchiveLikeError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.io) + } + } + + /// A failed extraction must name the entry that never reached the disk, not + /// just the `tar` description that stopped at the first bad write. + #[test] + fn reports_the_entry_that_never_reached_the_disk() { + let temp = tempfile::tempdir().unwrap(); + let archive_path = temp.path().join("jre.tar.gz"); + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(&archive_path).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + // The second entry cannot be created because its parent is a file. + append_file(&mut builder, "jdk-21/bin", b"not a directory", 0o644); + append_file(&mut builder, "jdk-21/bin/java", b"java", 0o755); + builder.into_inner().unwrap().finish().unwrap(); + + let error = extract_tar_gz(&archive_path, &temp.path().join("managed-jre")).unwrap_err(); + + assert!(error.starts_with("Failed to extract JRE archive:"), "unexpected error: {error}"); + assert!(error.contains("`jdk-21/bin/java`"), "unexpected error: {error}"); + assert!(error.contains("1 file(s) are missing or incomplete"), "unexpected error: {error}"); + assert!(error.contains("Extraction check:"), "unexpected error: {error}"); + } + + #[test] + fn reports_a_damaged_archive_instead_of_a_destination_problem() { + let temp = tempfile::tempdir().unwrap(); + let archive_path = temp.path().join("jre.tar.gz"); + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(&archive_path).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + append_file(&mut builder, "jdk-21/lib/modules", &vec![b'x'; 512 * 1024], 0o644); + builder.into_inner().unwrap().finish().unwrap(); + let full = std::fs::read(&archive_path).unwrap(); + std::fs::write(&archive_path, &full[..full.len() / 2]).unwrap(); + + let error = extract_tar_gz(&archive_path, &temp.path().join("managed-jre")).unwrap_err(); + + assert!(error.contains("could not be replayed"), "unexpected error: {error}"); + assert!(error.contains("damaged or incomplete"), "unexpected error: {error}"); + } + + #[test] + fn classifies_a_rejected_file_name_as_a_security_filter() { + let temp = tempfile::tempdir().unwrap(); + let denied = std::io::Error::from_raw_os_error(if cfg!(windows) { 5 } else { 13 }); + + let message = describe_extract_blocker(&temp.path().join("bin"), temp.path(), Ok(()), Some(Err(denied))); + + assert!(message.contains("file name itself is rejected"), "unexpected message: {message}"); + assert!(message.contains(&temp.path().display().to_string()), "unexpected message: {message}"); + } + + #[test] + fn classifies_an_existing_entry_as_transient() { + let temp = tempfile::tempdir().unwrap(); + let existing = std::io::Error::from(std::io::ErrorKind::AlreadyExists); + + let message = describe_extract_blocker(&temp.path().join("bin"), temp.path(), Ok(()), Some(Err(existing))); + + assert!(message.contains("transient"), "unexpected message: {message}"); + } + + #[cfg(unix)] + #[test] + fn classifies_an_unwritable_folder() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().unwrap(); + let directory = temp.path().join("bin"); + std::fs::create_dir(&directory).unwrap(); + std::fs::set_permissions(&directory, std::fs::Permissions::from_mode(0o500)).unwrap(); + + let message = classify_extract_blocker(&directory, Some(std::ffi::OsStr::new("java")), temp.path()); + + std::fs::set_permissions(&directory, std::fs::Permissions::from_mode(0o755)).unwrap(); + assert!(message.contains("folder is not writable"), "unexpected message: {message}"); + } + + #[test] + fn classifies_a_writable_folder_as_transient_and_cleans_up_the_probes() { + let temp = tempfile::tempdir().unwrap(); + let directory = temp.path().join("bin"); + std::fs::create_dir(&directory).unwrap(); + + let message = classify_extract_blocker(&directory, Some(std::ffi::OsStr::new("java")), temp.path()); + + assert!(message.contains("transient"), "unexpected message: {message}"); + assert_eq!(std::fs::read_dir(&directory).unwrap().count(), 0, "probe files must not be left behind"); + } + + #[test] + fn explains_well_known_os_errors() { + let disk_full = if cfg!(windows) { 112 } else { 28 }; + let denied = if cfg!(windows) { 5 } else { 13 }; + + assert!(describe_extract_os_error(Some(disk_full)).unwrap().contains("disk is full")); + assert!(describe_extract_os_error(Some(denied)).unwrap().contains("permissions")); + assert_eq!(describe_extract_os_error(Some(0)), None); + assert_eq!(describe_extract_os_error(None), None); + + // A security product that blocks the write itself reports these + // Windows codes; POSIX has no counterpart. + if cfg!(windows) { + assert!(describe_extract_os_error(Some(225)).unwrap().contains("rejected the file")); + assert!(describe_extract_os_error(Some(226)).unwrap().contains("deleted the file")); + } else { + assert_eq!(describe_extract_os_error(Some(225)), None); + assert_eq!(describe_extract_os_error(Some(226)), None); + } + } + + #[test] + fn describes_the_os_error_behind_an_archive_message() { + let error = std::io::Error::other(ArchiveLikeError { + description: "failed to unpack `dbx-jre/bin/api-ms-win-core-string-l1-1-0.dll`".to_string(), + io: std::io::Error::from_raw_os_error(5), + }); + + let message = describe_error(&error); + + assert!( + message.starts_with("failed to unpack `dbx-jre/bin/api-ms-win-core-string-l1-1-0.dll`"), + "unexpected message: {message}" + ); + assert!(message.contains("os error 5"), "unexpected message: {message}"); + } + + #[test] + fn describes_a_message_that_already_carries_the_os_error_unchanged() { + let os_error = std::io::Error::from_raw_os_error(5); + let expected = os_error.to_string(); + let error = std::io::Error::other(os_error); + + assert_eq!(describe_error(&error), expected); + } } pub async fn import_agent_driver(am: &AgentManager, db_type: &str, source_path: &Path) -> Result<(), String> { diff --git a/crates/dbx-web/src/routes/agents.rs b/crates/dbx-web/src/routes/agents.rs index 90fcd00017..15002e6d4e 100644 --- a/crates/dbx-web/src/routes/agents.rs +++ b/crates/dbx-web/src/routes/agents.rs @@ -307,6 +307,7 @@ pub async fn import_agents_from_zip( return Ok(Json(serde_json::json!({ "count": result.drivers_installed.len() as u32, "jreCount": result.jre_installed.len(), + "failures": result.failures, }))); } diff --git a/src-tauri/src/commands/agents.rs b/src-tauri/src/commands/agents.rs index 674ce11948..51973d5d36 100644 --- a/src-tauri/src/commands/agents.rs +++ b/src-tauri/src/commands/agents.rs @@ -233,7 +233,11 @@ pub async fn import_agents_from_zip( dbx_core::jdbc::import_offline_jdbc_payload(state.plugins.root_dir(), &package_path)?; let count = result.drivers_installed.len() as u32; emit_agent_progress(&app, &operation_id, AgentProgressEvent::step("done")); - Ok(serde_json::json!({ "count": count, "jreCount": result.jre_installed.len() })) + Ok(serde_json::json!({ + "count": count, + "jreCount": result.jre_installed.len(), + "failures": result.failures, + })) } #[tauri::command]