diff --git a/.gitignore b/.gitignore index e65995d..70baaff 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ # testing /coverage /e2e/app/target/ +/e2e/app/Cargo.lock /e2e/.driver/ # next.js diff --git a/AGENTS.md b/AGENTS.md index 12366c5..0e5fb2a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,10 @@ into the ignored `e2e/.driver` root) and launch an `e2e`-feature build. Every session gets its own temporary Donut data/cache/log root, home directory, WebView store, ports, and sync bucket. Never point a suite at production or development data. +`e2e/app/Cargo.lock` is generated, gitignored, and never edited by hand. `e2e/run.mjs` seeds it +from `src-tauri/Cargo.lock` whenever that file is newer, so the harness always links the exact +dependency versions Donut ships and a version bump or a Dependabot upgrade needs no second edit. + After a behavior change, run the smallest affected subset below in addition to the standard format/lint/unit-test command. A code change is not verified until its affected native suite passes: diff --git a/e2e/app/Cargo.lock b/e2e/app/Cargo.lock index 65abc47..5f3fee0 100644 --- a/e2e/app/Cargo.lock +++ b/e2e/app/Cargo.lock @@ -572,9 +572,9 @@ checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" [[package]] name = "base64" -version = "0.23.0" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b25655df2c3cdd83c5e5b293b88acd880332b2ddadd7c30ac43144fdc0033da9" +checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" [[package]] name = "base64ct" @@ -1046,9 +1046,9 @@ dependencies = [ [[package]] name = "cfg-expr" -version = "0.20.8" +version = "0.20.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb693542bcafa528e198be0ebd9d3632ca5b7c93dbe7237460e199910835997c" +checksum = "fe4ece8474b5f766c63426647e7b4b316b67431ade1036a8313cee24a03ae917" dependencies = [ "smallvec", "target-lexicon 0.13.5", @@ -1149,9 +1149,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.6.4" +version = "4.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d91e0c145792ef73a6ad36d27c75ac09f1832222a3c209689d90f534685ee5b7" +checksum = "473c7e07f409a8d772161724aa8db6a765a2532a70f9667eeb7b49d3d02fbdca" dependencies = [ "clap_builder", "clap_derive", @@ -1159,9 +1159,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.6.2" +version = "4.6.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09628afdcc538b57f3c6341e9c8e9970f18e4a481690a64974d7023bd33548b" +checksum = "7b48fea5a88e9ae728a2dcbedbfc0e730f7d60da42e1cb049a83c9fb8b789889" dependencies = [ "anstream", "anstyle", @@ -1178,7 +1178,7 @@ dependencies = [ "heck 0.5.0", "proc-macro2", "quote", - "syn 3.0.3", + "syn 3.0.4", ] [[package]] @@ -2391,9 +2391,9 @@ dependencies = [ [[package]] name = "futures-channel" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "262590f4fe6afeb0bc83be1daa64e52657fe185690a958af7f3ad0e92085c5ae" +checksum = "b1f9e3d69d39e4862ffed03ed071a76f9a13ba1d9109d355b0f0aa6b15e393c4" dependencies = [ "futures-core", "futures-sink", @@ -2401,9 +2401,9 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" +checksum = "92d699e522242e69e3003b94ecc1f960f3a5e015aa7c5d7486e65ad01dd94f5e" [[package]] name = "futures-executor" @@ -2418,9 +2418,9 @@ dependencies = [ [[package]] name = "futures-io" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4577ecaa3c4f96589d473f679a71b596316f6641bc350038b962a5daf0085d7a" +checksum = "53c0fa8157de1303bfffdaa1cc2a673bfffb60102f76b0ef4441659124373fed" [[package]] name = "futures-lite" @@ -2437,32 +2437,32 @@ dependencies = [ [[package]] name = "futures-macro" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d6d3cde68c518367be28956066ddfef33813991b77a55005a69dae04bf3b10b" +checksum = "9fb9654ba8355388abeb8dcb4fc62f511300867002afc858860463bdd9fe0c44" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 3.0.4", ] [[package]] name = "futures-sink" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e34418ac499d6305c2fb5ad0ed2f6ac998c5f8ca209b4510f7f94242c647e307" +checksum = "1944426bf7d03f1d14f708785e4b33efd750b36d48a157b836b3efc15ede8e1d" [[package]] name = "futures-task" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" +checksum = "cd417de3d1d015fc3bfd2b1ea46dfc7bab72ef86f1cc7cc9c78e728b34a6d1fd" [[package]] name = "futures-util" -version = "0.3.33" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" +checksum = "0d50a92467f8ba5dd6e3ee5d4bd04d73ab2e4e1c44474a0674821dfce14b79bc" dependencies = [ "futures-channel", "futures-core", @@ -6443,9 +6443,9 @@ dependencies = [ [[package]] name = "syn" -version = "3.0.3" +version = "3.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +checksum = "e6275cddf4610d1775e6d1fe9469b2e77d0f39fd98fb7450901b821e0c53649f" dependencies = [ "proc-macro2", "quote", @@ -6536,7 +6536,7 @@ version = "7.0.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "396a35feb67335377e0251fcbc1092fc85c484bd4e3a7a54319399da127796e7" dependencies = [ - "cfg-expr 0.20.8", + "cfg-expr 0.20.9", "heck 0.5.0", "pkg-config", "toml 1.1.2+spec-1.1.0", @@ -7038,7 +7038,7 @@ checksum = "535cd782aac407a0bbf593013306e89e821017e3862ddd7b36925aa80a29e649" dependencies = [ "async-trait", "axum", - "base64 0.23.0", + "base64 0.23.1", "block2", "cairo-rs", "clap", diff --git a/e2e/coverage-map.mjs b/e2e/coverage-map.mjs index ac3f86b..f560f9a 100644 --- a/e2e/coverage-map.mjs +++ b/e2e/coverage-map.mjs @@ -212,6 +212,7 @@ export const commandCoverage = { commands: [ "get_sync_settings", "save_sync_settings", + "check_sync_server_connection", "cloud_auth::restart_sync_service", "set_profile_sync_mode", "cancel_profile_sync", diff --git a/e2e/run.mjs b/e2e/run.mjs index 71fedbb..8e0a6ac 100644 --- a/e2e/run.mjs +++ b/e2e/run.mjs @@ -2,6 +2,7 @@ import { spawn, spawnSync } from "node:child_process"; import { + copyFileSync, createReadStream, createWriteStream, existsSync, @@ -44,6 +45,9 @@ const driverBinary = path.join( "bin", `tauri-wd${executableSuffix}`, ); +const appManifest = path.join(appManifestDir, "Cargo.toml"); +const appLockfile = path.join(appManifestDir, "Cargo.lock"); +const donutLockfile = path.join(projectRoot, "src-tauri", "Cargo.lock"); const suiteFiles = { smoke: ["diagnostics.test.mjs", "smoke.test.mjs", "coverage.test.mjs"], @@ -237,16 +241,13 @@ async function loadLocalValues(names) { return values; } -function lockedDriverVersion() { - const lockfile = readFileSync( - path.join(appManifestDir, "Cargo.lock"), - "utf8", - ); - const match = lockfile.match( - /\[\[package\]\]\s*\nname = "tauri-wd"\s*\nversion = "([^"]+)"/, - ); +function pinnedDriverVersion() { + const manifest = readFileSync(appManifest, "utf8"); + const match = manifest.match(/^tauri-wd\s*=\s*"=([^"]+)"$/m); if (!match) { - throw new Error("e2e/app/Cargo.lock does not resolve a tauri-wd version"); + throw new Error( + 'e2e/app/Cargo.toml must pin tauri-wd to an exact version, e.g. tauri-wd = "=0.1.11"', + ); } return match[1]; } @@ -263,7 +264,7 @@ function installedDriverVersion() { } function ensureDriver() { - const version = lockedDriverVersion(); + const version = pinnedDriverVersion(); if (installedDriverVersion() === version) { log(`tauri-wd ${version} already installed at ${driverBinary}`); return; @@ -285,15 +286,27 @@ function ensureDriver() { ); } +// The harness links the Donut crate, so it has to resolve the same versions +// Donut itself ships. Seeding the harness lockfile from src-tauri/Cargo.lock +// keeps the two in step whenever a dependency or the app version moves; cargo +// fills in the harness-only packages on top. It is generated, never hand-edited. +function syncHarnessLockfile() { + if ( + existsSync(appLockfile) && + statSync(appLockfile).mtimeMs >= statSync(donutLockfile).mtimeMs + ) { + return; + } + copyFileSync(donutLockfile, appLockfile); + log("seeded e2e/app/Cargo.lock from src-tauri/Cargo.lock"); +} + function buildAll() { run("pnpm", ["build"], projectRoot); run("pnpm", ["copy-proxy-binary"], projectRoot); run(process.execPath, ["src-tauri/download-xray.mjs"], projectRoot); - run( - "cargo", - ["build", "--locked", "--manifest-path", "e2e/app/Cargo.toml"], - projectRoot, - ); + syncHarnessLockfile(); + run("cargo", ["build", "--manifest-path", "e2e/app/Cargo.toml"], projectRoot); ensureDriver(); } diff --git a/e2e/tests/sync.test.mjs b/e2e/tests/sync.test.mjs index 2259dd6..a845367 100644 --- a/e2e/tests/sync.test.mjs +++ b/e2e/tests/sync.test.mjs @@ -1,5 +1,6 @@ import assert from "node:assert/strict"; import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; import path from "node:path"; import test from "node:test"; import { appFromEnvironment } from "../lib/app.mjs"; @@ -575,3 +576,85 @@ test("global config sealing and encrypted profile sync reject a wrong password, ]); } }); + +// A self-hosted server reaches its storage over an address only it can +// resolve — the documented compose file points S3_ENDPOINT at +// http://minio:9000, a Docker service name that exists on the compose network +// and nowhere else. Files never travel through the sync server, so every +// presigned URL then names a host the desktop cannot open: /health and /readyz +// stay green while every single transfer dies at connect. Reported as "the +// endpoint connection works every time, but no MB is ever synced". +test("the connection check fails a server whose storage host this device cannot reach", async () => { + assert.ok(syncUrl && syncToken, "Sync infrastructure was not started"); + const app = appFromEnvironment("sync-preflight"); + + // Answers exactly like a healthy self-hosted server that signs presigned + // URLs against a container-only host. + const misconfigured = createServer((request, response) => { + if (request.url === "/readyz") { + response.writeHead(200, { "content-type": "application/json" }); + response.end( + JSON.stringify({ + status: "ready", + s3: true, + storageEndpoint: "http://minio.invalid:9000", + }), + ); + return; + } + response.writeHead(404); + response.end(); + }); + await new Promise((resolve) => misconfigured.listen(0, "127.0.0.1", resolve)); + const misconfiguredUrl = `http://127.0.0.1:${misconfigured.address().port}`; + + try { + await app.start(); + + const healthy = await app.invoke("check_sync_server_connection", { + serverUrl: syncUrl, + }); + assert.equal(healthy.server_reachable, true, "real sync server answers"); + assert.notEqual( + healthy.storage_reachable, + false, + "the suite's own storage must be reachable from the test device", + ); + + // The regression itself: green server, storage nobody here can open. + const broken = await app.invoke("check_sync_server_connection", { + serverUrl: misconfiguredUrl, + }); + assert.equal(broken.server_reachable, true, "server itself answered"); + assert.equal(broken.storage_ready, true, "server reaches its own storage"); + assert.equal(broken.storage_endpoint, "http://minio.invalid:9000"); + assert.equal( + broken.storage_reachable, + false, + "an unreachable storage host must not report as a working connection", + ); + assert.ok( + broken.storage_error && broken.storage_error.length > 0, + "the failure must carry a cause", + ); + assert.notEqual( + broken.storage_error, + "error sending request", + "the cause must name the transport failure, not the bare reqwest text", + ); + + // A server that does not answer at all stays a plain connection failure, + // so the two are never confused in the UI. + const dead = await app.invoke("check_sync_server_connection", { + serverUrl: "http://127.0.0.1:1", + }); + assert.equal(dead.server_reachable, false); + assert.equal(dead.storage_reachable, null); + } catch (error) { + await app.capture("failure"); + throw error; + } finally { + await new Promise((resolve) => misconfigured.close(resolve)); + await app.close(); + } +}); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 945e284..0255bb7 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -157,12 +157,12 @@ use settings_manager::{ }; use sync::{ - cancel_profile_sync, check_has_e2e_password, delete_e2e_password, enable_sync_for_all_entities, - get_unsynced_entity_counts, is_group_in_use_by_synced_profile, is_proxy_in_use_by_synced_profile, - is_vpn_in_use_by_synced_profile, request_profile_sync, rollover_encryption_for_all_entities, - set_e2e_password, set_extension_group_sync_enabled, set_extension_sync_enabled, - set_group_sync_enabled, set_profile_sync_mode, set_proxy_sync_enabled, set_vpn_sync_enabled, - verify_e2e_password, + cancel_profile_sync, check_has_e2e_password, check_sync_server_connection, delete_e2e_password, + enable_sync_for_all_entities, get_unsynced_entity_counts, is_group_in_use_by_synced_profile, + is_proxy_in_use_by_synced_profile, is_vpn_in_use_by_synced_profile, request_profile_sync, + rollover_encryption_for_all_entities, set_e2e_password, set_extension_group_sync_enabled, + set_extension_sync_enabled, set_group_sync_enabled, set_profile_sync_mode, + set_proxy_sync_enabled, set_vpn_sync_enabled, verify_e2e_password, }; use tag_manager::get_all_tags; @@ -2799,6 +2799,7 @@ pub fn run_with_builder( validate_vless_uri, get_sync_settings, save_sync_settings, + check_sync_server_connection, set_profile_sync_mode, cancel_profile_sync, request_profile_sync, diff --git a/src-tauri/src/sync/client.rs b/src-tauri/src/sync/client.rs index 388b2e3..954355f 100644 --- a/src-tauri/src/sync/client.rs +++ b/src-tauri/src/sync/client.rs @@ -234,10 +234,15 @@ impl SyncClient { } } + // The storage host here comes from the presigned URL, so on a self-hosted + // server it is whatever the server signed against — frequently an address + // only the server can resolve. `reqwest`'s own Display collapses that to + // "error sending request", which is why this failure used to be + // undiagnosable; report the innermost cause instead. let response = req .send() .await - .map_err(|e| SyncError::NetworkError(e.to_string()))?; + .map_err(|e| SyncError::NetworkError(super::preflight::transport_reason(&e)))?; if !response.status().is_success() { let status = response.status(); @@ -256,7 +261,7 @@ impl SyncClient { .get(presigned_url) .send() .await - .map_err(|e| SyncError::NetworkError(e.to_string()))?; + .map_err(|e| SyncError::NetworkError(super::preflight::transport_reason(&e)))?; if !response.status().is_success() { return Err(SyncError::NetworkError(format!( diff --git a/src-tauri/src/sync/engine.rs b/src-tauri/src/sync/engine.rs index 897b7c5..ca3a889 100644 --- a/src-tauri/src/sync/engine.rs +++ b/src-tauri/src/sync/engine.rs @@ -134,12 +134,41 @@ fn critical_failure_message(action: &str, failures: &[(String, String)]) -> Stri match failures.first() { Some((_, cause)) => format!( - "Critical files failed to {action}: {files}. Cause: {cause}. Sync aborted to prevent data loss." + "Critical files failed to {action}: {files}. Cause: {cause}.{hint} Sync aborted to prevent data loss.", + hint = storage_endpoint_hint(cause) ), None => format!("Critical files failed to {action}: {files}. Sync aborted to prevent data loss."), } } +/// The one fix worth naming when every transfer dies at connect. +/// +/// Transfers go straight to the storage host named in the presigned URL, not +/// through the sync server, so a self-hosted server that signs URLs against an +/// address only it can resolve fails every file here while its own `/health` +/// and `/readyz` stay green. The cause string already carries the host; without +/// this line it still reads as an unexplained network fault, and the setting +/// that fixes it lives on the server, where the user is not looking. +fn storage_endpoint_hint(cause: &str) -> String { + let lowered = cause.to_ascii_lowercase(); + let is_transport_failure = [ + "connection failed", + "timed out", + "dns", + "error sending request", + ] + .iter() + .any(|marker| lowered.contains(marker)); + + if is_transport_failure { + " The storage host in the presigned URL could not be reached from this device. \ + On a self-hosted server, set S3_PUBLIC_ENDPOINT to an address this device can reach." + .to_string() + } else { + String::new() + } +} + /// Validate that a manifest-supplied relative file path is safe to join onto a /// profile directory before writing/deleting. The manifest is remote-controlled /// (a self-hosted or compromised sync server, a MITM on a plaintext Regular-mode @@ -4326,6 +4355,43 @@ mod tests { assert!(message.contains("failed to download")); } + #[test] + fn test_critical_failure_message_names_the_storage_endpoint_fix() { + // A self-hosted server that signs presigned URLs against a container-only + // host fails every transfer at connect while the server itself looks + // healthy. Naming the file and the socket error is not enough to find the + // setting that fixes it. + let failures = vec![( + "Default/Cookies".to_string(), + "Failed to upload Default/Cookies after 3 retries: connection failed: \ + failed to lookup address information for minio" + .to_string(), + )]; + + let message = critical_failure_message("upload", &failures); + assert!(message.contains("S3_PUBLIC_ENDPOINT"), "{message}"); + assert!(message.contains("could not be reached from this device")); + assert!(message.contains("Sync aborted to prevent data loss.")); + } + + #[test] + fn test_critical_failure_message_omits_the_hint_for_non_transport_causes() { + // A rejected signature or a full disk is not a routing problem, and + // pointing those users at S3_PUBLIC_ENDPOINT sends them the wrong way. + for cause in [ + "Upload failed with status 403: SignatureDoesNotMatch", + "Upload failed with status 507: quota exceeded", + "No space left on device", + ] { + let failures = vec![("Default/Cookies".to_string(), cause.to_string())]; + let message = critical_failure_message("upload", &failures); + assert!( + !message.contains("S3_PUBLIC_ENDPOINT"), + "hint must not fire for: {cause}" + ); + } + } + #[test] fn test_is_safe_manifest_path() { // Legitimate profile-relative paths are accepted. diff --git a/src-tauri/src/sync/mod.rs b/src-tauri/src/sync/mod.rs index d277b40..b8482af 100644 --- a/src-tauri/src/sync/mod.rs +++ b/src-tauri/src/sync/mod.rs @@ -2,6 +2,7 @@ mod client; pub mod encryption; mod engine; pub mod manifest; +pub mod preflight; pub mod scheduler; pub mod subscription; pub mod types; @@ -25,6 +26,7 @@ pub use manifest::{ compute_diff, compute_diff_with_bias, generate_manifest, DiffBias, HashCache, ManifestDiff, SyncManifest, }; +pub use preflight::{check_sync_server, check_sync_server_connection, SyncServerCheck}; pub use scheduler::{get_global_scheduler, set_global_scheduler, SyncScheduler}; pub use subscription::{SubscriptionManager, SyncWorkItem}; pub use types::{SyncError, SyncResult}; diff --git a/src-tauri/src/sync/preflight.rs b/src-tauri/src/sync/preflight.rs new file mode 100644 index 0000000..48cb64a --- /dev/null +++ b/src-tauri/src/sync/preflight.rs @@ -0,0 +1,263 @@ +//! Pre-flight check for a sync server, run from the network stack that +//! actually performs transfers. +//! +//! A self-hosted server almost always reaches its storage over an address only +//! it can resolve: the documented compose file points `S3_ENDPOINT` at +//! `http://minio:9000`, a Docker service name that exists on the compose +//! network and nowhere else. Presigned URLs are signed against the host they +//! name, so every URL handed to this device names a host it cannot open. The +//! server is healthy, `/health` and `/readyz` are green, and every single file +//! transfer fails at connect. +//! +//! Checking the server alone is what let that configuration look correct. This +//! module also opens the storage host the server says it hands out, from here, +//! with the same client the uploader uses, so the break is named at the moment +//! the user configures sync instead of after the first sync fails. + +use serde::{Deserialize, Serialize}; +use std::time::Duration; + +/// Both probes are liveness questions, not transfers, so they must fail fast +/// rather than sit on a connect that is never going to answer. +const PROBE_TIMEOUT: Duration = Duration::from_secs(8); + +/// What a pre-flight found. Every field is reported rather than collapsed into +/// one boolean: "the server answers but its storage is unreachable from here" +/// is a different problem with a different fix than "the server is down", and +/// the UI has to be able to say which one happened. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +pub struct SyncServerCheck { + /// The sync server itself answered. + pub server_reachable: bool, + /// The server reports it can reach its own storage. `None` when the server + /// is too old to serve `/readyz`, which is a working server, not a broken + /// one. + pub storage_ready: Option, + /// The host the server signs into presigned URLs, when it discloses one. + /// Withheld by cloud deployments on purpose. + pub storage_endpoint: Option, + /// Whether that host answered *this device*. `None` when there was nothing + /// to probe. + pub storage_reachable: Option, + /// Why the storage probe failed, for the log and the error surface. + pub storage_error: Option, +} + +impl SyncServerCheck { + /// Whether sync can actually move bytes. A green server with an unreachable + /// storage host is the exact state this check exists to stop reporting as + /// success. + pub fn is_usable(&self) -> bool { + self.server_reachable + && self.storage_ready != Some(false) + && self.storage_reachable != Some(false) + } +} + +/// The `/readyz` body. Every field is optional: older servers answer `/health` +/// only, and cloud deployments withhold `storageEndpoint`. +#[derive(Debug, Deserialize)] +struct ReadyzBody { + #[serde(default)] + s3: Option, + #[serde(default, rename = "storageEndpoint")] + storage_endpoint: Option, +} + +fn probe_client() -> reqwest::Client { + // Matches how `SyncClient` builds its client, so a TLS trust or proxy + // condition that would fail an upload fails the probe the same way. A probe + // that is more permissive than the uploader would report a working setup for + // a configuration that cannot transfer. + reqwest::Client::builder() + .timeout(PROBE_TIMEOUT) + .build() + .unwrap_or_default() +} + +/// Ask the sync server about itself, then verify the storage host it names. +pub async fn check_sync_server(server_url: &str) -> SyncServerCheck { + let base = server_url.trim().trim_end_matches('/'); + if base.is_empty() { + return SyncServerCheck::default(); + } + + let client = probe_client(); + let mut check = SyncServerCheck::default(); + + let readyz = match client.get(format!("{base}/readyz")).send().await { + Ok(response) => response, + Err(e) => { + log::warn!("Sync pre-flight: {base}/readyz did not answer: {e}"); + return check; + } + }; + + if readyz.status() == reqwest::StatusCode::NOT_FOUND { + // Predates /readyz. It is still a working server, so fall back rather than + // failing a healthy setup, and leave the storage fields unknown. + check.server_reachable = matches!( + client.get(format!("{base}/health")).send().await, + Ok(health) if health.status().is_success() + ); + return check; + } + + // A 503 from /readyz is the server telling us its storage is down. That is a + // reachable server with a real diagnosis in the body, so read it rather than + // discarding it as a failed request. + check.server_reachable = readyz.status().is_success() || readyz.status().as_u16() == 503; + if !check.server_reachable { + return check; + } + + let body = readyz.json::().await.ok(); + check.storage_ready = body.as_ref().and_then(|b| b.s3); + check.storage_endpoint = body.and_then(|b| b.storage_endpoint); + + if let Some(endpoint) = check.storage_endpoint.clone() { + match probe_storage_endpoint(&client, &endpoint).await { + Ok(()) => check.storage_reachable = Some(true), + Err(e) => { + log::warn!("Sync pre-flight: storage endpoint {endpoint} is unreachable from here: {e}"); + check.storage_reachable = Some(false); + check.storage_error = Some(e); + } + } + } + + check +} + +/// Open the storage host and report only whether it answered. +/// +/// ANY HTTP status counts as reachable, including 403 and 404. An unsigned GET +/// of a bucket root is supposed to be refused; being refused proves DNS, TCP +/// and TLS all worked, which is the entire question. Only a transport error +/// means the presigned URLs cannot be opened from this device. +async fn probe_storage_endpoint(client: &reqwest::Client, endpoint: &str) -> Result<(), String> { + match client.get(endpoint).send().await { + Ok(_) => Ok(()), + Err(e) => Err(transport_reason(&e)), + } +} + +/// A short reason for a failed request. +/// +/// `reqwest::Error`'s own `Display` is one line about the request and hides the +/// cause chain, so a DNS failure reads as "error sending request" — the exact +/// uninformative text that made this class of failure undiagnosable in the +/// first place. Walk to the innermost source instead. +/// +/// Shared with the transfer path so a failed upload and a failed probe describe +/// the same network condition in the same words. +pub(crate) fn transport_reason(error: &reqwest::Error) -> String { + let kind = if error.is_timeout() { + "timed out" + } else if error.is_connect() { + "connection failed" + } else { + "request failed" + }; + + let mut source: Option<&(dyn std::error::Error + 'static)> = std::error::Error::source(error); + let mut innermost: Option = None; + while let Some(cause) = source { + innermost = Some(cause.to_string()); + source = cause.source(); + } + + match innermost { + Some(detail) => format!("{kind}: {detail}"), + None => kind.to_string(), + } +} + +/// Pre-flight a sync server before saving it, and before trusting it to sync. +#[tauri::command] +pub async fn check_sync_server_connection(server_url: String) -> Result { + Ok(check_sync_server(&server_url).await) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unreachable_storage_is_not_usable() { + // The shape that used to report as healthy: server up, server's own + // storage fine, and the host it hands to clients resolving nowhere but the + // compose network. + let check = SyncServerCheck { + server_reachable: true, + storage_ready: Some(true), + storage_endpoint: Some("http://minio:9000".to_string()), + storage_reachable: Some(false), + storage_error: Some("connection failed: dns error".to_string()), + }; + assert!(!check.is_usable()); + } + + #[test] + fn reachable_storage_is_usable() { + let check = SyncServerCheck { + server_reachable: true, + storage_ready: Some(true), + storage_endpoint: Some("http://localhost:9101".to_string()), + storage_reachable: Some(true), + storage_error: None, + }; + assert!(check.is_usable()); + } + + #[test] + fn server_without_readyz_is_usable() { + // A server old enough to predate /readyz discloses nothing about storage. + // Unknown must not read as broken, or every older self-hosted server would + // start reporting a failure it does not have. + let check = SyncServerCheck { + server_reachable: true, + storage_ready: None, + storage_endpoint: None, + storage_reachable: None, + storage_error: None, + }; + assert!(check.is_usable()); + } + + #[test] + fn server_reporting_its_own_storage_down_is_not_usable() { + let check = SyncServerCheck { + server_reachable: true, + storage_ready: Some(false), + ..Default::default() + }; + assert!(!check.is_usable()); + } + + #[test] + fn unreachable_server_is_not_usable() { + assert!(!SyncServerCheck::default().is_usable()); + } + + #[tokio::test] + async fn empty_url_reports_unreachable_without_a_request() { + assert_eq!(check_sync_server(" ").await, SyncServerCheck::default()); + } + + #[tokio::test] + async fn unresolvable_storage_host_is_reported_with_a_cause() { + // Exercises the real probe against a host that cannot resolve, which is + // what a container-only endpoint looks like from the desktop. + let client = probe_client(); + let error = probe_storage_endpoint(&client, "http://minio.invalid:9000") + .await + .expect_err("an unresolvable host must not report as reachable"); + assert!( + error.contains("failed") || error.contains("timed out"), + "unexpected reason: {error}" + ); + // The bare reqwest Display is what this exists to avoid. + assert_ne!(error, "error sending request"); + } +} diff --git a/src/components/sync-config-dialog.tsx b/src/components/sync-config-dialog.tsx index 5950639..39b436d 100644 --- a/src/components/sync-config-dialog.tsx +++ b/src/components/sync-config-dialog.tsx @@ -26,7 +26,7 @@ import { } from "@/components/ui/tooltip"; import { useCloudAuth } from "@/hooks/use-cloud-auth"; import { showErrorToast, showSuccessToast } from "@/lib/toast-utils"; -import type { SyncSettings } from "@/types"; +import type { SyncServerCheck, SyncSettings } from "@/types"; const DEVICE_LINK_URL = "https://donutbrowser.com/auth/link"; @@ -78,49 +78,51 @@ export function SyncConfigDialog({ const [, setLiveProxyUsage] = useState(null); const [connectionStatus, setConnectionStatus] = useState< - "unknown" | "testing" | "connected" | "error" + "unknown" | "testing" | "connected" | "error" | "storage-unreachable" >("unknown"); const [storageEndpoint, setStorageEndpoint] = useState(null); const hasConfig = Boolean(serverUrl && token); - // `/health` is a bare liveness probe: it answers ok on a server whose storage - // is unreachable or misconfigured, which is how a green "connected" could sit - // next to a sync where every single file failed. `/readyz` checks storage and - // reports the endpoint clients are handed in presigned URLs, so surface that - // too — when transfers fail, it is the value worth checking first. - const probeServer = useCallback(async (url: string) => { - const base = url.replace(/\/$/, ""); - const response = await fetch(`${base}/readyz`); + // Probing the sync server alone is what let a broken setup look correct. + // Files never travel through that server: the client is handed a presigned + // URL and uploads straight to storage, so a server whose storage address is + // reachable only from its own network answers every probe while every single + // transfer fails at connect. The check runs in the backend because that is + // the client that performs the transfers — same DNS, proxy and TLS trust, so + // a setup that passes here can actually move bytes. + const probeServer = useCallback( + (url: string) => + invoke("check_sync_server_connection", { + serverUrl: url, + }), + [], + ); - // A server old enough to predate /readyz is still a working server, so - // fall back rather than reporting a healthy setup as broken. - if (response.status === 404) { - const health = await fetch(`${base}/health`); - return { ok: health.ok, storageEndpoint: undefined }; + const applyProbeResult = useCallback((result: SyncServerCheck) => { + setStorageEndpoint(result.storage_endpoint ?? null); + if (!result.server_reachable || result.storage_ready === false) { + setConnectionStatus("error"); + return "error" as const; } - - if (!response.ok) { - return { ok: false as const, storageEndpoint: undefined }; + if (result.storage_reachable === false) { + setConnectionStatus("storage-unreachable"); + return "storage-unreachable" as const; } - const body = (await response.json()) as { - storageEndpoint?: string; - } | null; - return { ok: true as const, storageEndpoint: body?.storageEndpoint }; + setConnectionStatus("connected"); + return "connected" as const; }, []); const testConnection = useCallback( async (url: string) => { setConnectionStatus("testing"); try { - const result = await probeServer(url); - setStorageEndpoint(result.storageEndpoint ?? null); - setConnectionStatus(result.ok ? "connected" : "error"); + applyProbeResult(await probeServer(url)); } catch { setStorageEndpoint(null); setConnectionStatus("error"); } }, - [probeServer], + [probeServer, applyProbeResult], ); const loadSettings = useCallback(async () => { @@ -173,12 +175,18 @@ export function SyncConfigDialog({ setConnectionStatus("testing"); try { const result = await probeServer(serverUrl); - setStorageEndpoint(result.storageEndpoint ?? null); - if (result.ok) { - setConnectionStatus("connected"); + const outcome = applyProbeResult(result); + if (outcome === "connected") { showSuccessToast(t("sync.config.connectionSuccess")); + } else if (outcome === "storage-unreachable") { + // Deliberately an error, not a warning. Nothing will sync in this + // state, and reporting it as success is the bug being fixed. + showErrorToast( + t("sync.config.storageUnreachable", { + endpoint: result.storage_endpoint ?? "", + }), + ); } else { - setConnectionStatus("error"); showErrorToast(t("sync.config.serverError")); } } catch { @@ -188,7 +196,7 @@ export function SyncConfigDialog({ } finally { setIsTesting(false); } - }, [serverUrl, t, probeServer]); + }, [serverUrl, t, probeServer, applyProbeResult]); const handleSave = useCallback(async () => { setIsSaving(true); @@ -485,6 +493,19 @@ export function SyncConfigDialog({ )} )} + {connectionStatus === "storage-unreachable" && ( +
+
+
+ {t("sync.config.storageUnreachableStatus")} +
+ + {t("sync.config.storageUnreachable", { + endpoint: storageEndpoint ?? "", + })} + +
+ )} {connectionStatus === "error" && (
diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 036e7d5..c6634c5 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -636,6 +636,8 @@ "serverError": "Server responded with an error", "connectFailed": "Failed to connect to server", "storageEndpoint": "Storage: {{endpoint}}", + "storageUnreachableStatus": "Storage unreachable", + "storageUnreachable": "The server is reachable, but its storage address {{endpoint}} cannot be reached from this device. File transfers will fail. If you self-host, set S3_PUBLIC_ENDPOINT to an address this device can reach.", "settingsSaved": "Sync settings saved", "saveFailed": "Failed to save settings", "disconnected": "Sync disconnected", @@ -1983,7 +1985,7 @@ "importSourceBrowserRunning": "Close {{browser}} first, or choose to import anyway", "wayfernFingerprintApplyFailed": "Could not apply this profile's fingerprint, so the browser was not started. {{detail}}", "wayfernFingerprintGenerationFailed": "Could not create a fingerprint for this profile. {{detail}}", - "wayfernGenerationLimitReached": "The fingerprint generation limit for this account has been reached. New fingerprints are unavailable for up to 24 hours. This limit applies to this computer, not to one profile.", + "wayfernGenerationLimitReached": "The fingerprint generation limit for this account has been reached. Your existing profiles will keep launching normally — only new fingerprints are paused, and they become available again a little later.", "wayfernCrossOsRequiresPlan": "This profile claims {{detail}}, which needs a paid plan and an active sign-in. Sign in or switch the profile to your own operating system." }, "rail": { diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index 9531eb7..4af7213 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -637,6 +637,8 @@ "serverError": "El servidor respondió con un error", "connectFailed": "Error al conectar con el servidor", "storageEndpoint": "Almacenamiento: {{endpoint}}", + "storageUnreachableStatus": "Almacenamiento inaccesible", + "storageUnreachable": "Se puede acceder al servidor, pero no a su dirección de almacenamiento {{endpoint}} desde este dispositivo. Las transferencias de archivos fallarán. Si usas un servidor propio, configura S3_PUBLIC_ENDPOINT con una dirección accesible desde este dispositivo.", "settingsSaved": "Ajustes de sincronización guardados", "saveFailed": "Error al guardar los ajustes", "disconnected": "Sincronización desconectada", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 2b91366..a1f4ab7 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -637,6 +637,8 @@ "serverError": "Le serveur a répondu avec une erreur", "connectFailed": "Échec de la connexion au serveur", "storageEndpoint": "Stockage : {{endpoint}}", + "storageUnreachableStatus": "Stockage inaccessible", + "storageUnreachable": "Le serveur est accessible, mais son adresse de stockage {{endpoint}} est inaccessible depuis cet appareil. Les transferts de fichiers échoueront. Si vous l'hébergez vous-même, définissez S3_PUBLIC_ENDPOINT sur une adresse accessible depuis cet appareil.", "settingsSaved": "Paramètres de synchronisation enregistrés", "saveFailed": "Échec de l’enregistrement des paramètres", "disconnected": "Synchronisation déconnectée", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 15b28fd..536d134 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -636,6 +636,8 @@ "serverError": "サーバーがエラーで応答しました", "connectFailed": "サーバーへの接続に失敗しました", "storageEndpoint": "ストレージ: {{endpoint}}", + "storageUnreachableStatus": "ストレージに接続できません", + "storageUnreachable": "サーバーには接続できますが、ストレージのアドレス {{endpoint}} にこのデバイスから接続できません。ファイル転送は失敗します。セルフホストの場合は、S3_PUBLIC_ENDPOINT にこのデバイスから接続できるアドレスを設定してください。", "settingsSaved": "同期設定を保存しました", "saveFailed": "設定の保存に失敗しました", "disconnected": "同期を切断しました", diff --git a/src/i18n/locales/ko.json b/src/i18n/locales/ko.json index a817cfd..6644f7a 100644 --- a/src/i18n/locales/ko.json +++ b/src/i18n/locales/ko.json @@ -636,6 +636,8 @@ "serverError": "서버가 오류로 응답했습니다", "connectFailed": "서버에 연결하지 못했습니다", "storageEndpoint": "스토리지: {{endpoint}}", + "storageUnreachableStatus": "스토리지에 연결할 수 없음", + "storageUnreachable": "서버에는 연결되지만 스토리지 주소 {{endpoint}}에 이 기기에서 연결할 수 없습니다. 파일 전송이 실패합니다. 자체 호스팅 중이라면 S3_PUBLIC_ENDPOINT를 이 기기에서 연결할 수 있는 주소로 설정하세요.", "settingsSaved": "동기화 설정이 저장되었습니다", "saveFailed": "설정 저장 실패", "disconnected": "동기화 연결 끊김", diff --git a/src/i18n/locales/pt.json b/src/i18n/locales/pt.json index f5b3443..0699fc2 100644 --- a/src/i18n/locales/pt.json +++ b/src/i18n/locales/pt.json @@ -637,6 +637,8 @@ "serverError": "O servidor respondeu com um erro", "connectFailed": "Falha ao conectar ao servidor", "storageEndpoint": "Armazenamento: {{endpoint}}", + "storageUnreachableStatus": "Armazenamento inacessível", + "storageUnreachable": "O servidor está acessível, mas o endereço de armazenamento {{endpoint}} não pode ser acessado deste dispositivo. As transferências de arquivos vão falhar. Se você hospeda o servidor, defina S3_PUBLIC_ENDPOINT com um endereço acessível deste dispositivo.", "settingsSaved": "Configurações de sincronização salvas", "saveFailed": "Falha ao salvar as configurações", "disconnected": "Sincronização desconectada", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index 58a734a..82f5673 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -638,6 +638,8 @@ "serverError": "Сервер вернул ошибку", "connectFailed": "Не удалось подключиться к серверу", "storageEndpoint": "Хранилище: {{endpoint}}", + "storageUnreachableStatus": "Хранилище недоступно", + "storageUnreachable": "Сервер доступен, но адрес хранилища {{endpoint}} недоступен с этого устройства. Передача файлов работать не будет. Если вы используете собственный сервер, укажите в S3_PUBLIC_ENDPOINT адрес, доступный с этого устройства.", "settingsSaved": "Настройки синхронизации сохранены", "saveFailed": "Не удалось сохранить настройки", "disconnected": "Синхронизация отключена", diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index 7e8f4d2..ab92ab1 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -636,6 +636,8 @@ "serverError": "Sunucu bir hatayla yanıt verdi", "connectFailed": "Sunucuya bağlanılamadı", "storageEndpoint": "Depolama: {{endpoint}}", + "storageUnreachableStatus": "Depolamaya erişilemiyor", + "storageUnreachable": "Sunucuya erişilebiliyor, ancak depolama adresine {{endpoint}} bu cihazdan erişilemiyor. Dosya aktarımları başarısız olacak. Kendi sunucunuzu barındırıyorsanız S3_PUBLIC_ENDPOINT değerini bu cihazdan erişilebilen bir adres olarak ayarlayın.", "settingsSaved": "Eşitleme ayarları kaydedildi", "saveFailed": "Ayarlar kaydedilemedi", "disconnected": "Eşitleme bağlantısı kesildi", diff --git a/src/i18n/locales/vi.json b/src/i18n/locales/vi.json index 84b18ac..790b057 100644 --- a/src/i18n/locales/vi.json +++ b/src/i18n/locales/vi.json @@ -636,6 +636,8 @@ "serverError": "Máy chủ trả về lỗi", "connectFailed": "Kết nối máy chủ thất bại", "storageEndpoint": "Bộ nhớ: {{endpoint}}", + "storageUnreachableStatus": "Không thể kết nối tới bộ nhớ", + "storageUnreachable": "Máy chủ có thể kết nối được, nhưng thiết bị này không truy cập được địa chỉ bộ nhớ {{endpoint}}. Việc truyền tệp sẽ thất bại. Nếu bạn tự lưu trữ, hãy đặt S3_PUBLIC_ENDPOINT thành địa chỉ mà thiết bị này truy cập được.", "settingsSaved": "Đã lưu cài đặt đồng bộ", "saveFailed": "Lưu cài đặt thất bại", "disconnected": "Đã ngắt kết nối đồng bộ", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index 3aa68a0..d354b6b 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -636,6 +636,8 @@ "serverError": "服务器返回了错误", "connectFailed": "连接服务器失败", "storageEndpoint": "存储: {{endpoint}}", + "storageUnreachableStatus": "无法连接存储", + "storageUnreachable": "服务器可以连接,但此设备无法访问其存储地址 {{endpoint}}。文件传输将会失败。如果你自建服务器,请将 S3_PUBLIC_ENDPOINT 设置为此设备可以访问的地址。", "settingsSaved": "同步设置已保存", "saveFailed": "保存设置失败", "disconnected": "已断开同步", diff --git a/src/types.ts b/src/types.ts index 1ca1383..de79d19 100644 --- a/src/types.ts +++ b/src/types.ts @@ -88,6 +88,24 @@ export interface SyncSettings { sync_token?: string; } +/** + * Result of `check_sync_server_connection`. Files upload straight to the + * storage host named in the presigned URL rather than through the sync server, + * so a healthy server is not evidence that sync works: `storage_reachable` + * false means every transfer will fail at connect. + * + * `null` means "not known", which is not the same as false — a server that + * predates `/readyz`, or a cloud deployment that withholds its storage host, + * discloses nothing to probe. + */ +export interface SyncServerCheck { + server_reachable: boolean; + storage_ready: boolean | null; + storage_endpoint: string | null; + storage_reachable: boolean | null; + storage_error: string | null; +} + /** * Capability/limit set derived from the plan by the backend. Features are gated * on these flags instead of a single "is paid?" check, so a plan like "solo"