Commit 6597457
authored
fix(vector): stop a failed restore from deleting multi-layer URL layers (#1761)
* 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
* Address CodeRabbit and Claude review feedback
- Do not fall back to a browser fetch when the native command rejected the
URL by policy. The webview is not subject to the backend SSRF guard, so a
link-local or metadata URL it refused could still be reached through the
fallback. New isBlockedUrlError mirrors the guard's messages and re-throws
those, leaving the fallback for genuine transport failures.
- Give both browser fetch paths the same 180s budget via AbortSignal.timeout.
Siblings now await one shared download, so a stalled fetch would have held
all of them pending rather than only itself.
- Correct the timeoutSecs doc: the backend clamps into [8, 600] rather than
falling back to the default, and move the field onto a fetchUrlBytes-only
options type since resolve_url_redirect hardcodes its own timeout.
- Drop the embedded-source branch from replayVectorControlLayerById. Refresh
is gated on isVectorControlRefreshLayer, which requires an HTTP URL, so it
was unreachable. Restore still preserves such layers.
- Guard replayVectorControlLayerById against concurrent calls for one id. The
getLayer check cannot hold across the addData await, so two callers would
both pass it and the second would throw on the duplicate id.
* Address Claude review feedback on the review fixes
- Give each download attempt its own budget instead of sharing one signal.
The shared deadline started before the native call, so in the exact case
the fallbacks exist for (a slow origin exhausting the native timeout) they
were handed an already-aborted signal and rejected instantly. Each attempt
now gets a fresh AbortSignal.timeout.
- Claim replaying ids during project restore too, not only in
replayVectorControlLayerById. The control does not register a layer until
its data has loaded, so an auto-refresh tick landing mid-restore saw the id
as absent and started a second addData for a layer restore was already
loading. restoreVectorLayers now tracks its in-flight replays through the
same registry, which closes the window for all three restore branches.
* Address CodeRabbit review feedback
- Preserve a local-file layer whose replay fails, matching the URL and
embedded branches. The file had already been read successfully, so the
failure was in the load rather than a missing source, and reopening the
project re-reads the same path. Only a failed read still drops the layer,
which is the case where the file is genuinely gone; replayVectorLayer
settles its own rejection, so that outer catch never saw replay failures
anyway and no restructuring was needed.
* Cover the missing-native-layer sync guard with regression tests
Locks in that a store layer whose map layers are absent (a preserved failed
restore) makes no style, visibility, or ordering calls, and that a stale id
alongside a live one skips only the stale one rather than dropping the whole
sync. Both fail without the getLayer guard.
* Address review feedback
- Tighten GeoLibreAppAPI.fetchVectorUrl to Promise<File | null>. The whole
cache-collapsing benefit depends on returning a File rather than a bare
Blob, and the type now enforces that instead of only documenting it.
- Fix a test fixture comment that named the wrong layer id.
* Guard restoreVectorLayers against replaying an in-flight layer
Project loading can invoke the restore pass more than once before the
first pass finishes. addData does not expose a layer through getLayer
until its async ingest completes, so getLayer alone let a second pass
replay the same id and race the first when adding its MapLibre source
("Source ... already exists"). Skip ids already claimed in
replayingLayerIds, which also blocks refresh replays for the same
in-flight id.
* Address Claude review feedback
- Close a redirect/DNS SSRF bypass in the native fetch path. The guarded
redirect policy stopped a blocked hop with `attempt.stop()`, which reqwest
documents as returning the 30x response as `Ok` — so it reached the frontend
as "Request failed with status 302", which `isBlockedUrlError` does not
match, and `fetchVectorUrl` then retried the URL with an unguarded webview
`fetch` that followed the very redirect the policy refused. A blocked hop now
fails via `attempt.error(SSRF_BLOCKED_MESSAGE)`, and `request_error_message`
walks the error source chain so both that and `GuardedDnsResolver`'s
rejection (which reqwest's connector buries the same way) surface with the
guard's own wording. The 10-hop cap keeps `stop()`: an over-long chain is not
an SSRF rejection. Covered by a new `is_ssrf_guard_error` unit test.
- Hoist `failedLayerIds` to module scope beside `replayingLayerIds`. The two
were mismatched in lifetime: overlapping restore passes divide the layers
between them via the shared claim, but each pass ran its own closing
`syncVectorLayersToStore`, and the last to run — which the shared suspension
counter decides — knew only its own pass's failures and pruned its sibling's,
reintroducing the exact layer loss this PR fixes. `trackReplay` clears an id
when a fresh attempt starts so a successful refresh drops a stale mark, and
the closing sync prunes ids the store no longer knows about.1 parent 6d54b35 commit 6597457
13 files changed
Lines changed: 844 additions & 75 deletions
File tree
- apps/geolibre-desktop
- src-tauri/src
- src
- components/panels
- hooks
- lib
- packages
- map/src
- plugins/src
- plugins
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
139 | 143 | | |
140 | 144 | | |
141 | 145 | | |
| |||
821 | 825 | | |
822 | 826 | | |
823 | 827 | | |
| 828 | + | |
| 829 | + | |
| 830 | + | |
| 831 | + | |
| 832 | + | |
| 833 | + | |
| 834 | + | |
| 835 | + | |
| 836 | + | |
| 837 | + | |
824 | 838 | | |
825 | 839 | | |
826 | 840 | | |
827 | 841 | | |
828 | 842 | | |
829 | 843 | | |
830 | 844 | | |
831 | | - | |
| 845 | + | |
832 | 846 | | |
833 | 847 | | |
834 | 848 | | |
835 | 849 | | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
| 853 | + | |
| 854 | + | |
| 855 | + | |
| 856 | + | |
| 857 | + | |
| 858 | + | |
| 859 | + | |
| 860 | + | |
| 861 | + | |
| 862 | + | |
| 863 | + | |
| 864 | + | |
| 865 | + | |
| 866 | + | |
| 867 | + | |
| 868 | + | |
| 869 | + | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
| 877 | + | |
| 878 | + | |
| 879 | + | |
| 880 | + | |
836 | 881 | | |
837 | 882 | | |
838 | 883 | | |
| |||
1035 | 1080 | | |
1036 | 1081 | | |
1037 | 1082 | | |
| 1083 | + | |
| 1084 | + | |
| 1085 | + | |
| 1086 | + | |
| 1087 | + | |
| 1088 | + | |
1038 | 1089 | | |
1039 | | - | |
1040 | | - | |
| 1090 | + | |
| 1091 | + | |
1041 | 1092 | | |
1042 | 1093 | | |
1043 | 1094 | | |
1044 | 1095 | | |
1045 | | - | |
| 1096 | + | |
| 1097 | + | |
| 1098 | + | |
| 1099 | + | |
| 1100 | + | |
| 1101 | + | |
| 1102 | + | |
| 1103 | + | |
| 1104 | + | |
| 1105 | + | |
| 1106 | + | |
| 1107 | + | |
1046 | 1108 | | |
1047 | 1109 | | |
1048 | 1110 | | |
| 1111 | + | |
1049 | 1112 | | |
1050 | 1113 | | |
1051 | 1114 | | |
1052 | | - | |
| 1115 | + | |
1053 | 1116 | | |
1054 | | - | |
| 1117 | + | |
1055 | 1118 | | |
1056 | 1119 | | |
1057 | 1120 | | |
| |||
1552 | 1615 | | |
1553 | 1616 | | |
1554 | 1617 | | |
1555 | | - | |
| 1618 | + | |
1556 | 1619 | | |
1557 | 1620 | | |
1558 | 1621 | | |
| |||
4002 | 4065 | | |
4003 | 4066 | | |
4004 | 4067 | | |
4005 | | - | |
| 4068 | + | |
| 4069 | + | |
4006 | 4070 | | |
4007 | 4071 | | |
4008 | 4072 | | |
| |||
4258 | 4322 | | |
4259 | 4323 | | |
4260 | 4324 | | |
| 4325 | + | |
| 4326 | + | |
| 4327 | + | |
| 4328 | + | |
| 4329 | + | |
| 4330 | + | |
| 4331 | + | |
| 4332 | + | |
| 4333 | + | |
| 4334 | + | |
| 4335 | + | |
| 4336 | + | |
| 4337 | + | |
| 4338 | + | |
| 4339 | + | |
| 4340 | + | |
| 4341 | + | |
| 4342 | + | |
| 4343 | + | |
| 4344 | + | |
| 4345 | + | |
| 4346 | + | |
| 4347 | + | |
| 4348 | + | |
| 4349 | + | |
| 4350 | + | |
| 4351 | + | |
| 4352 | + | |
| 4353 | + | |
4261 | 4354 | | |
4262 | 4355 | | |
4263 | 4356 | | |
| |||
4661 | 4754 | | |
4662 | 4755 | | |
4663 | 4756 | | |
| 4757 | + | |
| 4758 | + | |
| 4759 | + | |
| 4760 | + | |
| 4761 | + | |
| 4762 | + | |
| 4763 | + | |
| 4764 | + | |
| 4765 | + | |
| 4766 | + | |
| 4767 | + | |
| 4768 | + | |
| 4769 | + | |
| 4770 | + | |
| 4771 | + | |
| 4772 | + | |
| 4773 | + | |
| 4774 | + | |
| 4775 | + | |
| 4776 | + | |
4664 | 4777 | | |
4665 | 4778 | | |
4666 | 4779 | | |
| |||
Lines changed: 9 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
1376 | 1377 | | |
1377 | 1378 | | |
1378 | 1379 | | |
1379 | | - | |
| 1380 | + | |
| 1381 | + | |
| 1382 | + | |
| 1383 | + | |
| 1384 | + | |
| 1385 | + | |
| 1386 | + | |
1380 | 1387 | | |
1381 | 1388 | | |
1382 | | - | |
| 1389 | + | |
1383 | 1390 | | |
1384 | 1391 | | |
1385 | 1392 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
121 | 121 | | |
122 | 122 | | |
123 | 123 | | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
124 | 129 | | |
125 | 130 | | |
126 | 131 | | |
| |||
1033 | 1038 | | |
1034 | 1039 | | |
1035 | 1040 | | |
1036 | | - | |
1037 | | - | |
1038 | | - | |
1039 | | - | |
1040 | | - | |
1041 | | - | |
1042 | | - | |
1043 | | - | |
1044 | | - | |
| 1041 | + | |
| 1042 | + | |
| 1043 | + | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
| 1048 | + | |
| 1049 | + | |
| 1050 | + | |
| 1051 | + | |
| 1052 | + | |
| 1053 | + | |
| 1054 | + | |
1045 | 1055 | | |
1046 | | - | |
1047 | | - | |
1048 | | - | |
| 1056 | + | |
| 1057 | + | |
| 1058 | + | |
| 1059 | + | |
| 1060 | + | |
| 1061 | + | |
| 1062 | + | |
| 1063 | + | |
| 1064 | + | |
| 1065 | + | |
| 1066 | + | |
| 1067 | + | |
| 1068 | + | |
| 1069 | + | |
| 1070 | + | |
| 1071 | + | |
| 1072 | + | |
| 1073 | + | |
| 1074 | + | |
| 1075 | + | |
| 1076 | + | |
| 1077 | + | |
| 1078 | + | |
| 1079 | + | |
| 1080 | + | |
1049 | 1081 | | |
1050 | | - | |
1051 | | - | |
1052 | | - | |
1053 | | - | |
1054 | 1082 | | |
1055 | 1083 | | |
1056 | | - | |
1057 | | - | |
1058 | | - | |
1059 | | - | |
1060 | | - | |
1061 | | - | |
1062 | | - | |
1063 | | - | |
1064 | | - | |
| 1084 | + | |
| 1085 | + | |
| 1086 | + | |
| 1087 | + | |
| 1088 | + | |
| 1089 | + | |
| 1090 | + | |
| 1091 | + | |
1065 | 1092 | | |
1066 | 1093 | | |
1067 | 1094 | | |
| |||
1333 | 1360 | | |
1334 | 1361 | | |
1335 | 1362 | | |
| 1363 | + | |
| 1364 | + | |
| 1365 | + | |
| 1366 | + | |
| 1367 | + | |
| 1368 | + | |
| 1369 | + | |
| 1370 | + | |
1336 | 1371 | | |
1337 | 1372 | | |
1338 | 1373 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
25 | 41 | | |
26 | 42 | | |
27 | 43 | | |
| |||
79 | 95 | | |
80 | 96 | | |
81 | 97 | | |
82 | | - | |
| 98 | + | |
83 | 99 | | |
84 | 100 | | |
85 | 101 | | |
86 | | - | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
87 | 108 | | |
88 | 109 | | |
89 | 110 | | |
| |||
112 | 133 | | |
113 | 134 | | |
114 | 135 | | |
115 | | - | |
| 136 | + | |
| 137 | + | |
116 | 138 | | |
117 | 139 | | |
118 | 140 | | |
119 | 141 | | |
120 | | - | |
| 142 | + | |
121 | 143 | | |
122 | 144 | | |
123 | 145 | | |
| |||
0 commit comments