From 7c9e19ba0b0a64f9ad27ab66949c63774e92e3ac Mon Sep 17 00:00:00 2001 From: Bastian Wagner Date: Sat, 15 Aug 2026 20:31:57 +0200 Subject: [PATCH] errors --- app/mywhoosh/client.py | 11 ++++++-- app/mywhoosh/tokenstore.py | 15 ++++++++++ app/sync/manager.py | 25 +++++++++++++++-- app/web/templates/dashboard.html | 2 ++ app/web/templates/users/detail.html | 5 ++++ tests/mywhoosh/test_client_auth.py | 39 ++++++++++++++++++++++++++ tests/mywhoosh/test_tokenstore.py | 11 ++++++++ tests/sync/test_manager.py | 43 ++++++++++++++++++++++++++++- 8 files changed, 146 insertions(+), 5 deletions(-) diff --git a/app/mywhoosh/client.py b/app/mywhoosh/client.py index 4b645f9..7e1050a 100644 --- a/app/mywhoosh/client.py +++ b/app/mywhoosh/client.py @@ -20,6 +20,10 @@ class MyWhooshAuthError(MyWhooshError): pass +class MyWhooshDeviceConflictError(MyWhooshAuthError): + pass + + class MyWhooshTransientError(MyWhooshError): pass @@ -52,7 +56,7 @@ class MyWhooshClient: "Platform": "Android", "Action": 1001, "CorrelationId": str(uuid.uuid4()), - "DeviceId": str(uuid.uuid4()), + "DeviceId": self.token_store.get_or_create_device_id(), "Authorization": "", } try: @@ -70,7 +74,10 @@ class MyWhooshClient: if not isinstance(body, dict): raise MyWhooshIntegrationError("MyWhoosh login response is not a JSON object") if body.get("Success") is not True or not body.get("AccessToken"): - raise MyWhooshAuthError(str(body.get("Message") or "MyWhoosh login failed")) + message = str(body.get("Message") or "MyWhoosh login failed") + if "another device" in message.lower(): + raise MyWhooshDeviceConflictError(message) + raise MyWhooshAuthError(message) self.token = MyWhooshToken( access_token=str(body["AccessToken"]), refresh_token=str(body["RefreshToken"]) if body.get("RefreshToken") else None, diff --git a/app/mywhoosh/tokenstore.py b/app/mywhoosh/tokenstore.py index 347bea2..f2645e5 100644 --- a/app/mywhoosh/tokenstore.py +++ b/app/mywhoosh/tokenstore.py @@ -1,5 +1,6 @@ import json import os +import uuid from pathlib import Path from app.mywhoosh.models import MyWhooshToken @@ -8,6 +9,7 @@ from app.mywhoosh.models import MyWhooshToken class MyWhooshTokenStore: def __init__(self, path: Path) -> None: self.path = path + self.device_id_path = path.with_name("device_id") def load(self) -> MyWhooshToken | None: try: @@ -40,3 +42,16 @@ class MyWhooshTokenStore: def clear(self) -> None: self.path.unlink(missing_ok=True) + + def get_or_create_device_id(self) -> str: + try: + existing = self.device_id_path.read_text("utf-8").strip() + except (FileNotFoundError, OSError): + existing = "" + if existing: + return existing + device_id = str(uuid.uuid4()) + self.device_id_path.parent.mkdir(parents=True, exist_ok=True, mode=0o700) + self.device_id_path.write_text(device_id, "utf-8") + os.chmod(self.device_id_path, 0o600) + return device_id diff --git a/app/sync/manager.py b/app/sync/manager.py index 1c3c4cc..6429687 100644 --- a/app/sync/manager.py +++ b/app/sync/manager.py @@ -14,7 +14,12 @@ from app.garmin.uploader import ( GarminTransientError, GarminUploadBlocked, ) -from app.mywhoosh.client import MyWhooshAuthError, MyWhooshIntegrationError, MyWhooshTransientError +from app.mywhoosh.client import ( + MyWhooshAuthError, + MyWhooshDeviceConflictError, + MyWhooshIntegrationError, + MyWhooshTransientError, +) from app.mywhoosh.tokenstore import MyWhooshTokenStore from app.security.credentials import CredentialCipher from app.sync.states import SyncOutcome @@ -30,7 +35,9 @@ class SyncAlreadyRunning(RuntimeError): # succeeding (reaching the end of the activity loop with stop_user_run still # False already proves MyWhoosh is working again -- every MyWhoosh-related # exception branch that could fire also sets stop_user_run=True). -_MYWHOOSH_ACTION_REASONS = frozenset({"mywhoosh_auth_required", "mywhoosh_integration_changed"}) +_MYWHOOSH_ACTION_REASONS = frozenset( + {"mywhoosh_auth_required", "mywhoosh_integration_changed", "mywhoosh_device_conflict"} +) # action_reason values that can only be resolved by actual, this-run evidence # of a successful Garmin import -- the mere absence of a Garmin exception does @@ -136,6 +143,14 @@ class SyncManager: remote_activities = [] stop_user_run = True summary_error = str(exc) + except MyWhooshDeviceConflictError as exc: + user.health_state = HealthState.ACTION_REQUIRED + user.mywhoosh_state = "device_conflict" + user.action_reason = "mywhoosh_device_conflict" + session.commit() + remote_activities = [] + stop_user_run = True + summary_error = str(exc) except MyWhooshAuthError as exc: user.health_state = HealthState.ACTION_REQUIRED user.mywhoosh_state = "auth_required" @@ -245,6 +260,12 @@ class SyncManager: user.mywhoosh_state = "error" activity_repo.mark_failed(activity.id, str(exc), retryable=True) failed_count += 1 + except MyWhooshDeviceConflictError as exc: + user.health_state = HealthState.ACTION_REQUIRED + user.mywhoosh_state = "device_conflict" + user.action_reason = "mywhoosh_device_conflict" + stop_user_run = True + summary_error = str(exc) except MyWhooshAuthError as exc: user.health_state = HealthState.ACTION_REQUIRED user.mywhoosh_state = "auth_required" diff --git a/app/web/templates/dashboard.html b/app/web/templates/dashboard.html index a94bc8f..7b700e1 100644 --- a/app/web/templates/dashboard.html +++ b/app/web/templates/dashboard.html @@ -26,6 +26,8 @@ {% if row.action_reason == "garmin_mfa_required" %} Garmin MFA required — resolve + {% elif row.action_reason == "mywhoosh_device_conflict" %} + MyWhoosh account logged in on another device — log out there, then retry {% endif %}
diff --git a/app/web/templates/users/detail.html b/app/web/templates/users/detail.html index a04f042..5e5c841 100644 --- a/app/web/templates/users/detail.html +++ b/app/web/templates/users/detail.html @@ -39,6 +39,11 @@
{% include "fragments/mfa_form.html" %}
+{% elif user.action_reason == "mywhoosh_device_conflict" %} +

MyWhoosh device conflict

+
+

MyWhoosh reports this account is already logged in on another device. Log out of MyWhoosh there (app or website), then retry the sync.

+
{% endif %}

Recent sync runs

diff --git a/tests/mywhoosh/test_client_auth.py b/tests/mywhoosh/test_client_auth.py index 37908ec..9681f17 100644 --- a/tests/mywhoosh/test_client_auth.py +++ b/tests/mywhoosh/test_client_auth.py @@ -1,9 +1,12 @@ +import json + import httpx import pytest from app.mywhoosh.client import ( MyWhooshAuthError, MyWhooshClient, + MyWhooshDeviceConflictError, MyWhooshIntegrationError, MyWhooshTransientError, ) @@ -44,6 +47,42 @@ async def test_invalid_credentials_raise_auth_error(tmp_path) -> None: await client.login("rider@example.com", "bad") +@pytest.mark.asyncio +async def test_device_conflict_message_raises_device_conflict_error(tmp_path) -> None: + async def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, json={"Success": False, "Message": "You are already logged in from another device."} + ) + + client = MyWhooshClient( + MyWhooshTokenStore(tmp_path / "mywhoosh.json"), + http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler)), + ) + with pytest.raises(MyWhooshDeviceConflictError): + await client.login("rider@example.com", "secret") + + +@pytest.mark.asyncio +async def test_login_reuses_stable_device_id_across_calls(tmp_path) -> None: + seen_device_ids = [] + + async def handler(request: httpx.Request) -> httpx.Response: + seen_device_ids.append(json.loads(request.content)["DeviceId"]) + return httpx.Response( + 200, + json={"Success": True, "AccessToken": "access", "RefreshToken": "refresh", "WhooshId": "w-1"}, + ) + + store = MyWhooshTokenStore(tmp_path / "mywhoosh.json") + client = MyWhooshClient(store, http_client=httpx.AsyncClient(transport=httpx.MockTransport(handler))) + await client.login("rider@example.com", "secret") + await client.login("rider@example.com", "secret") + + assert len(seen_device_ids) == 2 + assert seen_device_ids[0] == seen_device_ids[1] + assert seen_device_ids[0] == store.get_or_create_device_id() + + @pytest.mark.asyncio async def test_login_returns_json_array_raises_integration_error(tmp_path) -> None: async def handler(request: httpx.Request) -> httpx.Response: diff --git a/tests/mywhoosh/test_tokenstore.py b/tests/mywhoosh/test_tokenstore.py index 5c33a66..e1bb350 100644 --- a/tests/mywhoosh/test_tokenstore.py +++ b/tests/mywhoosh/test_tokenstore.py @@ -41,3 +41,14 @@ def test_clear_removes_token_and_load_returns_none(tmp_path: Path) -> None: assert store.load() is None assert not store.path.exists() + + +def test_get_or_create_device_id_persists_and_is_stable(tmp_path: Path) -> None: + path = tmp_path / "tokens" / "7" / "mywhoosh.json" + device_id = MyWhooshTokenStore(path).get_or_create_device_id() + + reloaded_id = MyWhooshTokenStore(path).get_or_create_device_id() + + assert reloaded_id == device_id + device_id_path = path.with_name("device_id") + assert device_id_path.read_text("utf-8").strip() == device_id diff --git a/tests/sync/test_manager.py b/tests/sync/test_manager.py index da97af0..f367d57 100644 --- a/tests/sync/test_manager.py +++ b/tests/sync/test_manager.py @@ -6,10 +6,11 @@ from sqlalchemy import select from app.db.models import ActivityStatus, HealthState, SyncRun, SyncRunStatus, SyncUser from app.db.repositories import UserRepository from app.garmin.uploader import UploadResult +from app.mywhoosh.client import MyWhooshDeviceConflictError from app.mywhoosh.models import MyWhooshActivity from app.sync.manager import SyncManager from tests.sync.conftest import FakeFitConverter, _create_user -from tests.sync.fakes import FakeMyWhooshClient +from tests.sync.fakes import FakeGarminUploader, FakeMyWhooshClient @pytest.mark.asyncio @@ -188,6 +189,46 @@ async def test_garmin_action_required_cleared_after_successful_import( assert reloaded.health_state == HealthState.HEALTHY +class DeviceConflictMyWhooshClient: + """Fake MyWhoosh client that always raises MyWhooshDeviceConflictError + from list_activities, simulating MyWhoosh's "already logged in from + another device" response.""" + + async def list_activities(self, email: str, password: str): + raise MyWhooshDeviceConflictError("You are already logged in from another device.") + + async def download_fit(self, activity_file_id: str, email: str, password: str) -> bytes: + raise AssertionError("download_fit should not be reached in this test") + + +@pytest.mark.asyncio +async def test_device_conflict_sets_distinct_action_reason(seeded_user: SyncUser, session_factory, cipher, settings) -> None: + """A MyWhoosh device-conflict response must be distinguishable in the UI + from a generic auth failure, so users get an actionable hint instead of + being told to re-check their password.""" + mywhoosh = DeviceConflictMyWhooshClient() + converter = FakeFitConverter() + garmin = FakeGarminUploader() + manager = SyncManager( + session_factory=session_factory, + credential_cipher=cipher, + settings=settings, + mywhoosh_factory=lambda token_store: mywhoosh, + garmin_factory=lambda email, password, tokenstore: garmin, + fit_converter=converter, + ) + + outcome = await manager.sync_user(seeded_user.id) + assert outcome.status == "failed" + assert "another device" in outcome.message + + with session_factory() as session: + reloaded = UserRepository(session).get(seeded_user.id) + assert reloaded.action_reason == "mywhoosh_device_conflict" + assert reloaded.mywhoosh_state == "device_conflict" + assert reloaded.health_state == HealthState.ACTION_REQUIRED + + @pytest.mark.asyncio async def test_mfa_code_reaches_real_garmin_uploader_via_sync_manager( session_factory, cipher, settings, seeded_user: SyncUser