fix(sync): 「全部历史」真的拉全部历史,自动同步不再每次静默失败
四个独立的 bug 叠在一起,表现为「只同步两天、没有进度」:
* 前端 `...(days ? { days } : {})` 把 days=0 当成未传。「全部历史」
存的就是 0,请求体里根本没有 days,后端退回 7 天默认值。
* scheduler 用 `s[0]` 读 query_one 返回的 dict,抛 KeyError 后被
per-account 的 except 吞掉。只要用户存过一次设置,每 30 分钟的
自动同步就一次都没成功过——库里那 2 天全是手动点出来的。
* 增量同步查 `health_daily`(表其实叫 health_data),后台线程直接
死掉,状态永远卡在 syncing,进度条不动。
* UI 完全不看 /sync 的返回值,rate_limited 时按钮点了没反应;轮询
结束时又把 rate_limited 归进 else 分支报「同步完成」。
顺带:
* 日循环遇到 429 立即退避并保留已拉到的天数,而不是当成「跳过一天」
继续往下捶 700 天——这正是之前限流死循环的来源之一。
* 定时循环显式传 SYNC_DAYS。历史范围按 UI 文案只描述手动全量同步,
让半小时一次的 tick 重拉 730 天必然把限流撞得更深。
* 短同步逐天上报进度(原来每 5 天一次,7 天的同步全程停在 0)。
* /sync 路由重复解析 body,空 body 会 None.get 崩。
* 4 个 StubGarth 缺 configure(),7 个测试在此之前一直是红的。
新增 deploy/push.sh:NAS 只认密码,脚本开一个 ssh 复用连接,密码只
输一次,后面推送 / 重启 / 健康检查全走它。不碰 .env、.venv 和数据库。
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -43,7 +43,9 @@ def sync():
|
||||
400,
|
||||
)
|
||||
|
||||
days = request.get_json(silent=True).get("days") if request.is_json else None
|
||||
# `data` is the body already parsed above; re-parsing here used to blow up
|
||||
# on a body-less POST (`None.get`).
|
||||
days = data.get("days")
|
||||
try:
|
||||
if days is not None:
|
||||
days = int(days)
|
||||
|
||||
@@ -1021,8 +1021,14 @@ def sync_data(user_id, creds, days=None, client=None):
|
||||
# -1 means "incremental sync": pick up from the latest date already in the
|
||||
# local database rather than pulling a fixed window.
|
||||
if days == -1:
|
||||
row = query_one("SELECT MAX(date) FROM health_daily WHERE user_id = ?", (user_id,))
|
||||
latest = row[0] if row and row[0] else None
|
||||
# The daily rows live in `health_data`; `health_daily` is the name of
|
||||
# the *writer* (health.upsert_health_daily), not of any table. Reading
|
||||
# it raised inside the background thread, so an incremental sync died
|
||||
# silently and left the status stuck on "syncing".
|
||||
row = query_one(
|
||||
"SELECT MAX(date) AS latest FROM health_data WHERE user_id = ?", (user_id,)
|
||||
)
|
||||
latest = row["latest"] if row and row.get("latest") else None
|
||||
if latest is None:
|
||||
days = DEFAULT_SYNC_DAYS # first sync → fall back to default window
|
||||
else:
|
||||
@@ -1038,12 +1044,21 @@ def sync_data(user_id, creds, days=None, client=None):
|
||||
|
||||
days_synced = 0
|
||||
day_errors = []
|
||||
rate_limited_mid_run = None
|
||||
for i in range(days):
|
||||
date_str = (today - datetime.timedelta(days=i)).isoformat()
|
||||
try:
|
||||
record = _extract_daily(client, date_str)
|
||||
record.update(extras.daily_extras(client, date_str))
|
||||
except Exception as e:
|
||||
# A 429 inside the day loop used to be filed as one more skipped
|
||||
# day, so a 730-day backfill kept hammering Garmin for another
|
||||
# 700 days and drove the throttle deeper. Stand down at the first
|
||||
# one and keep whatever was already stored.
|
||||
if isinstance(e, RateLimited) or _is_rate_limited(e):
|
||||
_note_rate_limit(user_id)
|
||||
rate_limited_mid_run = describe(e)
|
||||
break
|
||||
day_errors.append(f"{date_str}: {describe(e)}")
|
||||
continue
|
||||
# A day Garmin has no data for comes back all-None; storing it would
|
||||
@@ -1060,15 +1075,34 @@ def sync_data(user_id, creds, days=None, client=None):
|
||||
except Exception as e: # noqa: BLE001
|
||||
day_errors.append(f"{date_str} series: {describe(e)}")
|
||||
|
||||
# Reported every few days rather than every day: the write is cheap
|
||||
# but not free, and the UI polls on a 2s cadence anyway.
|
||||
if (i + 1) % 5 == 0 or i + 1 == days:
|
||||
# A long backfill reports every fifth day — the write is cheap but not
|
||||
# free. A short one reports every day: at 7 days a "every 5th" cadence
|
||||
# meant the bar sat at 0 for most of the run and then vanished.
|
||||
if days <= 30 or (i + 1) % 5 == 0 or i + 1 == days:
|
||||
_set_sync_status(
|
||||
user_id, "syncing", now,
|
||||
records_synced=days_synced, progress_current=i + 1,
|
||||
progress_total=days, stage=f"每日数据 {date_str}",
|
||||
)
|
||||
|
||||
if rate_limited_mid_run:
|
||||
until, msg = _rate_limit_block(user_id)
|
||||
message = msg or "Garmin 限制了请求频率,已自动退避。"
|
||||
_set_sync_status(
|
||||
user_id, "rate_limited", now, records_synced=days_synced,
|
||||
progress_current=days_synced, progress_total=days, stage=None,
|
||||
last_error=message,
|
||||
)
|
||||
return {
|
||||
"status": "rate_limited",
|
||||
"recordsSynced": days_synced,
|
||||
"message": f"已同步 {days_synced} 天后被 Garmin 限流:{message}",
|
||||
"retryAfterSeconds": (
|
||||
int((until - datetime.datetime.utcnow()).total_seconds()) if until else None
|
||||
),
|
||||
"lastSyncTime": now,
|
||||
}
|
||||
|
||||
activities_synced = 0
|
||||
_set_sync_status(user_id, "syncing", now, records_synced=days_synced,
|
||||
progress_current=days, progress_total=days,
|
||||
|
||||
@@ -130,7 +130,8 @@ def sync_all_accounts(days=None, respect_schedule=False):
|
||||
|
||||
When `days` is not provided, each account's saved `history_days` from
|
||||
`user_settings` is used (the user's 历史范围 picker), falling back to
|
||||
`SYNC_DAYS`.
|
||||
`SYNC_DAYS`. The scheduled loop always passes `days` explicitly — 历史范围
|
||||
is the window for the manual 全量同步, not for a half-hourly tick.
|
||||
"""
|
||||
rows = query_all("SELECT user_id FROM garmin_tokens")
|
||||
results = []
|
||||
@@ -150,8 +151,14 @@ def sync_all_accounts(days=None, respect_schedule=False):
|
||||
# Resolve the sync window: prefer the user's saved history_days,
|
||||
# then the caller override, then the global default.
|
||||
if days is None:
|
||||
s = query_one("SELECT history_days FROM user_settings WHERE user_id = ?", (uid,))
|
||||
user_days = s[0] if s and s[0] is not None else None
|
||||
# query_one returns a dict, so `s[0]` raised KeyError — caught
|
||||
# by the per-account handler below, which meant every single
|
||||
# scheduled tick failed for every account and nothing was ever
|
||||
# synced automatically.
|
||||
s = query_one(
|
||||
"SELECT history_days FROM user_settings WHERE user_id = ?", (uid,)
|
||||
)
|
||||
user_days = s.get("history_days") if s else None
|
||||
if user_days == 0:
|
||||
user_days = 730 # 全部历史 → 最大范围
|
||||
d = user_days if user_days is not None else SYNC_DAYS
|
||||
@@ -180,7 +187,12 @@ def _loop():
|
||||
try:
|
||||
if claim(interval=TICK_SECONDS):
|
||||
try:
|
||||
sync_all_accounts(respect_schedule=True)
|
||||
# Explicitly the recent window, never the user's 历史范围:
|
||||
# that setting describes the manual 全量同步. Letting a
|
||||
# half-hourly tick re-pull "全部历史" meant 730 days x ~7
|
||||
# Garmin calls every 30 minutes, which is precisely what
|
||||
# kept the account in a 429 loop.
|
||||
sync_all_accounts(days=SYNC_DAYS, respect_schedule=True)
|
||||
finally:
|
||||
release()
|
||||
except Exception as e: # noqa: BLE001 - the loop must outlive any single failure
|
||||
|
||||
@@ -357,6 +357,8 @@ class TestMfaHandling:
|
||||
"""garth's default MFA prompt calls input(); with no stdin that raises
|
||||
a bare EOFError, which says nothing about what to do about it."""
|
||||
class StubGarth:
|
||||
# _connect widens garth's 10s timeout before any call.
|
||||
def configure(self, **kw): pass
|
||||
def loads(self, s): pass
|
||||
def refresh_oauth2(self): pass
|
||||
|
||||
@@ -397,6 +399,9 @@ class TestMfaHandling:
|
||||
class StubGarth:
|
||||
profile = {"displayName": "Tester"}
|
||||
|
||||
# _connect widens garth's 10s timeout before any call.
|
||||
def configure(self, **kw): pass
|
||||
|
||||
def loads(self, s):
|
||||
loaded["blob"] = s
|
||||
|
||||
@@ -419,7 +424,7 @@ class TestMfaHandling:
|
||||
def test_no_token_and_no_password_is_refused_clearly(self, db, user, monkeypatch):
|
||||
class StubGarmin:
|
||||
def __init__(self, *a, **k):
|
||||
self.garth = None
|
||||
self.garth = type("G", (), {"configure": lambda *a, **k: None})()
|
||||
|
||||
monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: StubGarmin)
|
||||
with pytest.raises(RuntimeError, match="缺少 Garmin 密码"):
|
||||
@@ -464,6 +469,9 @@ class TestApiUserAgent:
|
||||
|
||||
class StubGarth:
|
||||
profile = {"displayName": "Tester"}
|
||||
|
||||
# _connect widens garth's 10s timeout before any call.
|
||||
def configure(self, **kw): pass
|
||||
def __init__(self): self.sess = StubSess()
|
||||
def loads(self, s): pass
|
||||
def refresh_oauth2(self): pass
|
||||
@@ -491,6 +499,8 @@ class TestApiUserAgent:
|
||||
request to '.../None'."""
|
||||
class StubGarth:
|
||||
profile = {"displayName": "Tester"}
|
||||
# _connect widens garth's 10s timeout before any call.
|
||||
def configure(self, **kw): pass
|
||||
sess = type("S", (), {"headers": {}})()
|
||||
def loads(self, s): pass
|
||||
def refresh_oauth2(self): pass
|
||||
@@ -689,6 +699,7 @@ class TestRateLimiting:
|
||||
class Stub:
|
||||
def __init__(self, *a, **k):
|
||||
self.garth = type("G", (), {
|
||||
"configure": lambda *a, **k: None,
|
||||
"loads": lambda *a: None,
|
||||
"refresh_oauth2": lambda *a: (_ for _ in ()).throw(
|
||||
AssertionError("must not reach Garmin while backing off")),
|
||||
@@ -710,6 +721,7 @@ class TestRateLimiting:
|
||||
def __init__(self, *a, **k):
|
||||
unexpired = type("T", (), {"expired": False})()
|
||||
self.garth = type("G", (), {
|
||||
"configure": lambda *a, **k: None,
|
||||
"loads": lambda *a: None,
|
||||
"refresh_oauth2": lambda *a: calls.append(1),
|
||||
"oauth2_token": unexpired,
|
||||
@@ -720,3 +732,76 @@ class TestRateLimiting:
|
||||
monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: Stub)
|
||||
garmin_svc._connect({}, user_id=user["id"])
|
||||
assert calls == [], "an unexpired token needs no refresh"
|
||||
|
||||
|
||||
class TestSyncWindow:
|
||||
"""How many days a sync actually covers.
|
||||
|
||||
The 历史范围 picker offers 全部历史 (0) and 自上次同步 (-1); both used to
|
||||
end up pulling the 7-day default instead — the first because the client
|
||||
dropped a falsy `days`, the second because the incremental lookup read a
|
||||
table that does not exist and died inside the background thread.
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
garmin_svc._rate_limited_until.clear()
|
||||
|
||||
def teardown_method(self):
|
||||
garmin_svc._rate_limited_until.clear()
|
||||
|
||||
def test_incremental_resumes_from_the_latest_stored_day(
|
||||
self, db, user, seed_health
|
||||
):
|
||||
seed_health([{"date": day(5), "steps": 4000}])
|
||||
client = StubClient()
|
||||
out = garmin_svc.sync_data(user["id"], CREDS, days=-1, client=client)
|
||||
|
||||
pulled = {c[1] for c in client.calls if c[0] == "summary"}
|
||||
assert out["status"] == "success"
|
||||
assert pulled == {day(i) for i in range(5)}
|
||||
|
||||
def test_incremental_without_history_falls_back_to_the_default(self, db, user):
|
||||
client = StubClient()
|
||||
garmin_svc.sync_data(user["id"], CREDS, days=-1, client=client)
|
||||
pulled = {c[1] for c in client.calls if c[0] == "summary"}
|
||||
assert len(pulled) == garmin_svc.DEFAULT_SYNC_DAYS
|
||||
|
||||
def test_full_history_asks_for_the_maximum_window(self, db, user):
|
||||
"""`days=0` is 全部历史, not "unset"."""
|
||||
client = StubClient()
|
||||
garmin_svc.sync_data(user["id"], CREDS, days=0, client=client)
|
||||
assert len([c for c in client.calls if c[0] == "summary"]) == 730
|
||||
|
||||
def test_the_sync_endpoint_passes_a_zero_through(
|
||||
self, client, auth, user, db, monkeypatch
|
||||
):
|
||||
garmin_svc.save_token(user["id"], "blob")
|
||||
seen = {}
|
||||
|
||||
def record(uid, creds, days=None):
|
||||
seen["days"] = days
|
||||
return {"status": "syncing", "days": days}
|
||||
|
||||
monkeypatch.setattr(garmin_svc, "start_sync", record)
|
||||
r = client.post("/api/garmin/sync", headers=auth, json={"days": 0})
|
||||
assert r.status_code == 202
|
||||
assert seen["days"] == 730, "全部历史 must not fall back to the default"
|
||||
|
||||
def test_a_429_mid_run_stops_the_sync(self, db, user):
|
||||
"""Filing a 429 as one more skipped day meant a 730-day backfill kept
|
||||
hammering Garmin for another 700 days and dug the throttle deeper."""
|
||||
|
||||
class Throttled(StubClient):
|
||||
def get_user_summary(self, cdate):
|
||||
self.calls.append(("summary", cdate))
|
||||
if cdate == day(2):
|
||||
raise RuntimeError("too many 429 error responses")
|
||||
return summary()
|
||||
|
||||
client = Throttled()
|
||||
out = garmin_svc.sync_data(user["id"], CREDS, days=365, client=client)
|
||||
|
||||
assert out["status"] == "rate_limited"
|
||||
assert out["recordsSynced"] == 2, "the days pulled before the 429 are kept"
|
||||
assert len([c for c in client.calls if c[0] == "summary"]) == 3
|
||||
assert garmin_svc.rate_limited_until(user["id"]) is not None
|
||||
|
||||
@@ -184,3 +184,58 @@ class TestEndpoints:
|
||||
r = client.get("/api/garmin/auto-sync", headers=auth)
|
||||
assert r.status_code == 200
|
||||
assert "intervalSeconds" in r.get_json()
|
||||
|
||||
|
||||
class TestSyncWindowResolution:
|
||||
"""Which window each account is synced over.
|
||||
|
||||
`query_one` returns a dict, so reading the saved 历史范围 as `s[0]` raised
|
||||
KeyError — swallowed by the per-account handler, which meant every
|
||||
scheduled tick failed for every account that had ever opened 设置, and
|
||||
nothing was synced automatically at all.
|
||||
"""
|
||||
|
||||
def _account(self, user, monkeypatch):
|
||||
from services import garmin as garmin_svc
|
||||
|
||||
garmin_svc.save_token(user["id"], "t1")
|
||||
seen = {}
|
||||
|
||||
def record(uid, creds, days=None, client=None):
|
||||
seen["days"] = days
|
||||
return {"status": "success", "recordsSynced": 0}
|
||||
|
||||
monkeypatch.setattr(garmin_svc, "sync_data", record)
|
||||
return seen
|
||||
|
||||
def test_saved_history_days_is_honoured(self, db, user, monkeypatch):
|
||||
from services import settings as settings_svc
|
||||
|
||||
seen = self._account(user, monkeypatch)
|
||||
settings_svc.save_settings(user["id"], {"historyDays": 90})
|
||||
|
||||
results = scheduler.sync_all_accounts()
|
||||
assert [r["status"] for r in results] == ["success"]
|
||||
assert seen["days"] == 90
|
||||
|
||||
def test_full_history_becomes_the_maximum(self, db, user, monkeypatch):
|
||||
from services import settings as settings_svc
|
||||
|
||||
seen = self._account(user, monkeypatch)
|
||||
settings_svc.save_settings(user["id"], {"historyDays": 0})
|
||||
|
||||
scheduler.sync_all_accounts()
|
||||
assert seen["days"] == 730
|
||||
|
||||
def test_the_scheduled_tick_ignores_the_history_setting(
|
||||
self, db, user, monkeypatch
|
||||
):
|
||||
"""历史范围 is the window for the manual 全量同步. A half-hourly tick
|
||||
re-pulling 全部历史 is what kept the account in a 429 loop."""
|
||||
from services import settings as settings_svc
|
||||
|
||||
seen = self._account(user, monkeypatch)
|
||||
settings_svc.save_settings(user["id"], {"historyDays": 0, "autoSync": True})
|
||||
|
||||
scheduler.sync_all_accounts(days=scheduler.SYNC_DAYS, respect_schedule=True)
|
||||
assert seen["days"] == scheduler.SYNC_DAYS
|
||||
|
||||
Reference in New Issue
Block a user