Skip to content

Commit 657e4bd

Browse files
authored
fix: resolve relative style/tileset URLs for OGC vector tiles (#1641)
* fix: resolve relative style/tileset URLs for OGC vector tiles Adding an Esri vector tile service through Add Data -> OGC Vector Tiles added the layer and then rendered nothing, with no error anywhere. Every Esri vector tile style declares its tileset relatively: "sources": { "esri": { "type": "vector", "url": "../../" } } `resolveOgcVectorTiles` copied that value straight into the MapLibre source, and MapLibre resolves what it is handed against the *app* origin rather than the document it came from -- so the source pointed at the GeoLibre origin and never issued a single tile request. The `tiles` form of a style source had the same flaw, as does an Esri `VectorTileServer` document, which lists `tile/{z}/{y}/{x}.pbf`. Resolve both forms against the document they were read from, and read the tileset document behind a style's source URL so a non-TileJSON service still yields usable absolute templates. `URL` percent-encodes `{`/`}`, which would defeat MapLibre's placeholder substitution, so the braces are restored after resolution. Also surface Esri's HTTP 200 error bodies. ArcGIS reports a missing or renamed service as 200 with `{"error":{"code":404,"message":...}}`, so `response.ok` was true and the body parsed cleanly. That reached the dialog as a valid-but-empty document and was reported as "no source layers" -- hiding the real cause ("Requested Service not available.") and sending the reporter looking for a nonexistent client problem. Fixes #1639 * fix: surface ArcGIS error `details` when `message` is empty ArcGIS routinely returns an error envelope whose `message` is an empty string, with the only useful text in `details`. Asking a hosted FeatureServer for a layer id it does not have answers: {"code":400,"message":"","details": ["The requested layer (layerId: 0) was not found."]} Both error paths read `message` alone and fell back to a generic string, so the Add Data dialog said "ArcGIS service request failed." and dropped the one line naming what to correct. Prefer `message`, then `details`, then the fallback. * Address review feedback - Do not read `code: 0` as a failure in `assertNoServiceError`: zero conventionally means success, so an envelope carrying only `code: 0` would have rejected a perfectly good document. - Recognize a string `code` (`{"error":{"code":"NotFound"}}`) as a reported error. Only a numeric code counted before, so a host using string codes with no `message` slipped through as an empty document — the exact failure mode this check exists to catch. - Add tests for the code-only branch (numeric 498 and a string code), and for the shapes that must *not* trip it: `code: 0`, and an `error` member that is a string, an array, or null.
1 parent bbec6dd commit 657e4bd

5 files changed

Lines changed: 369 additions & 9 deletions

File tree

apps/geolibre-desktop/src/lib/ogc-json.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,42 @@ async function responseStatusError(response: Response): Promise<Error> {
7979
}
8080
}
8181

