Conversation
Applying init data threw part-way through on several paths, which abandoned everything after them - so a share link with one broken layer loaded no layers at all and never moved the camera. - loadModelStratum: "Invalid model traits" now returns an error Result and "Model cannot be dereferenced" is collected, instead of throwing. Loading a container is also guarded so a broken container can't take out the models around it. - _applyInitData: each independent part (catalog, elements, stories, viewerMode, baseMaps, homeCamera, initialCamera, splitter, settings, each model, previewedItemId, workbench, timeline, pickedFeatures, shortenShareUrls) now runs through an error-collecting helper. - loadPickedFeatures no longer leaves a floating promise. Errors are still combined and thrown at the end, so the user is warned about what failed - the map just loads everything it can first. Fixes #5168
ljowen
changed the title
- Don't raise picked feature load failures to the user. Leaflet rejects `allFeaturesAvailablePromise` by design and the failure is already surfaced via `pickedFeatures.error`, so a share link with picked features and one failing GetFeatureInfo request popped a modal on load. Log to the error service instead. - Return the base map promise from `_applyInitData` even when parts failed. It was returned after the throw, so any error dropped it and let `loadPersistedOrInitBaseMap()` race the base map set from init data. `_applyInitData` now returns the combined error rather than throwing it, and callers rethrow. - Make the workbench part resilient item by item. The whole block - model lookup, loading, analytics and the final assignment - was inside one `applyPart`, so a single throw dropped every workbench item, including the ones that loaded. Items are now collected outside the part and committed unconditionally. `applyPart`/`applyPartAsync` now forward the result of `apply`, which keeps the per-item guards readable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
soyarsauce
reviewed
Sep 15, 2026
Collaborator
Contributor
Author
There was a problem hiding this comment.
Sounds good to me, maybe that should happen as part of the config change @zoran995 is working through
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5168.
Description
A share link that refers to one broken layer loads nothing - none of the good layers, and the map never moves to the shared location.
Terria._applyInitDataalready collects most errors and carries on, but a handful of paths stillthrowpart-way through, which abandons every part of the init data after them (workbench, timeline, picked features, ...):loadModelStratumthrows"Invalid model traits"whenmodels[id]isn't an object - the throw escapes thePromise.allin_applyInitDataand takes down the whole init source.loadModelStratumthrows"Model cannot be dereferenced"when the share data has adereferencedblock but the model is no longer a Reference. This is the common real-world case: an item whose type is no longer registered (removed plugin, renamed type) falls back to a stub, and the stub can't be dereferenced. It also escapes the recursive calls for containers andSplitItemReference.initialCameraorhomeCamerathrows out ofCameraView.fromJsonbefore the workbench is populated.What this PR does
loadModelStratumreturns/collects those two errors instead of throwing, and guards container loading so one broken container can't take out the models around it._applyInitDataruns each independent part through a small error-collecting helper (applyPart/applyPartAsync), so one bad part never aborts the rest. The sync helper is deliberately notasync, so the MobX@actionsemantics before the firstawaitare unchanged.loadPickedFeaturesno longer leaves a floating promise.The contract is unchanged: the combined
TerriaErroris still thrown at the end, so the user still gets a warning naming what failed - the map just loads everything it can first.Test it
Each link puts two inline-GeoJSON markers on Sydney Harbour (green Opera House, blue Harbour Bridge) with an
initialCameraover the harbour, plus one broken thing. Compare the same share link onmainand on this branch:maintype: "a-catalog-type-that-no-longer-exists"and adereferencedblock - what a share of a reference item looks like once its type is goneinitialCamera: { "west": "this is not a number" }- the original report: nothing loads and the map never movesThe "on this branch" links need this branch's CI deployment to finish. Swap the base for
http://localhost:3001/to run them locally.Specs
Seven new specs in
TerriaSpecunderapplyInitData -> is resilient to broken init data: a control (nothing broken), non-object model traits,dereferencedon a non-reference model, a broken container, invalidinitialCamera, invalidhomeCamera, and init data applied after a broken model. All fail onmainexcept the control.