Skip to content

fix(stores): a corrupt state file must not stop server startup - #836

Open
TheeCoderAhmed wants to merge 1 commit into
andrewyng:mainfrom
TheeCoderAhmed:fix/corrupt-state-files-startup
Open

TheeCoderAhmed wants to merge 1 commit into
andrewyng:mainfrom
TheeCoderAhmed:fix/corrupt-state-files-startup

Conversation

@TheeCoderAhmed

Copy link
Copy Markdown

Part of #204.

Failure. Six JSON stores read their file with an unguarded json.loads in the constructor, and SessionManager.__init__ builds them all at startup. One truncated file (a save cut off by a crash or a full disk) raises straight out of the constructor, so the server won't start until the user finds and deletes the file. To reproduce:

from pathlib import Path
from coworker.unrouted import UnroutedStore

Path("state.json").write_text('{"items": [')
UnroutedStore("state.json")
# json.decoder.JSONDecodeError: Expecting value: line 1 column 12 (char 11)

Fix. InboxStore, InboxRouting, UnattendedRegistry, WakeStore, SubscriptionStore and UnroutedStore now use the same guard as MentionSessionStore (#690): catch (OSError, ValueError, TypeError, AttributeError), log a warning, and start empty. A half-read file doesn't leave partial state behind.

Left out on purpose:

WakeStore is in scope because selfwake.py still has the unguarded load on main, even though #159 was closed with "main already has the logic".

Validation. New tests/test_store_robustness.py covers a truncated file, an empty file, null, a JSON string, and records with unknown fields.

main:         27 failed, 1 passed
this branch:  28 passed

pytest tests/test_store_robustness.py tests/test_inbox.py tests/test_inbox_routing.py \
  tests/test_self_wake.py tests/test_subscriptions.py tests/test_unattended.py \
  tests/test_attendance_auto.py tests/test_wake_resume.py tests/test_mention_store_robustness.py
105 passed

I ran those focused files with .[dev] on Python 3.12, not the full suite. A screenshot doesn't apply: this has no UI change.

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.

1 participant