82+
/**
83+
* Throws when a 2xx JSON document is really an Esri error report.
84+
*
85+
* ArcGIS answers a missing, renamed, or token-protected service with HTTP
86+
* **200** and `{"error":{"code":404,"message":"Requested Service not
87+
* available."}}`, so `response.ok` is true and the body parses cleanly. Left
88+
* undetected it reaches the caller as a valid-but-empty document and surfaces
89+
* as a misleading downstream complaint ("no source layers") instead of the
90+
* actual cause.
91+
*
92+
* The shape is checked, not just the key: a legitimate document is free to
93+
* carry an `error` property of some other form, so a `message` or a reported
94+
* `code` is required before the document is rejected. `code: 0` conventionally
95+
* means success, so it is not on its own a failure; a string code
96+
* (`{"code":"NotFound"}`) is as much a report as a numeric one.
97+
*/
98+
function assertNoServiceError(doc: unknown): void {
99+
const error = (doc as { error?: unknown } | null | undefined)?.error;
100+
if (!error || typeof error !== "object" || Array.isArray(error)) return;
101+
const { message, code, details } = error as {
102+
message?: unknown;
103+
code?: unknown;
104+
details?: unknown;
105+
};
106+
const hasMessage = typeof message === "string" && message.trim() !== "";
107+
const hasCode =
108+
(typeof code === "number" && code !== 0) || (typeof code === "string" && code.trim() !== "");
109+
if (!hasMessage && !hasCode) return;
110+
// `details` is an array of extra lines; append it when it adds anything.
111+
const detail = Array.isArray(details)
112+
? details.filter((line): line is string => typeof line === "string" && line.trim() !== "")
113+
: [];
114+
const text = hasMessage ? message.trim() : `Service error ${String(code).trim()}`;
115+
throw new Error([text, ...detail].join(" ").slice(0, 300));
116+
}
117+
82118
/**
83119
* Fetches and parses a remote JSON document, working around cross-origin limits
84120
* (see the module comment). When the caller passes a `signal` it owns the
@@ -106,7 +142,11 @@ export async function fetchOgcJson(
106142
try {
107143
const bytes = await Promise.race([bytesPromise, rejectOnAbort(abort)]);
108144
const array = bytes instanceof Uint8Array ? bytes : new Uint8Array(bytes);
109-
return JSON.parse(new TextDecoder().decode(array));
145+
const doc = JSON.parse(new TextDecoder().decode(array));
146+
// `normalizeFetchError` passes an Error through unchanged, so the service
147+
// error message survives the catch below.
148+
assertNoServiceError(doc);
149+
return doc;
110150
} catch (error) {
111151
throw normalizeFetchError(error);
112152
}
@@ -122,5 +162,7 @@ export async function fetchOgcJson(
122162
if (!response.ok) {
123163
throw await responseStatusError(response);
124164
}
125-
return response.json();
165+
const doc = await response.json();
166+
assertNoServiceError(doc);
167+
return doc;
126168
}

apps/geolibre-desktop/src/lib/ogc-vector-tiles.ts

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ function asBounds(value: unknown): [number, number, number, number] | undefined
7272
return undefined;
7373
}
7474

75+
/** Whether a reference already carries a scheme (or is protocol-relative), and
76+
* so needs no resolution against the document it was read from. */
77+
function isAbsoluteUrl(value: string): boolean {
78+
return /^[a-z][a-z0-9+.-]*:/i.test(value) || value.startsWith("//");
79+
}
80+
7581
/** Non-empty string tile templates from an unknown `tiles` value. */
7682
function asTiles(value: unknown): string[] | undefined {
7783
if (!Array.isArray(value)) return undefined;
@@ -251,6 +257,19 @@ export function tileJsonConfig(
251257
) {
252258
config.center = center as number[];
253259
}
260+
// An Esri `VectorTileServer` document is TileJSON-shaped enough to read
261+
// (`name`, `maxzoom`) but lists its tiles relatively — `tile/{z}/{y}/{x}.pbf`.
262+
// MapLibre would resolve those against the app origin, so when the document
263+
// carries relative templates, hand it explicit absolute `tiles` instead of the
264+
// document URL. Absolute templates are left alone so a conforming TileJSON
265+
// keeps being loaded by MapLibre itself (it re-reads more than is copied here).
266+
const tiles = asTiles(tilejson.tiles);
267+
if (tiles?.some((tile) => !isAbsoluteUrl(tile))) {
268+
config.tiles = tiles.map((tile) =>
269+
normalizeTilePlaceholders(resolveDocumentUrl(tile, tilejsonUrl)),
270+
);
271+
delete config.url;
272+
}
254273
const sourceLayers = vectorLayerIds(tilejson);
255274
if (sourceLayers.length > 0) config.sourceLayers = sourceLayers;
256275
return config;
@@ -263,6 +282,35 @@ function normalizeTilePlaceholders(url: string): string {
263282
return url.replace(/\{z\}/gi, "{z}").replace(/\{x\}/gi, "{x}").replace(/\{y\}/gi, "{y}");
264283
}
265284

285+
/**
286+
* Resolves a reference taken out of a fetched document against that document's
287+
* own URL.
288+
*
289+
* Style and service documents routinely use relative references: every Esri
290+
* vector tile style declares its tileset as `{"type":"vector","url":"../../"}`,
291+
* and an Esri `VectorTileServer` document lists `tiles:
292+
* ["tile/{z}/{y}/{x}.pbf"]`. MapLibre resolves whatever it is handed against
293+
* the *app* origin rather than the document it came from, so passing such a
294+
* value through verbatim requests the wrong host entirely — the layer is added
295+
* and then silently renders nothing (GeoLibre#1639).
296+
*
297+
* `URL` percent-encodes the `{`/`}` of a tile template, which would defeat
298+
* MapLibre's placeholder substitution, so the braces are restored afterwards.
299+
*
300+
* @param value - The (possibly relative) reference read from the document.
301+
* @param baseUrl - The URL the document was fetched from.
302+
* @returns The absolute URL, or the trimmed input when it cannot be resolved.
303+
*/
304+
export function resolveDocumentUrl(value: string, baseUrl?: string): string {
305+
const trimmed = value.trim();
306+
if (trimmed === "" || !baseUrl) return trimmed;
307+
try {
308+
return new URL(trimmed, baseUrl).toString().replace(/%7B/gi, "{").replace(/%7D/gi, "}");
309+
} catch {
310+
return trimmed;
311+
}
312+
}
313+
266314
/**
267315
* Resolves the configuration for an OGC API vector tiles layer from the URLs the
268316
* user provided, fetching the TileJSON and/or style document as needed.
@@ -324,12 +372,49 @@ export async function resolveOgcVectorTiles(input: {
324372
}
325373
if (vector) {
326374
// Fall back to the style's own vector source only when no tiles input was
327-
// given...
375+
// given. Both forms are resolved against the style URL first: they are
376+
// routinely relative (see `resolveDocumentUrl`).
328377
if (!config.url && !config.tiles) {
329378
if (typeof vector.source.url === "string") {
330-
config.url = vector.source.url;
379+
const sourceUrl = resolveDocumentUrl(vector.source.url, styleUrl);
380+
config.url = sourceUrl || undefined;
381+
// The style only names the tileset; the document it points at holds
382+
// the tile templates and zoom range. Read it so a non-TileJSON
383+
// service (Esri's `VectorTileServer`) still yields usable absolute
384+
// tiles. Best-effort: the style's own metadata below still applies if
385+
// this fails, so a fetch error must not block adding the layer.
386+
if (sourceUrl) {
387+
const tileset = await fetchOgcJson(sourceUrl, signal).catch((error) => {
388+
if (input.signal?.aborted) throw error;
389+
return null;
390+
});
391+
if (tileset && typeof tileset === "object") {
392+
// Style-derived source layers and name stay authoritative
393+
// (manual > style > TileJSON), so the tileset must not clobber
394+
// them. `url` is assigned rather than spread: `tileJsonConfig`
395+
// *drops* it when it emits explicit `tiles`, and a spread would
396+
// leave the stale value behind — a MapLibre vector source given
397+
// both `url` and `tiles` is invalid.
398+
const {
399+
sourceLayers: tilesetLayers,
400+
name: tilesetName,
401+
url: tilesetUrl,
402+
...tilesetConfig
403+
} = tileJsonConfig(tileset as Record<string, unknown>, sourceUrl);
404+
config = {
405+
...config,
406+
...tilesetConfig,
407+
url: tilesetUrl,
408+
name: config.name ?? tilesetName,
409+
sourceLayers:
410+
config.sourceLayers.length > 0 ? config.sourceLayers : (tilesetLayers ?? []),
411+
};
412+
}
413+
}
331414
} else {
332-
config.tiles = asTiles(vector.source.tiles)?.map(normalizeTilePlaceholders);
415+
config.tiles = asTiles(vector.source.tiles)?.map((tile) =>
416+
normalizeTilePlaceholders(resolveDocumentUrl(tile, styleUrl)),
417+
);
333418
}
334419
}
335420
// ...but always fill a missing zoom range / bounds from the style, even

packages/plugins/src/plugins/arcgis-layer.ts

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,35 @@ async function addArcGISFeatureLayerAsGeoJson(
253253
return id;
254254
}
255255

256+
/** The JSON error envelope ArcGIS returns, usually with an HTTP 200 status. */
257+
interface ArcGISErrorEnvelope {
258+
message?: string;
259+
details?: unknown;
260+
}
261+
262+
/**
263+
* The most specific text available from an ArcGIS error envelope.
264+
*
265+
* `message` is frequently an empty string, with the only useful text in
266+
* `details` — asking a hosted FeatureServer for a layer id it does not have
267+
* answers `{"code":400,"message":"","details":["The requested layer (layerId:
268+
* 0) was not found."]}`. Reading `message` alone drops that and reports the
269+
* generic fallback, which says nothing about what to correct.
270+
*
271+
* @param error - The `error` member of an ArcGIS JSON response.
272+
* @param fallback - The message to use when the envelope carries no text.
273+
*/
274+
function arcgisErrorMessage(error: ArcGISErrorEnvelope | undefined, fallback: string): string {
275+
const message = typeof error?.message === "string" ? error.message.trim() : "";
276+
if (message) return message;
277+
const details = Array.isArray(error?.details)
278+
? error.details.filter(
279+
(detail): detail is string => typeof detail === "string" && detail.trim() !== "",
280+
)
281+
: [];
282+
return details.length > 0 ? details.join(" ").trim() : fallback;
283+
}
284+
256285
/**
257286
* Fetch and validate a GeoJSON FeatureCollection from an ArcGIS query URL.
258287
*
@@ -281,7 +310,7 @@ async function fetchArcGISGeoJson(url: string): Promise<FeatureCollection> {
281310
);
282311
}
283312
let json: FeatureCollection & {
284-
error?: { message?: string };
313+
error?: ArcGISErrorEnvelope;
285314
exceededTransferLimit?: boolean;
286315
};
287316
try {
@@ -290,7 +319,7 @@ async function fetchArcGISGeoJson(url: string): Promise<FeatureCollection> {
290319
throw new Error("The ArcGIS feature layer did not return GeoJSON features.");
291320
}
292321
if (json.error) {
293-
throw new Error(json.error.message || "ArcGIS feature query failed.");
322+
throw new Error(arcgisErrorMessage(json.error, "ArcGIS feature query failed."));
294323
}
295324
if (json.type !== "FeatureCollection" || !Array.isArray(json.features)) {
296325
throw new Error("The ArcGIS feature layer did not return GeoJSON features.");
@@ -378,10 +407,10 @@ async function fetchArcGISJson<T>(
378407
});
379408
}
380409
const json = (await response.json()) as T & {
381-
error?: { message?: string };
410+
error?: ArcGISErrorEnvelope;
382411
};
383412
if (json.error) {
384-
throw new Error(json.error.message || "ArcGIS service request failed.", {
413+
throw new Error(arcgisErrorMessage(json.error, "ArcGIS service request failed."), {
385414
cause,
386415
});
387416
}

tests/arcgis-feature-layer.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,32 @@ describe("addArcGISLayer (feature layer)", () => {
149149
assert.equal(useAppStore.getState().layers.length, 0);
150150
});
151151

152+
// ArcGIS routinely leaves `message` empty and puts the only useful text in
153+
// `details` — asking a hosted FeatureServer for a layer id it does not have
154+
// answers `{"code":400,"message":"","details":["The requested layer (layerId:
155+
// 0) was not found."]}`. Reporting the generic fallback instead hides the one
156+
// thing the user needs to correct.
157+
it("surfaces error `details` when the service leaves `message` empty", async () => {
158+
globalThis.fetch = (async () =>
159+
jsonResponse({
160+
error: {
161+
code: 400,
162+
message: "",
163+
details: ["The requested layer (layerId: 0) was not found."],
164+
},
165+
})) as typeof fetch;
166+
167+
await assert.rejects(
168+
addArcGISLayer(app, {
169+
layerType: "feature",
170+
sourceType: "url",
171+
url: "https://example.com/arcgis/rest/services/Cities/FeatureServer/0",
172+
}),
173+
/The requested layer \(layerId: 0\) was not found\./,
174+
);
175+
assert.equal(useAppStore.getState().layers.length, 0);
176+
});
177+
152178
it("rejects an HTML login page returned with a 200 status", async () => {
153179
globalThis.fetch = (async (input: RequestInfo | URL) => {
154180
const url = typeof input === "string" ? input : input.toString();

0 commit comments

Comments
 (0)