Relations: Swallow exceptions when retrieving references from incompatible property values (closes #22197)#22207
Merged
AndyButland merged 4 commits intomainfrom Apr 22, 2026
Conversation
…es with changed property types.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes reference-tracking robustness in DataValueReferenceFactoryCollection by ensuring exceptions from incompatible stored property values are logged and swallowed (instead of being re-thrown), preventing relation/reference lookups from blocking operations like save/publish when editors change.
Changes:
- Removed an unintended
throw;that re-threw exceptions after logging during reference extraction. - Routed the
IDataValueReferenceFactorypath through the same per-value try/catch helper used forIDataValueReferenceeditors. - Added unit tests covering both editor and factory paths, including continuation after a throwing value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollection.cs |
Ensures exceptions during reference extraction are consistently swallowed for both editor- and factory-based reference resolution. |
tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs |
Adds regression tests to verify incompatible values don’t propagate exceptions and that processing continues for subsequent values. |
… if the links JSON could not be deserialised.
lauraneto
approved these changes
Apr 22, 2026
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
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.
Description
#22197 reports an error when a data type is changed to a new property editor, incompatible with previously stored property values. An exception is thrown in trying to retrieve references from the incorrect JSON data.
We actually addressed this before. PR #18576 added a
try/catcharound reference retrieval specifically to handle the scenario where a property editor on a data type is changed and existing stored values are incompatible with the new editor. However, despite the comment in the catch block saying "Log the exception but don't throw, continue with the next value", the code containedthrow;on the next line, re-throwing the exception after logging it. This meant the protection never actually worked.So this PR corrects that, handles a missing case for the
GetReferencesFromPropertyValuesmethod, and adds unit tests verifying that incompatible property values do not propagate exceptions.Additional Fix
In testing I found the scenario outlined in the test steps below would log an exception as expected, but the property was still not editable. The blocks JSON was still be returned to the multi-URL picker client side property editor, which would crash as it got data it wasn't expecting.
This happened because the
MultiUrlPickerValueEditor.ToEditor()catch block previously fell back tobase.ToEditor(), which deserializes the incompatible value as a dynamic JSONI changed the fallback to return
Enumerable.Empty<object>()(matching the existing empty-value behaviour), so theproperty renders as empty and allows the user to enter new values.
Testing
Automated
DataValueReferenceFactoryCollectionTestscovering both the editor and factory paths, including continuation after a throwing value. These tests fail before the fix is applied.Manual
JsonException; after this fix the save succeeds (a warning is logged but the operation completes).Error getting references from valuemessage at Error level.