Repository navigation
fix(services): ask a WMS layer again in the next answer format after the service's error - #969
Conversation
…the service's error
swisstopo's MapServer (wms.geo.admin.ch) offers GeoJSON for GetFeatureInfo
but fails to write it for some layers ("OGR_DS_CreateLayer failed … with
driver 'GEOJSON'", e.g. the swissGEOCOVER2D lines), while GML answers.
Identify showed the error instead of the features.
- An answer that is the service's exception report, without a code no
format changes (LayerNotQueryable, InvalidPoint, …), gives the request in
the next format the layer offers: both names of GeoJSON are left out
together, then GML, other XML, plain text, HTML.
- The format comes from the request answered (its INFO_FORMAT), so a
repeated or late answer doesn't drop another; the layer keeps its own
until a later format answers, then asks in that one for the session.
- identify.featureInfo takes "url" (the request the text answers) with lon
and lat; the app passes them, and turns the returned url into the next
background fetch. On the command line the fetch loop does the same.
- Without url or the point, or with no format left, the error is told as
before. Project files are unchanged.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Format ranking and retry rules crates/data/src/ogc_info.rs, crates/data/src/ogc.rs |
Feature-info formats use a shared ranking. XML exception reports are checked for eligibility before a retry. FeatureInfo can select a later format, and tests cover classification and format progression. |
Feature-info command fallback crates/engine/src/cmd/select_info.rs, crates/engine/src/tests.rs, docs/user-guide/editing.md |
The command validates request URLs, retries eligible errors in later formats, and updates the selected format after a successful response. Tests and the guide describe the fallback behavior. |
Map-view request URL tracking crates/ui-egui/src/mapview.rs |
Feature-info asks retain request URLs. The map view passes the URL and popup coordinates with each answer, then stores any returned follow-up URL. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Sequence Diagram(s)
sequenceDiagram
participant MapView
participant IdentifyFeatureInfo
participant WMS
MapView->>IdentifyFeatureInfo: Send answer, request URL, and popup coordinates
IdentifyFeatureInfo->>WMS: Request the next advertised format when eligible
WMS->>IdentifyFeatureInfo: Return the format response
IdentifyFeatureInfo->>MapView: Return results or a follow-up request URL
Possibly related PRs
- opengeos/RustGIS#943: Introduced the feature-info types and request flow that this change extends.
Suggested reviewers: giswqs
Merge Risk: 
afab3
-
The format fallback and map-view retry paths have no established merge-blocking issue. Merge after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: retrying a WMS layer with the next response format after a service error. Description check The description provides detailed problem, fix, testing, related issue, and validation information. It does not use the template's exact Summary or Related issue headings, but it includes the required… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description provides detailed problem, fix, testing, related issue, and validation information. It does not use the template's exact Summary or Related issue headings, but it includes the required content and is complete overall.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 4 files. (2 skipped: 1 unsupported, 1 too large.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the formats in a row,
Then hops to the next when errors say so.
A GML reply is kept in view,
The map remembers the request URL too.
Soft paws celebrate the fallback flow.
Comment @coderabbitai help to get the list of available commands.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// The answer format (one the layer offers) a request of the layer asked in: its INFO_FORMAT. | ||
| fn format_of(&self, request: &str) -> Option<&str> { | ||
| let asked = query_value(request, "info_format")?; | ||
| self.formats.iter().find(|f| super::encode(f).eq_ignore_ascii_case(asked)).map(String::as_str) |
There was a problem hiding this comment.
Low (confidence: medium). answered trims formats to the format that answered and the ones after it, so after that format_of can no longer find the format of an earlier request. An error to a request still in flight in an earlier format (for example a GeoJSON request sent before the layer learned GML) then makes retry_after return None, and the error is shown instead of retried. A fix would keep the full list in formats and track the chosen format separately, or accept the stale error and say so in a test.
There was a problem hiding this comment.
Declining: the case can't come up. In the app, each popup asks a layer through one chain (request, then the retry after an error), and a new click replaces the popup, so the old chain's answers are dropped unread. The command line asks one request at a time. So no request in an earlier format is still in flight once the layer has moved to a later one, and format_of always finds the format of the request it is given.
| if self.info_format().is_none_or(|f| format_rank(f) >= rank) { | ||
| return None; | ||
| } | ||
| let formats: Vec<String> = self.formats.iter().filter(|f| format_rank(f) >= rank).cloned().collect(); |
There was a problem hiding this comment.
Low (confidence: medium, design). One successful answer in a later format permanently drops every earlier format for the session. A transient service exception on GeoJSON (for example "Internal server error", which carries no code, so may_answer_in_another_format retries it) followed by a GML answer will move the layer to GML for good, even though GeoJSON would work again. This matches the stated goal of the PR, but it's worth deciding whether a retried success should only change the layer after it has been seen to recur, or whether the session-long switch is acceptable.
There was a problem hiding this comment.
Intended: the layer moves to a later format only after its earlier format failed with the service's error and the later one answered, which is what this PR is for. It holds for the session only. A project reopened, or the layer added again, reads the capabilities and starts from the preferred format. Waiting for a second failure would mean every click on such a service makes a failing request first.
Code reviewBugs
Security
Performance
Quality
AGENTS.md
I read the diff and the surrounding code in Change sizeCompared with the base branch: the whole PR, merge base 0844c6e to head afab3c7.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
📖 Docs preview
GitHub Pages takes a minute or two to publish a new commit. |
Follow-up to #943 (Identify on WMS layers).
Problem
swisstopo's WMS (
https://wms.geo.admin.ch/) listsapplication/jsonandapplication/json; subtype=geojsonfor GetFeatureInfo, so Identify asks in GeoJSON. For some layers its MapServer can't write GeoJSON and answers with an exception, e.g. Tectonic lines swissGEOCOVER2D (ch.swisstopo.geologie-swissgeocover2d_lines):The same point answers in GML (
application/vnd.ogc.gml),text/xmlandtext/plain. Other layers of the same service (e.g.ch.bfs.gebaeude_wohnungs_register) answer in GeoJSON. Identify showed the error toast instead of the features.Fix
rustgis-data(ogc_info.rs):may_answer_in_another_format(body): the answer is an exception report (read with the same XML reader as the answers). Codes that no format changes don't count:LayerNotQueryable,LayerNotDefined,InvalidPoint,InvalidCRS/InvalidSRS,StyleNotDefined, dimension and update-sequence codes.FeatureInfo::retry_after(request): the request in the formats read after the one the failed request asked in. ItsINFO_FORMATis matched against the layer's formats. Formats of the same kind go together, so both GeoJSON names are dropped at once.FeatureInfo::answered(request): once a later format has answered, the layer asks in it.identify.featureInfo:"url", the request that"text"answers. Withlonandlat, a retryable error returns{tileLayer, url}with the next request.urlor the point, or with no format left, the error is shown as before.mapview.rs): each pending answer keeps its request address and passes it with the point. A returnedurlalready becomes the next background fetch, as with capabilities. While the retry is in flight, the popup keeps waiting.docs/user-guide/editing.md, one paragraph.The project format is unchanged: the request is runtime-only, as in #943, and a test checks this.
Tests
ogc_info:a_format_the_service_fails_in_gives_way_to_the_nextcovers:answered.identify_asks_again_in_the_next_format_after_the_services_errorcovers:LayerNotQueryableand broken JSON not retried;cargo xtask cilocally: fmt, clippy and layers pass, and 52 test suites pass. Only the two render tests that fail on this machine without its fonts fail (text_takes_italic_alignment_and_letter_spacing,a_text_graphic_is_drawn_aligned_slanted_and_spaced); they are unrelated.rustgis-cli: identify onch.swisstopo.geologie-swissgeocover2d_linesnear Winterthur (8.729, 47.484) now returns the feature (kind_desc: Gesteinshorizont,lpro_litho_desc: Bentonit), and the next identify asks in GML straight away.Summary by CodeRabbit