Skip to content

fix(appconfig): make type-conflict error self-explanatory - #63450

Merged
susnux merged 1 commit into
masterfrom
fix/config/lexicon-type-mismatch-message
Aug 25, 2026
Merged

fix(appconfig): make type-conflict error self-explanatory#63450
susnux merged 1 commit into
masterfrom
fix/config/lexicon-type-mismatch-message

Conversation

@ChristophWurst

@ChristophWurst ChristophWurst commented Aug 21, 2026

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

When a stored app config value has a different type than the one an app requests via a typed getter, getTypedValue() threw the opaque message 'conflict with value type from database' with no app, key or type information. Admins had no way to tell which key was affected or why. Example: nextcloud/mail#13537.

Name the config key and both types (using convertTypeToString(), like the setTypedValue() throw already does) in the log line and exception message.

Before:

Technical details

    Remote Address: 127.0.0.1
    Request ID: WW1YSUk264BSTx35WFtG
    Type: OCP\Exceptions\AppConfigTypeConflictException
    Code: 0
    Message: conflict with value type from database
    File: /nextcloud/lib/private/AppConfig.php
    Line: 542

After:

Technical details

    Remote Address: 127.0.0.1
    Request ID: hJHOpVUa9PD4YKLrkV9m
    Type: OCP\Exceptions\AppConfigTypeConflictException
    Code: 0
    Message: Config value mail/importance_classification_default is stored as string but was requested as boolean
    File: /nextcloud/lib/private/AppConfig.php
    Line: 549

TODO

  • Do

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

When a stored app config value has a different type than the one an app requests via a typed getter, getTypedValue() threw the opaque message 'conflict with value type from database' with no app, key or type information. Admins had no way to tell which key was affected or why.

Name the config key and both types (using convertTypeToString(), like the setTypedValue() throw already does) in the log line and exception message.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
@ChristophWurst
ChristophWurst requested a review from a team as a code owner August 21, 2026 09:27
@ChristophWurst ChristophWurst added bug 3. to review Waiting for reviews labels Aug 21, 2026
@ChristophWurst
ChristophWurst requested review from Altahrim, come-nc, icewind1991 and provokateurin and removed request for a team August 21, 2026 09:27
@ChristophWurst

Copy link
Copy Markdown
Member Author

/backport to stable34

@ChristophWurst

Copy link
Copy Markdown
Member Author

/backport to stable33

@ChristophWurst

Copy link
Copy Markdown
Member Author

/backport to stable32

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 23, 2026
Comment thread lib/private/AppConfig.php
Comment on lines +543 to +544
$requestedType = $this->convertTypeToString($type);
$storedType = $this->convertTypeToString($knownType);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should each of them have it's own try-catch block?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Used the same snippet as

/**
* we only accept a different type from the one stored in database
* if the one stored in database is not-defined (VALUE_MIXED)
*/
if (!$this->isTyped(self::VALUE_MIXED, $currType)
&& ($type | self::VALUE_SENSITIVE) !== ($currType | self::VALUE_SENSITIVE)) {
try {
$currType = $this->convertTypeToString($currType);
$type = $this->convertTypeToString($type);
} catch (AppConfigIncorrectTypeException) {
// can be ignored, this was just needed for a better exception message.
}
throw new AppConfigTypeConflictException('conflict between new type (' . $type . ') and old type (' . $currType . ')');
}
so not sure

@skjnldsv skjnldsv mentioned this pull request Aug 25, 2026
@ChristophWurst

Copy link
Copy Markdown
Member Author

/backport to stable35

@susnux
susnux merged commit 9e2be5d into master Aug 25, 2026
261 of 264 checks passed
@susnux
susnux deleted the fix/config/lexicon-type-mismatch-message branch August 25, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish AI assisted bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants