Skip to content

fix: downgrade missing userPreferences log from error to warning, deduplicate#1718

Merged
grigasp merged 5 commits into
masterfrom
copilot/fix-usenullvaluestorage-error
Jun 12, 2026
Merged

fix: downgrade missing userPreferences log from error to warning, deduplicate#1718
grigasp merged 5 commits into
masterfrom
copilot/fix-usenullvaluestorage-error

Conversation

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

IModelAppUserPreferencesStorage logs an error on every get()/set() call when IModelApp.userPreferences is not defined. This is noisy and misleading—it's a configuration gap, not a runtime failure.

Changes:

  • Logger.logErrorLogger.logWarning for the "userPreferences not defined" case
  • Warning is logged only once per storage instance via a #warnedAboutMissingPreferences guard
  • Actual runtime failures (e.g. save/get throwing) remain as logError
private logMissingPreferencesWarning(key: string, action: string) {
  if (!this.#warnedAboutMissingPreferences) {
    Logger.logWarning(LOGGER_CATEGORY, `Cannot ${action} persisted user preference ${key} because 'IModelApp.userPreferences' not defined.`);
    this.#warnedAboutMissingPreferences = true;
  }
}

Tests updated to assert logWarning and verify single-emission behavior.

…y once

When `IModelApp.userPreferences` is not defined, the
`IModelAppUserPreferencesStorage` now logs a warning instead of an error,
and only logs it once per storage instance to avoid log spam.

Closes #1714
Copilot AI changed the title [WIP] Fix useNullValueStorage error when no storage provider is present fix: downgrade missing userPreferences log from error to warning, deduplicate Jun 12, 2026
Copilot AI requested a review from grigasp June 12, 2026 07:52
@grigasp

grigasp commented Jun 12, 2026

Copy link
Copy Markdown
Member

@copilot, the change file is missing

Copilot AI commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, the change file is missing

Added in commit Add beachball change file for property-grid-react patch.

Copilot AI requested a review from grigasp June 12, 2026 07:57
@grigasp grigasp marked this pull request as ready for review June 12, 2026 07:58
@grigasp grigasp requested review from a team as code owners June 12, 2026 07:58
Copilot AI review requested due to automatic review settings June 12, 2026 07:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces noise from IModelAppUserPreferencesStorage when IModelApp.userPreferences is not configured by downgrading the log level and deduplicating the message per storage instance, while keeping real runtime failures logged as errors.

Changes:

  • Downgrades the “IModelApp.userPreferences not defined” log from Logger.logError to Logger.logWarning.
  • Deduplicates the missing-preferences warning so it emits only once per IModelAppUserPreferencesStorage instance.
  • Updates/extends unit tests to assert logWarning and verify single-emission behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/itwin/property-grid/src/property-grid-react/api/PreferencesStorage.ts Logs missing IModelApp.userPreferences as a one-time warning via a private guard/helper.
packages/itwin/property-grid/src/test/api/PreferencesStorage.test.ts Updates tests to assert logWarning (not logError) and adds single-emission assertions.
change/@itwin-property-grid-react-5e8f999c-021a-422a-9a20-37213d3f8f5c.json Adds a patch change entry documenting the behavior/logging change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/itwin/property-grid/src/property-grid-react/api/PreferencesStorage.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment thread packages/itwin/property-grid/src/property-grid-react/api/PreferencesStorage.ts Outdated
@grigasp grigasp enabled auto-merge (squash) June 12, 2026 08:48
@grigasp grigasp merged commit 39a5632 into master Jun 12, 2026
13 checks passed
@grigasp grigasp deleted the copilot/fix-usenullvaluestorage-error branch June 12, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants