Skip to content

Commit 4c44d5d

Browse files
committed
fix(vector): stop a failed restore from deleting multi-layer URL layers
A multi-layer container (a KMZ of KML folders, a GeoPackage of tables) becomes one store layer per source layer, and every one of them keeps the container's URL. Three problems compounded from there, reported against NASA FIRMS fire-footprint KMZ feeds in discussion #1757. Restoring such a project replays each layer independently, and any layer whose replay failed was then pruned by the closing store sync, which reads "the control does not have this layer" as "the user removed it". A feed that was briefly unreachable therefore deleted the layers from the project outright, and the next save wrote that loss to disk. Restore now records which replays failed and keeps those layers, and refresh (the button and the auto-refresh tick) falls through to a new replay path so the next successful fetch brings the layer back instead of leaving a dead entry. Each of those sibling layers also downloaded the container separately: on desktop a six-layer KMZ pulled the same archive six times over on every project open and every refresh interval, and unzipped and registered it into DuckDB six times. Add Vector Layer downloads are now collapsed to one in-flight request per URL, handing every sibling the identical File so the control's per-source unzip and registration caches collapse with them. Those six concurrent downloads then ran into the native fetch command's tile-sized 8s budget, which is what produced the reporter's "Could not read response body" failures at exactly 8.4s. fetch_url_bytes now accepts a caller-supplied timeout, clamped so it can only be raised and never removed, and the vector loader asks for a download-sized budget. Finally, a store layer that outlives its map layers no longer has its style synced, which was filling the Diagnostics panel with "Cannot get style of non-existing layer" the user could do nothing about. Ref: #1757
1 parent ca6a5fc commit 4c44d5d

11 files changed

Lines changed: 515 additions & 43 deletions

File tree

apps/geolibre-desktop/src-tauri/src/lib.rs

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ const UV_INSTALL_BASE_URL: &str = "https://astral.sh/uv";
136136
const REMOTE_TILE_TIMEOUT_SECS: u64 = 8;
137137
const REMOTE_TILE_CONNECT_TIMEOUT_SECS: u64 = 4;
138138
const URL_RESOLVE_TIMEOUT_SECS: u64 = 15;
139+
/// Ceiling for a caller-supplied `fetch_url_bytes` budget. The default suits a
140+
/// tile; callers that download a whole dataset (Add Vector Layer) ask for more,
141+
/// but not without bound, so a bad value cannot wedge a request indefinitely.
142+
const MAX_FETCH_TIMEOUT_SECS: u64 = 600;
139143

140144
#[cfg(all(unix, not(feature = "mas")))]
141145
const SIGTERM: i32 = 15;
@@ -1035,21 +1039,39 @@ fn build_guarded_http_client() -> Result<reqwest::blocking::Client, String> {
10351039
.map_err(|error| format!("Could not create HTTP client: {error}"))
10361040
}
10371041

