Skip to content

[PropertyDDS] Relax error handling for removal of non-existent keys (PR-008) #26157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bhavin121 wants to merge 3 commits into
base: main
Choose a base branch
from
+13 −7
Open

[PropertyDDS] Relax error handling for removal of non-existent keys (PR-008) #26157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55f02e1
Remove throw error when property trying to delete doesn't exist
bhavinprajapat-adsk Jan 7, 2026
81386bd
Merge branch 'microsoft:main' into big-fix/remove-error-pr-008
bhavin121 Jan 8, 2026
abe962b
Merge branch 'main' into big-fix/remove-error-pr-008
bhavin121 Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .../PropertyDDS/packages/property-properties/src/properties/indexedCollectionBaseProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ export class IndexedCollectionBaseProperty extends AbstractStaticCollectionPrope

this._setDirty(in_reportToView);
} else {
throw new Error(MSG.REMOVED_NON_EXISTING_ENTRY + in_key);
console.warn(MSG.REMOVED_NON_EXISTING_ENTRY + in_key);
Copy link
Contributor

@anthony-murphy anthony-murphy Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fluid should never write directly to the console. there should be a logger somewhere that you can log an error to.

Copy link
Author

@bhavin121 bhavin121 Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @anthony-murphy, there is no logger being used in PropertyDDS. And only direct console logs are there.

}
}

Expand Down
6 changes: 4 additions & 2 deletions experimental/PropertyDDS/packages/property-properties/src/properties/mapProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,11 @@ export class MapProperty extends IndexedCollectionBaseProperty {
/**
* Removes the entry with the given key from the map
*
* Note: If the key does not exist, a warning is logged and no error is thrown.
* This is intentional to support idempotent operations in collaborative editing scenarios.
*
* @param {string} in_key - The key of the entry to remove from the map
* @throws If trying to remove an entry that does not exist
* @return {*} the item removed
* @return {*} the item removed, or undefined if the key does not exist
*/
remove(in_key) {
var item = this.get(in_key);
Expand Down
6 changes: 4 additions & 2 deletions experimental/PropertyDDS/packages/property-properties/src/properties/referenceMapProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ export class ReferenceMapProperty extends StringMapProperty {
/**
* Removes the entry with the given key from the map
*
* Note: If the key does not exist, a warning is logged and no error is thrown.
* This is intentional to support idempotent operations in collaborative editing scenarios.
*
* @param {string} in_key - The key of the entry to remove from the map
* @throws If trying to remove an entry that does not exist
* @returns {String} the item removed (a string pathT
* @returns {String} the item removed (a string path), or undefined if the key does not exist
*/
remove(in_key) {
var item = this.getValue(in_key);
Expand Down
6 changes: 4 additions & 2 deletions experimental/PropertyDDS/packages/property-properties/src/properties/setProperty.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,11 @@ export class SetProperty extends IndexedCollectionBaseProperty {
/**
* Removes the given property from the set
*
* Note: If the entry does not exist, a warning is logged and no error is thrown.
* This is intentional to support idempotent operations in collaborative editing scenarios.
*
* @param {property-properties.NamedProperty|string} in_entry - The property or its URN to remove from the set
* @return {property-properties.NamedProperty} the property that was removed.
* @throws if trying to remove an entry that does not exist
* @return {property-properties.NamedProperty} the property that was removed, or undefined if not found
*/
remove(in_entry) {
if (_.isString(in_entry)) {
Expand Down