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

[bhavin121]

Description

This PR modifies the error handling behavior in IndexedCollectionBaseProperty to prevent client crashes when attempting to remove a key that does not exist. Instead of throwing a critical PR-008 error, the system will now log a warning and continue execution.

Problem

• In high-load scenarios (e.g., 50-100MB data transfer over WebSocket), clients can fall significantly out of sync. This leads to a specific race condition
• If two clients were out of sync and they have generated identicle remove ops
• Now one delete op A gets summarized where the other delete op B remain unsummarized.
• In this case when a new client joins the session will always get PR-008 error.
• And the document cannot be used at all.

Solution

The _removeByKey method in IndexedCollectionBaseProperty has been updated to:
Warn instead of Throw: When a non-existent key removal is attempted, it now logs console.warn(MSG.REMOVED_NON_EXISTING_ENTRY + in_key) instead of throwing an error.

Breaking Changes

This PR introduces a breaking change to the behavior of remove() methods in PropertyDDS collection types (MapProperty, ReferenceMapProperty, SetProperty).

Previous Behavior

• Attempting to remove a non-existent key would throw an error: Error: PR-008: Trying to remove a non-existing entry: <key>
• This would cause the client to crash and stop processing operations

New Behavior

• Attempting to remove a non-existent key will:
  • Log a warning to the console: PR-008: Trying to remove a non-existing entry: <key>
  • Return undefined (instead of throwing)
  • Continue execution without interruption

Migration Impact

Code that relies on errors being thrown will need to be updated:

// ❌ OLD CODE - Will no longer work as expected
try {
  mapProperty.remove('nonExistentKey');
} catch (error) {
  // This catch block will never execute now
  handleError(error);
}

// ✅ NEW CODE - Check return value instead
const removed = mapProperty.remove('nonExistentKey');
if (removed === undefined) {
  // Key didn't exist
  handleMissingKey();
}

[anthony-murphy]

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

[bhavin121]

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

[bhavin121]

@microsoft-github-policy-service agree company="Autodesk"

[anthony-murphy]

/azp run Build - api-markdown-documenter, Build - benchmark-tool, Build - build-common, Build - build-tools, Build - client packages, Build - common-utils, Build - eslint-config-fluid, Build - eslint-plugin-fluid, Build - protocol-definitions, Build - test-tools

[anthony-murphy]

/azp run server-gitrest, server-gitssh, server-historian, server-routerlicious

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[anthony-murphy]

/azp run repo-policy-check

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).