1042+
/// Fetches a URL's bytes, bypassing browser CORS.
1043+
///
1044+
/// `timeout_secs` overrides the tile-sized default for callers that download a
1045+
/// whole dataset rather than a tile; it is clamped to
1046+
/// `[REMOTE_TILE_TIMEOUT_SECS, MAX_FETCH_TIMEOUT_SECS]`, so the budget can only
1047+
/// ever be raised and never removed.
10381048
#[tauri::command]
1039-
async fn fetch_url_bytes(url: String) -> Result<Vec<u8>, String> {
1040-
tauri::async_runtime::spawn_blocking(move || fetch_url_bytes_blocking(url))
1049+
async fn fetch_url_bytes(url: String, timeout_secs: Option<u64>) -> Result<Vec<u8>, String> {
1050+
tauri::async_runtime::spawn_blocking(move || fetch_url_bytes_blocking(url, timeout_secs))
10411051
.await
10421052
.map_err(|error| format!("Tile fetch task failed: {error}"))?
10431053
}
10441054

1045-
fn fetch_url_bytes_blocking(url: String) -> Result<Vec<u8>, String> {
1055+
/// Resolves the request budget for a fetch, defaulting to the tile timeout and
1056+
/// clamping a caller-supplied value into `[REMOTE_TILE_TIMEOUT_SECS,
1057+
/// MAX_FETCH_TIMEOUT_SECS]`. A caller can therefore only ever raise the budget,
1058+
/// and never past the ceiling or down to zero (which reqwest reads as "no
1059+
/// timeout" and would let a stalled request hang forever).
1060+
fn resolve_fetch_timeout_secs(timeout_secs: Option<u64>) -> u64 {
1061+
timeout_secs
1062+
.unwrap_or(REMOTE_TILE_TIMEOUT_SECS)
1063+
.clamp(REMOTE_TILE_TIMEOUT_SECS, MAX_FETCH_TIMEOUT_SECS)
1064+
}
1065+
1066+
fn fetch_url_bytes_blocking(url: String, timeout_secs: Option<u64>) -> Result<Vec<u8>, String> {
10461067
ensure_fetchable_url(&url)?;
10471068

10481069
let client = guarded_http_client()?;
1070+
let timeout = resolve_fetch_timeout_secs(timeout_secs);
10491071

10501072
let response = client
10511073
.get(&url)
1052-
.timeout(Duration::from_secs(REMOTE_TILE_TIMEOUT_SECS))
1074+
.timeout(Duration::from_secs(timeout))
10531075
.send()
10541076
.map_err(|error| format!("Request failed: {error}"))?;
10551077
let status = response.status();
@@ -4002,7 +4024,8 @@ mod tests {
40024024
use super::{
40034025
client_cert_is_pkcs12, client_cert_password_without_path, ensure_fetchable_url,
40044026
is_allowed_local_vector_path, is_allowed_project_path, is_disallowed_ip,
4005-
is_safe_absolute_path, path_is_under, tcp_table_port,
4027+
is_safe_absolute_path, path_is_under, resolve_fetch_timeout_secs, tcp_table_port,
4028+
MAX_FETCH_TIMEOUT_SECS, REMOTE_TILE_TIMEOUT_SECS,
40064029
};
40074030
#[cfg(target_os = "linux")]
40084031
use super::{linux_uses_nvidia_renderer, nvidia_is_primary_gpu};
@@ -4661,6 +4684,26 @@ mod tests {
46614684
assert_eq!(tcp_table_port(0x0000_223E), 0x3E22);
46624685
}
46634686

4687+
// Add Vector Layer downloads a whole dataset through the same command that
4688+
// fetches tiles, so it asks for a longer budget. The clamp is what keeps
4689+
// that override from becoming a way to disable the timeout entirely.
4690+
#[test]
4691+
fn clamps_the_fetch_timeout_into_the_allowed_range() {
4692+
// No override: the tile-sized default.
4693+
assert_eq!(resolve_fetch_timeout_secs(None), REMOTE_TILE_TIMEOUT_SECS);
4694+
// A dataset-sized budget is honored as asked.
4695+
assert_eq!(resolve_fetch_timeout_secs(Some(180)), 180);
4696+
// Zero would mean "no timeout" to reqwest, so it is raised to the floor
4697+
// rather than letting a stalled request hang forever.
4698+
assert_eq!(resolve_fetch_timeout_secs(Some(0)), REMOTE_TILE_TIMEOUT_SECS);
4699+
// Below the floor is raised; above the ceiling is capped.
4700+
assert_eq!(resolve_fetch_timeout_secs(Some(1)), REMOTE_TILE_TIMEOUT_SECS);
4701+
assert_eq!(
4702+
resolve_fetch_timeout_secs(Some(u64::MAX)),
4703+
MAX_FETCH_TIMEOUT_SECS
4704+
);
4705+
}
4706+
46644707
// The image-path guard is what keeps the reaper from killing a Jupyter the
46654708
// user installed themselves: only executables under our own runtime
46664709
// directory are ours to terminate.

apps/geolibre-desktop/src/components/panels/LayerPanel.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import {
5555
materializeEmbeddableVectorLayers,
5656
RASTER_SOURCE_KIND,
5757
reloadVectorControlLayer,
58+
replayVectorControlLayerById,
5859
SKETCHES_SOURCE_KIND,
5960
TIME_SLIDER_PLUGIN_ID,
6061
type TimePropertyCandidate,
@@ -1376,10 +1377,16 @@ export function LayerPanel({
13761377
return;
13771378
}
13781379
if (isVectorControlRefreshLayer(layer)) {
1379-
const info = await reloadVectorControlLayer(layer.id);
1380+
// A layer whose restore failed stays in the project but never made it
1381+
// into the control, so reloadLayer cannot find it. Replaying it is
1382+
// what brings such a layer back once the source is reachable again,
1383+
// which is why refresh (manual and automatic) tries that second.
1384+
const info =
1385+
(await reloadVectorControlLayer(layer.id)) ??
1386+
(await replayVectorControlLayerById(layer.id));
13801387
if (!info) {
13811388
// The control is unavailable (panel never opened, or torn down
1382-
// and not yet replayed) or no longer knows this layer id.
1389+
// and not yet replayed) or the replay above did not succeed.
13831390
// Automatic ticks fire on a timer the user didn't initiate, so
13841391
// skip silently and clear the transient note instead of surfacing
13851392
// an error every interval until the control comes back.

apps/geolibre-desktop/src/hooks/usePlugins.ts

Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ import { appendDiagnostic } from "../lib/diagnostics";
121121
import { pickZarrDirectory, zarrDirectoryPickerSupported } from "../lib/zarr-directory-picker";
122122
import { openExternalLink } from "../lib/open-external";
123123
import { fetchUrlBytes } from "../lib/native-http";
124+
import { dedupeVectorUrlFetch, vectorDownloadFileName } from "../lib/vector-url-fetch";
124125
import { partitionProjectPluginManifestUrls } from "../lib/plugin-trust";
125126
import { setTimeSliderOpenedByBinding, shouldCloseTimeSliderDock } from "../lib/time-slider-dock";
126127
import { createWmsTileUrl, normalizeWmsVersion } from "../components/layout/add-data/helpers";
@@ -1033,35 +1034,47 @@ export function createAppAPI(mapControllerRef?: RefObject<MapController | null>)
10331034
// presence to auto-discover shapefile sidecars instead of forcing the user
10341035
// to select every component, and to capture the file's path for restore.
10351036
pickVectorFilesWithSidecars: isTauriRuntime() ? pickVectorFilesWithSidecars : undefined,
1036-
fetchVectorUrl: async (url: string) => {
1037-
if (isTauriRuntime()) {
1038-
try {
1039-
const bytes = await fetchUrlBytes(url, { context: "Add Vector Layer" });
1040-
const array = bytes instanceof Uint8Array ? bytes : new Uint8Array(bytes);
1041-
return new Blob([array as Uint8Array<ArrayBuffer>]);
1042-
} catch {
1043-
// The shared native command has a short, tile-oriented timeout.
1044-
// Preserve the former browser path for large CORS-enabled datasets.
1037+
// Shared across the sibling layers of one multi-layer container, which all
1038+
// carry the container's URL: without this a six-layer KMZ downloaded itself
1039+
// six times over on every project open and every refresh tick.
1040+
fetchVectorUrl: (url: string) =>
1041+
dedupeVectorUrlFetch(url, async () => {
1042+
const name = vectorDownloadFileName(url);
1043+
if (isTauriRuntime()) {
10451044
try {
1046-
const response = await fetch(url);
1047-
if (!response.ok) {
1048-
throw new Error(`HTTP ${response.status} ${response.statusText}`);
1049-
}
1050-
return response.blob();
1045+
const bytes = await fetchUrlBytes(url, {
1046+
context: "Add Vector Layer",
1047+
// The default budget on this command is tile-sized (8s). A vector
1048+
// dataset is not a tile. A few megabytes from a slow origin
1049+
// routinely needs longer, and timing out here used to drop the
1050+
// layer entirely, so ask for a download-sized budget instead.
1051+
timeoutSecs: VECTOR_DOWNLOAD_TIMEOUT_SECS,
1052+
});
1053+
const array = bytes instanceof Uint8Array ? bytes : new Uint8Array(bytes);
1054+
return new File([array as Uint8Array<ArrayBuffer>], name);
10511055
} catch {
1052-
// GitHub's /raw route rejects browser CORS, so fall through to the
1053-
// same guarded proxy used by the web build.
1056+
// Keep the browser path as a fallback for CORS-enabled origins the
1057+
// native command could not reach.
1058+
try {
1059+
const response = await fetch(url);
1060+
if (!response.ok) {
1061+
throw new Error(`HTTP ${response.status} ${response.statusText}`);
1062+
}
1063+
return new File([await response.blob()], name);
1064+
} catch {
1065+
// GitHub's /raw route rejects browser CORS, so fall through to the
1066+
// same guarded proxy used by the web build.
1067+
}
10541068
}
10551069
}
1056-
}
1057-
const proxyUrl = githubRawVectorProxyUrl(url);
1058-
if (!proxyUrl) return null;
1059-
const response = await fetch(proxyUrl);
1060-
if (!response.ok) {
1061-
throw new Error(`HTTP ${response.status} ${response.statusText}`);
1062-
}
1063-
return response.blob();
1064-
},
1070+
const proxyUrl = githubRawVectorProxyUrl(url);
1071+
if (!proxyUrl) return null;
1072+
const response = await fetch(proxyUrl);
1073+
if (!response.ok) {
1074+
throw new Error(`HTTP ${response.status} ${response.statusText}`);
1075+
}
1076+
return new File([await response.blob()], name);
1077+
}),
10651078
readLocalVectorFile: readVectorFileWithSidecars,
10661079
exportTextFile: (filename: string, content: string, options?: GeoLibreFileDialogOptions) => {
10671080
const description = options?.description ?? "GeoJSON";
@@ -1333,6 +1346,14 @@ function isTauriRuntime(): boolean {
13331346

13341347
const GITHUB_RAW_VECTOR_PROXY = "https://tiles.geolibre.app/github-raw";
13351348

1349+
/**
1350+
* Budget for a native Add Vector Layer download, in seconds. Deliberately far
1351+
* above `fetch_url_bytes`'s tile-sized default: this command carries whole
1352+
* datasets, not 256px tiles, and a timeout here is not a slow tile that resolves
1353+
* next frame but a layer that fails to restore.
1354+
*/
1355+
const VECTOR_DOWNLOAD_TIMEOUT_SECS = 180;
1356+
13361357
function githubRawVectorProxyUrl(value: string): string | null {
13371358
let url: URL;
13381359
try {

apps/geolibre-desktop/src/lib/native-http.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ export type NativeHttpCommand = "fetch_url_bytes" | "resolve_url_redirect";
2020
interface NativeHttpOptions {
2121
/** Short feature label (e.g. "WFS GetCapabilities") added to the record. */
2222
context?: string;
23+
/**
24+
* Request budget in seconds, overriding the command's tile-sized default.
25+
* Set it for calls that carry a whole dataset rather than a tile; the backend
26+
* clamps the value, so an out-of-range number degrades to the default rather
27+
* than removing the timeout.
28+
*/
29+
timeoutSecs?: number;
2330
}
2431

2532
function recordSource(command: NativeHttpCommand, context?: string): string {
@@ -83,7 +90,12 @@ async function invokeNativeHttp<T>(
8390
): Promise<T> {
8491
const startedAt = performance.now();
8592
try {
86-
const result = await invoke<T>(command, { url });
93+
// `timeoutSecs` is omitted rather than sent as undefined so the Rust side
94+
// sees an absent argument and applies its own default.
95+
const result = await invoke<T>(command, {
96+
url,
97+
...(options?.timeoutSecs === undefined ? {} : { timeoutSecs: options.timeoutSecs }),
98+
});
8799
appendDiagnostic(
88100
nativeHttpSuccessRecord(
89101
command,
@@ -112,7 +124,8 @@ async function invokeNativeHttp<T>(
112124
* subject to browser CORS), recording the request in the diagnostics log.
113125
*
114126
* @param url - The absolute HTTP(S) URL to fetch.
115-
* @param options - Optional context label for the diagnostics record.
127+
* @param options - Optional context label for the diagnostics record, and an
128+
* optional request budget for callers downloading more than a tile.
116129
* @returns The response body bytes (Tauri may hand back a plain number array).
117130
*/
118131
export function fetchUrlBytes(
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* De-duplication for Add Vector Layer's remote downloads.
3+
*
4+
* A multi-layer container (a KMZ of KML folders, a GeoPackage of tables, a DXF)
5+
* becomes one store layer per source layer, but every one of them keeps the
6+
* *container's* URL. Restoring such a project replays each layer independently,
7+
* and auto-refresh ticks them independently too, so a six-layer KMZ issued six
8+
* concurrent downloads of the same archive on every project open and on every
9+
* refresh interval (opengeos/GeoLibre discussion #1757). On a slow origin that
10+
* is enough to blow the fetch budget and fail layers that would have loaded had
11+
* they asked once.
12+
*
13+
* `dedupeVectorUrlFetch` collapses those into a single in-flight request per
14+
* URL. Two properties matter:
15+
*
16+
* - **In-flight only.** The entry is dropped as soon as the download settles, so
17+
* this is a request collapser and never a cache: a later auto-refresh always
18+
* re-downloads and genuinely refreshes the layer. Sibling layers of one
19+
* container share a download only because they ask within the same window.
20+
* - **One identity.** Every caller gets the *same* `File` object, not a copy.
21+
* maplibre-gl-vector keys its per-source caches (the unzipped KML it
22+
* registers with DuckDB, a GeoPackage's bytes) on the source object, so
23+
* handing all six layers one identical `File` collapses the unzip and the
24+
* DuckDB registration too: a 91 MB KML is registered once instead of six
25+
* times. Returning a `Blob` would not: the control wraps a plain `Blob` in a
26+
* fresh `File` per call, and the caches would miss again.
27+
*/
28+
29+
/**
30+
* In-flight downloads keyed by URL. Entries live only until they settle.
31+
*
32+
* The value is a wrapper rather than the bare promise so the download's own
33+
* `finally` can identify (and clear) exactly its own entry while still running
34+
* *inside* the promise. A `.finally()` chained onto the outside would resolve a
35+
* tick after awaiting callers, leaving a settled entry briefly visible and
36+
* letting an immediate retry share an already-finished download.
37+
*/
38+
interface InFlightEntry {
39+
promise: Promise<File | null>;
40+
}
41+
42+
const inFlight = new Map<string, InFlightEntry>();
43+
44+
/**
45+
* Runs `download` for `url`, sharing the request with any call for the same URL
46+
* that is still in flight.
47+
*
48+
* @param url - The absolute URL being downloaded.
49+
* @param download - Performs the actual download. Called at most once per
50+
* in-flight window; returns null when no loader could serve the URL.
51+
* @returns The downloaded file (the identical object for every sharer), or null.
52+
*/
53+
export function dedupeVectorUrlFetch(
54+
url: string,
55+
download: () => Promise<File | null>,
56+
): Promise<File | null> {
57+
const existing = inFlight.get(url);
58+
if (existing) return existing.promise;
59+
60+
const entry = {} as InFlightEntry;
61+
entry.promise = (async () => {
62+
try {
63+
return await download();
64+
} finally {
65+
// Cleared either way: a failure must not be replayed to a later refresh,
66+
// and a success must not be served as a stale cache hit. Only our own
67+
// entry is cleared, since a retry that started later owns the key.
68+
if (inFlight.get(url) === entry) inFlight.delete(url);
69+
}
70+
})();
71+
inFlight.set(url, entry);
72+
return entry.promise;
73+
}
74+
75+
/**
76+
* Names a downloaded blob so the vector control's format detection still works.
77+
*
78+
* The control derives the format from the file name, so the URL's last path
79+
* segment is used when it has an extension; otherwise the blob is handed over
80+
* under a generic name and the control falls back to sniffing the content type.
81+
*
82+
* @param url - The URL the bytes came from.
83+
* @param fallback - Name to use when the URL carries no usable file name.
84+
* @returns A file name for the downloaded bytes.
85+
*/
86+
export function vectorDownloadFileName(url: string, fallback = "data"): string {
87+
let pathname: string;
88+
try {
89+
pathname = new URL(url).pathname;
90+
} catch {
91+
return fallback;
92+
}
93+
const last = pathname.slice(pathname.lastIndexOf("/") + 1);
94+
const decoded = safeDecode(last);
95+
return /\.[A-Za-z0-9]+$/.test(decoded) ? decoded : fallback;
96+
}
97+
98+
function safeDecode(value: string): string {
99+
try {
100+
return decodeURIComponent(value);
101+
} catch {
102+
return value;
103+
}
104+
}
105+
106+
/** Clears every in-flight entry. Exported for tests. */
107+
export function resetVectorUrlFetchDedupe(): void {
108+
inFlight.clear();
109+
}

packages/map/src/layer-sync.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,15 @@ function syncExternalNativeLayer(
479479
clearExternalNativeExtrusion(map, layer, nativeLayerIds);
480480

481481
for (const nativeLayerId of nativeLayerIds) {
482+
// A store layer can legitimately outlive its map layers: a layer whose
483+
// restore failed keeps the `nativeLayerIds` it was saved with, and the
484+
// owning control has not (yet) recreated them. Styling a layer that is
485+
// not on the map raises "Cannot get style of non-existing layer" on the
486+
// map's error channel, which fills the Diagnostics panel with noise the
487+
// user can do nothing about, so skip those ids until the control brings
488+
// them back.
489+
const nativeLayer = map.getLayer(nativeLayerId);
490+
if (!nativeLayer) continue;
482491
moveLayer(map, nativeLayerId, beforeId);
483492
// The owning control mirrors direct per-layer visibility changes from
484493
// the store, but effective state such as a hidden parent group never
@@ -492,8 +501,7 @@ function syncExternalNativeLayer(
492501
// hide-unmatched filter: filtering is independent of the paint the
493502
// control owns. Native layers without a filter (deck.gl / 3D Tiles
494503
// custom layers) are skipped by the type guard.
495-
const nativeLayer = map.getLayer(nativeLayerId);
496-
if (nativeLayer && nativeLayerSupportsFilter(nativeLayer.type)) {
504+
if (nativeLayerSupportsFilter(nativeLayer.type)) {
497505
applyExternalNativeFeatureFilters(map, nativeLayerId, layer);
498506
}
499507
}

packages/plugins/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ export {
265265
materializeEmbeddableVectorLayers,
266266
openVectorLayerPanel,
267267
reloadVectorControlLayer,
268+
replayVectorControlLayerById,
268269
restoreVectorLayers,
269270
setKmlFileImportHandler,
270271
isKmlFileSelection,

0 commit comments

Comments
 (0)