Non-atomic settings save + unvalidated load permanently break rivalcfg after an interrupted write

[krishna3554]

Summary

The device-settings file (~/.config/rivalcfg/<vid>_<pid>.device.json) is written non-atomically and loaded without any error handling or structural validation. An interrupted save (power loss, crash, Ctrl-C) leaves a truncated/empty JSON file, after which every rivalcfg invocation fails with an unhandled json.JSONDecodeError until the user manually deletes the file. A syntactically valid but structurally incomplete file (e.g. {}) breaks set()/get() differently — with KeyError: 'default' — because _load() never ensures the active profile key exists.

Location

• File: rivalcfg/mouse_settings.py
• save(): non-atomic write
• _load(): no JSONDecodeError handling, no profile-key validation
• set() / get(): assume self._settings[self._current_profile_name] exists

def save(self):
    settings_dir = os.path.dirname(self._settings_path)
    if not os.path.isdir(settings_dir):
        os.makedirs(settings_dir)
    with open(self._settings_path, "w") as file_:   # truncates existing file first
        json.dump(self._settings, file_, indent=2)

def _load(self):
    if os.path.isfile(self._settings_path):
        with open(self._settings_path, "r") as file_:
            self._settings = json.load(file_)       # raises on truncated file; accepts {}
    else:
        self._settings = {"default": self.get_default_values()}

Problem

1. Non-atomic save: open(path, "w") truncates the file before writing. Any interruption between truncation and the completion of json.dump leaves a zero-byte or partial file. save() is called by Mouse.save() right after sending a HID command to the device, i.e. during ordinary use.
2. No load-time recovery: _load() propagates JSONDecodeError, and this happens inside get_mouse_settings(...) during startup (get_mouse() → MouseSettings.__init__ → _load()), before any CLI command can intervene. There is no fallback to defaults, no rename-aside, no warning.
3. Missing profile key check: when the file parses but does not contain the current profile name (current_profile_name="default"), line 137 of set() / line 157 of get() execute self._settings[self._current_profile_name][setting_name] and raise KeyError: 'default'. A valid-but-empty {} file reaches exactly this state.

Trigger / Reproduction

Static analysis finding — behavior derived from the code paths above, not confirmed by execution:

• Truncation case: interrupt the process (SIGKILL / power loss) while save() is writing. Next run: every command (including --list) fails with json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0).
• Empty-object case: write {} to the settings file, then run any rivalcfg --some-setting … command: KeyError: 'default' from MouseSettings.set.

Expected Behavior

A corrupt or incomplete user-settings file should degrade gracefully: warn, move the file aside (or ignore it), and fall back to factory defaults — never permanently brick the CLI. The save itself should be atomic (write temp file + os.replace).

Actual Behavior

Startup crashes with a raw traceback on every subsequent run until the file is manually removed.

Impact

One unlucky interruption during a routine settings save turns rivalcfg unusable for that device for non-expert users (the fix — deleting an obscure file under ~/.config/rivalcfg/ — is not discoverable from the traceback).

Suggested Direction

1. In save(): serialize to a temporary file in the same directory, then atomically os.replace() it over the target.
2. In _load(): wrap json.load() in a try/except that logs a warning and falls back to {"default": self.get_default_values()}; additionally self._settings.setdefault(self._current_profile_name, {}) so a structurally-valid file without the active profile still works.

Happy to provide more detail if useful.

[flozz]

This part of the code is still work in progress; as you can see, most methods of the class are raising a NotImplementedError. Also, as the probability of an interruption while writing the few bytes of JSON is very low, it will stay as is for now and will be improved later, when I will work again on this part of the code.