errors
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -26,6 +26,8 @@
|
||||
</div>
|
||||
{% if row.action_reason == "garmin_mfa_required" %}
|
||||
<span class="action-required">Garmin MFA required — <a href="/users/{{ row.id }}">resolve</a></span>
|
||||
{% elif row.action_reason == "mywhoosh_device_conflict" %}
|
||||
<span class="action-required">MyWhoosh account logged in on another device — log out there, then <a href="/users/{{ row.id }}">retry</a></span>
|
||||
{% endif %}
|
||||
</div>
|
||||
<div class="user-actions">
|
||||
|
||||
@@ -39,6 +39,11 @@
|
||||
<div class="card">
|
||||
{% include "fragments/mfa_form.html" %}
|
||||
</div>
|
||||
{% elif user.action_reason == "mywhoosh_device_conflict" %}
|
||||
<h2>MyWhoosh device conflict</h2>
|
||||
<div class="card">
|
||||
<p>MyWhoosh reports this account is already logged in on another device. Log out of MyWhoosh there (app or website), then retry the sync.</p>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
<h2>Recent sync runs</h2>
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user