Prevent server crash on malformed player config entries#3967
Merged
Conversation
A race between delete_player_config() and a stale player update_state() could leave a partial player config in settings.json (only default_name and values, no player_id). The next config/players read then crashed with KeyError: 'player_id', breaking the players UI. Harden both sides: - get_player_configs / get_player_config tolerate (and auto-clean) entries missing the player_id base key. - set_player_default_name and set_player_type no-op when the player config root has been deleted, so a late update can't resurrect a partial entry via nested set(). - remove_provider_config cleanup loop uses .get() instead of bracket access, so it survives the same kind of partial entry.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens player configuration persistence against partially-written/malformed entries in settings.json, preventing crashes in the players UI after a race between config deletion and stale player state updates.
Changes:
- Auto-detect and remove malformed player config entries missing
player_idduringget_player_configs, with warning logs. - Make
get_player_configmore tolerant of missingplayer_idfor unavailable players by defaulting/sanitizing fields. - Prevent
set_player_default_nameandset_player_typefrom recreating partial config roots after deletion; make provider-config cleanup resilient to partial entries.
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.
What does this implement/fix?
A race between
delete_player_config()and a stale playerupdate_state()could leave a partial player config insettings.json(onlydefault_nameandvalues, noplayer_id). On the next read,get_player_configscrashed withKeyError: 'player_id', breaking the players UI until the file was manually repaired.Changes:
get_player_configs/get_player_confignow tolerate (and auto-clean) entries missing theplayer_idbase key, with a warning log.set_player_default_nameandset_player_typeno-op when the player config root has been deleted, so a lateupdate_statecan't resurrect a partial entry via nestedset().remove_provider_configcleanup loop uses.get()instead of bracket access, so it survives the same kind of partial entry.Related issue (if applicable):
Types of changes
bugfixnew-featureenhancementnew-providerbreaking-changerefactordocumentationmaintenancecidependenciesChecklist