fix: 本地限流估算不再拦截同步——一律真实请求 Garmin,仅真实 429 记录冷却并退避
此前 sync_data/start_sync/_connect/scheduler 四处会在请求前按本地 rate_limited_until 估算直接拒绝同步,用户看到'立即同步→被限流'实际是 本地拦截、零请求。若 Garmin 已恢复,陈旧估算会让账号一直闲置。 2026-09-02 起冷却只是信息不是闸门: - start_sync / sync_data 开头移除本地拒绝,_connect 移除冷却提前抛错, scheduler 不再跳过冷却中的账号 - 真实 429(refresh_oauth2 / 拉取中途)仍 _note_rate_limit 写 24h 冷却 并 stand down,中途退避分支保留用冷却算恢复时间 - 成功收尾 _clear_rate_limit 退休陈旧冷却,避免误导后续诊断 - 测试:blocked→仍会真实请求;stale cooldown→healthy connect 放行; 真实 429 mid-run 仍记 rate_limited;成功清除冷却(596 passed)
This commit is contained in:
@@ -185,11 +185,15 @@ class RateLimited(RuntimeError):
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
# Retrying while rate limited is what deepens the limit, so once Garmin says
|
# Garmin answering 429 is what deepens the limit: the account once stayed
|
||||||
# 429 the whole process stands down until this passes. The account was stuck for
|
# stuck for days because every retry re-hit the throttle before its own
|
||||||
# days because the backoff kept expiring before Garmin's own (multi-hour) window
|
# (multi-hour) window closed. We therefore record a 24h cooldown when a 429
|
||||||
# closed, so every tick re-hit it and the limit never lifted. A 24h stand-down
|
# actually arrives, so the UI and the sync history can say when to expect
|
||||||
# is what actually outlasts the throttle and lets the window close for good.
|
# recovery. The cooldown is *information*, not a gate: since 2026-09-02 the
|
||||||
|
# local estimate is no longer used to refuse a sync before it starts — every
|
||||||
|
# entry point (manual, 立即同步 and the automatic scheduler alike) issues a
|
||||||
|
# real request and trusts Garmin's live answer. Only a real 429 writes a new
|
||||||
|
# cooldown; a stale local estimate must never keep a healthy account idle.
|
||||||
RATE_LIMIT_BACKOFF = datetime.timedelta(hours=24)
|
RATE_LIMIT_BACKOFF = datetime.timedelta(hours=24)
|
||||||
# In-process cache of the cooldown, kept in sync with the DB copy below and
|
# In-process cache of the cooldown, kept in sync with the DB copy below and
|
||||||
# still the lever the tests reach for via _rate_limited_until.clear().
|
# still the lever the tests reach for via _rate_limited_until.clear().
|
||||||
@@ -263,11 +267,29 @@ def _note_rate_limit(user_id):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _rate_limit_block(user_id):
|
def _clear_rate_limit(user_id):
|
||||||
"""If the account is cooling down, return (until, message); else (None, None).
|
"""Drop a recorded cooldown after a sync that actually succeeded.
|
||||||
|
|
||||||
Central guard used by both the manual and scheduled sync entry points, so a
|
A live success is proof Garmin stopped throttling, so the estimate has
|
||||||
blocked account issues zero Garmin requests until the window closes.
|
served its purpose; leaving a future deadline behind would just mislead
|
||||||
|
the next diagnosis.
|
||||||
|
"""
|
||||||
|
_rate_limited_until.pop(user_id, None)
|
||||||
|
try:
|
||||||
|
execute(
|
||||||
|
"UPDATE sync_status SET rate_limited_until = NULL "
|
||||||
|
"WHERE user_id = ?", [user_id],
|
||||||
|
)
|
||||||
|
except Exception: # noqa: BLE001 - clearing is best-effort
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def _rate_limit_block(user_id):
|
||||||
|
"""Human-readable recovery estimate from the *recorded* cooldown.
|
||||||
|
|
||||||
|
Only meaningful right after a real 429 wrote a fresh cooldown (mid-run
|
||||||
|
stand-down). It is not a pre-request gate — nothing consults the cooldown
|
||||||
|
before calling Garmin any more.
|
||||||
"""
|
"""
|
||||||
until = rate_limited_until(user_id)
|
until = rate_limited_until(user_id)
|
||||||
if not until or datetime.datetime.utcnow() >= until:
|
if not until or datetime.datetime.utcnow() >= until:
|
||||||
@@ -457,13 +479,6 @@ def _connect(creds, user_id=None):
|
|||||||
client.garth.loads(token)
|
client.garth.loads(token)
|
||||||
_use_api_user_agent(client)
|
_use_api_user_agent(client)
|
||||||
|
|
||||||
blocked = rate_limited_until(user_id)
|
|
||||||
if blocked and datetime.datetime.utcnow() < blocked:
|
|
||||||
raise RateLimited(
|
|
||||||
"Garmin 暂时限制了请求频率,稍后会自动恢复(约 "
|
|
||||||
f"{max(1, int((blocked - datetime.datetime.utcnow()).total_seconds() // 60))} 分钟)。"
|
|
||||||
)
|
|
||||||
|
|
||||||
# Only when it has actually expired. Refreshing on every connect spends
|
# Only when it has actually expired. Refreshing on every connect spends
|
||||||
# quota for nothing, and that is what walked the account into a 429.
|
# quota for nothing, and that is what walked the account into a 429.
|
||||||
oauth2 = getattr(client.garth, "oauth2_token", None)
|
oauth2 = getattr(client.garth, "oauth2_token", None)
|
||||||
@@ -1047,33 +1062,13 @@ def start_sync(user_id, creds, days=None):
|
|||||||
|
|
||||||
Progress lands in sync_status, which the UI polls; a full backfill runs
|
Progress lands in sync_status, which the UI polls; a full backfill runs
|
||||||
far longer than any sensible HTTP timeout.
|
far longer than any sensible HTTP timeout.
|
||||||
|
|
||||||
|
No local rate-limit estimate is consulted here: every manual start issues
|
||||||
|
a real request and trusts Garmin's live answer. A real 429 writes the
|
||||||
|
cooldown and the run stands down inside sync_data.
|
||||||
"""
|
"""
|
||||||
days = DEFAULT_SYNC_DAYS if days is None else days
|
days = DEFAULT_SYNC_DAYS if days is None else days
|
||||||
now = datetime.datetime.utcnow().isoformat(timespec="seconds")
|
now = datetime.datetime.utcnow().isoformat(timespec="seconds")
|
||||||
until, msg = _rate_limit_block(user_id)
|
|
||||||
if until:
|
|
||||||
# A refused start synced nothing — keep the previous last_sync_time so
|
|
||||||
# the UI's 上次同步 keeps telling the truth (the data is stale since
|
|
||||||
# the last *successful* sync, and that date is what the user needs).
|
|
||||||
row = query_one(
|
|
||||||
"SELECT last_sync_time FROM sync_status WHERE user_id = ?", [user_id]
|
|
||||||
)
|
|
||||||
_set_sync_status(
|
|
||||||
user_id, "rate_limited", now,
|
|
||||||
last_sync_time=row["last_sync_time"] if row else None,
|
|
||||||
records_synced=0, progress_current=0, progress_total=days,
|
|
||||||
started_at=now, last_error=msg,
|
|
||||||
)
|
|
||||||
_log_sync_history(user_id, "manual", days, now, {
|
|
||||||
"status": "rate_limited",
|
|
||||||
"recordsSynced": 0,
|
|
||||||
"message": msg,
|
|
||||||
})
|
|
||||||
return {
|
|
||||||
"status": "rate_limited",
|
|
||||||
"message": msg,
|
|
||||||
"retryAfterSeconds": int((until - datetime.datetime.utcnow()).total_seconds()),
|
|
||||||
}
|
|
||||||
_set_sync_status(
|
_set_sync_status(
|
||||||
user_id, "syncing", now,
|
user_id, "syncing", now,
|
||||||
records_synced=0, progress_current=0, progress_total=days,
|
records_synced=0, progress_current=0, progress_total=days,
|
||||||
@@ -1104,25 +1099,10 @@ def sync_data(user_id, creds, days=None, client=None, trigger="manual"):
|
|||||||
_log_sync_history(user_id, trigger, days, started, result)
|
_log_sync_history(user_id, trigger, days, started, result)
|
||||||
return result
|
return result
|
||||||
|
|
||||||
until, msg = _rate_limit_block(user_id)
|
# No local cooldown gate here (2026-09-02): the recorded rate_limited_until
|
||||||
if until:
|
# is an estimate, and refusing on it kept accounts idle after Garmin had
|
||||||
# A blocked account must issue zero Garmin requests — that is the whole
|
# already recovered. Every run issues a real request; only a real 429
|
||||||
# point. Return immediately without touching the network, and keep the
|
# stands the run down (mid-run, below) and writes a fresh cooldown.
|
||||||
# previous last_sync_time (this was not a sync).
|
|
||||||
row = query_one(
|
|
||||||
"SELECT last_sync_time FROM sync_status WHERE user_id = ?", [user_id]
|
|
||||||
)
|
|
||||||
_set_sync_status(
|
|
||||||
user_id, "rate_limited", now,
|
|
||||||
last_sync_time=row["last_sync_time"] if row else None,
|
|
||||||
records_synced=0, last_error=msg,
|
|
||||||
)
|
|
||||||
return finish({
|
|
||||||
"status": "rate_limited",
|
|
||||||
"recordsSynced": 0,
|
|
||||||
"message": msg,
|
|
||||||
"retryAfterSeconds": int((until - datetime.datetime.utcnow()).total_seconds()),
|
|
||||||
})
|
|
||||||
_set_sync_status(
|
_set_sync_status(
|
||||||
user_id, "syncing", now,
|
user_id, "syncing", now,
|
||||||
records_synced=0, progress_current=0, progress_total=days,
|
records_synced=0, progress_current=0, progress_total=days,
|
||||||
@@ -1331,6 +1311,9 @@ def sync_data(user_id, creds, days=None, client=None, trigger="manual"):
|
|||||||
progress_current=days, progress_total=days, stage=None,
|
progress_current=days, progress_total=days, stage=None,
|
||||||
last_error="; ".join(day_errors[:3]) if day_errors else None,
|
last_error="; ".join(day_errors[:3]) if day_errors else None,
|
||||||
)
|
)
|
||||||
|
# A live success proves Garmin stopped throttling — retire any recorded
|
||||||
|
# cooldown so it cannot mislead a later diagnosis.
|
||||||
|
_clear_rate_limit(user_id)
|
||||||
message = (
|
message = (
|
||||||
f"同步完成,更新 {days_synced} 天数据、{activities_synced} 条运动记录"
|
f"同步完成,更新 {days_synced} 天数据、{activities_synced} 条运动记录"
|
||||||
f"(含 {details_synced} 条详情)、"
|
f"(含 {details_synced} 条详情)、"
|
||||||
|
|||||||
@@ -151,16 +151,10 @@ def sync_all_accounts(days=None, respect_schedule=False):
|
|||||||
"reason": "not due"})
|
"reason": "not due"})
|
||||||
continue
|
continue
|
||||||
d = SYNC_DAYS if days is None else days
|
d = SYNC_DAYS if days is None else days
|
||||||
# Never poke Garmin while it is rate-limiting us — that is exactly
|
# 2026-09-02: the recorded cooldown is no longer consulted before
|
||||||
# what keeps the limit alive. Respect the persisted cooldown and sit
|
# a sync — a stale estimate must not keep a healthy account idle,
|
||||||
# this tick out.
|
# and only Garmin's live answer (a real 429, handled inside
|
||||||
blocked = garmin_svc.rate_limited_until(uid)
|
# sync_data) decides whether the throttle is actually closed.
|
||||||
if blocked and blocked > _now():
|
|
||||||
results.append({
|
|
||||||
"user": uid, "status": "skipped", "reason": "rate-limited",
|
|
||||||
"retryAfterSeconds": int((blocked - _now()).total_seconds()),
|
|
||||||
})
|
|
||||||
continue
|
|
||||||
out = garmin_svc.sync_data(uid, {}, days=d, trigger="auto")
|
out = garmin_svc.sync_data(uid, {}, days=d, trigger="auto")
|
||||||
results.append({"user": uid, "status": out.get("status"),
|
results.append({"user": uid, "status": out.get("status"),
|
||||||
"records": out.get("recordsSynced")})
|
"records": out.get("recordsSynced")})
|
||||||
|
|||||||
@@ -691,18 +691,27 @@ class TestRateLimiting:
|
|||||||
until = garmin_svc.rate_limited_until(user["id"])
|
until = garmin_svc.rate_limited_until(user["id"])
|
||||||
assert until is not None and until > datetime.datetime.utcnow()
|
assert until is not None and until > datetime.datetime.utcnow()
|
||||||
|
|
||||||
def test_a_blocked_account_does_not_call_garmin_again(self, db, user, monkeypatch):
|
def test_a_recorded_cooldown_no_longer_blocks_connect(self, db, user, monkeypatch):
|
||||||
"""The retry is what deepens the limit, so it must not happen."""
|
"""Since 2026-09-02 the cooldown is information, not a gate: a stale
|
||||||
|
local estimate must not keep an account idle after Garmin recovered,
|
||||||
|
so _connect issues the refresh and trusts the live answer. Only a real
|
||||||
|
429 (raised by refresh_oauth2) records a fresh cooldown and surfaces
|
||||||
|
as RateLimited."""
|
||||||
garmin_svc.save_token(user["id"], "token-blob")
|
garmin_svc.save_token(user["id"], "token-blob")
|
||||||
garmin_svc._note_rate_limit(user["id"])
|
garmin_svc._note_rate_limit(user["id"]) # a future deadline is recorded
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def refresh(self): # bound by the stub instance -> receives self
|
||||||
|
calls.append(1)
|
||||||
|
raise RuntimeError("too many 429 error responses")
|
||||||
|
|
||||||
class Stub:
|
class Stub:
|
||||||
def __init__(self, *a, **k):
|
def __init__(self, *a, **k):
|
||||||
self.garth = type("G", (), {
|
self.garth = type("G", (), {
|
||||||
"configure": lambda *a, **k: None,
|
"configure": lambda *a, **k: None,
|
||||||
"loads": lambda *a: None,
|
"loads": lambda *a: None,
|
||||||
"refresh_oauth2": lambda *a: (_ for _ in ()).throw(
|
"refresh_oauth2": refresh,
|
||||||
AssertionError("must not reach Garmin while backing off")),
|
|
||||||
"oauth2_token": None,
|
"oauth2_token": None,
|
||||||
"sess": type("S", (), {"headers": {}})(),
|
"sess": type("S", (), {"headers": {}})(),
|
||||||
})()
|
})()
|
||||||
@@ -710,6 +719,31 @@ class TestRateLimiting:
|
|||||||
monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: Stub)
|
monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: Stub)
|
||||||
with pytest.raises(garmin_svc.RateLimited):
|
with pytest.raises(garmin_svc.RateLimited):
|
||||||
garmin_svc._connect({}, user_id=user["id"])
|
garmin_svc._connect({}, user_id=user["id"])
|
||||||
|
assert calls == [1], "the real request must still be attempted"
|
||||||
|
assert garmin_svc.rate_limited_until(user["id"]) is not None
|
||||||
|
|
||||||
|
def test_a_stale_cooldown_does_not_stop_a_healthy_connect(self, db, user, monkeypatch):
|
||||||
|
"""The mirror image: with a cooldown recorded but Garmin healthy, the
|
||||||
|
refresh succeeds and the connect proceeds (no local refusal)."""
|
||||||
|
garmin_svc.save_token(user["id"], "token-blob")
|
||||||
|
garmin_svc._note_rate_limit(user["id"])
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
class Stub:
|
||||||
|
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,
|
||||||
|
"profile": {"displayName": "x"},
|
||||||
|
"sess": type("S", (), {"headers": {}})(),
|
||||||
|
})()
|
||||||
|
|
||||||
|
monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: Stub)
|
||||||
|
garmin_svc._connect({}, user_id=user["id"])
|
||||||
|
assert calls == [], "an unexpired token needs no refresh"
|
||||||
|
|
||||||
def test_a_valid_token_is_not_refreshed(self, db, user, monkeypatch):
|
def test_a_valid_token_is_not_refreshed(self, db, user, monkeypatch):
|
||||||
"""Refreshing on every connect spends quota for nothing — and that is
|
"""Refreshing on every connect spends quota for nothing — and that is
|
||||||
@@ -902,13 +936,36 @@ class TestSyncHistory:
|
|||||||
assert garmin_svc.get_sync_history(user["id"])[0]["status"] == "rate_limited"
|
assert garmin_svc.get_sync_history(user["id"])[0]["status"] == "rate_limited"
|
||||||
garmin_svc._rate_limited_until.clear()
|
garmin_svc._rate_limited_until.clear()
|
||||||
|
|
||||||
def test_a_refused_sync_is_still_a_record(self, db, user, monkeypatch):
|
def test_a_recorded_cooldown_does_not_refuse_a_sync(self, db, user, monkeypatch):
|
||||||
|
"""The local cooldown is an estimate, not a gate (2026-09-02): a sync
|
||||||
|
with a recorded deadline still runs for real and records its actual
|
||||||
|
outcome — Garmin's live answer decides, not our guess."""
|
||||||
|
future = datetime.datetime.utcnow() + datetime.timedelta(hours=1)
|
||||||
|
monkeypatch.setitem(garmin_svc._rate_limited_until, user["id"], future)
|
||||||
|
garmin_svc.save_token(user["id"], "token-blob")
|
||||||
|
|
||||||
|
out = garmin_svc.sync_data(user["id"], CREDS, days=1, client=StubClient())
|
||||||
|
assert out["status"] == "success", "a stale cooldown must not block the run"
|
||||||
|
row = garmin_svc.get_sync_history(user["id"])[0]
|
||||||
|
assert row["status"] == "success"
|
||||||
|
# A success retires the recorded cooldown.
|
||||||
|
assert garmin_svc.rate_limited_until(user["id"]) is None
|
||||||
|
|
||||||
|
def test_a_real_429_mid_run_is_still_a_rate_limited_record(self, db, user, monkeypatch):
|
||||||
|
"""Only a genuine 429 stands a run down — and it writes a fresh
|
||||||
|
cooldown instead of relying on whatever stale estimate was there."""
|
||||||
future = datetime.datetime.utcnow() + datetime.timedelta(hours=1)
|
future = datetime.datetime.utcnow() + datetime.timedelta(hours=1)
|
||||||
monkeypatch.setitem(garmin_svc._rate_limited_until, user["id"], future)
|
monkeypatch.setitem(garmin_svc._rate_limited_until, user["id"], future)
|
||||||
|
|
||||||
out = garmin_svc.sync_data(user["id"], CREDS, days=1, client=StubClient())
|
class Throttled(StubClient):
|
||||||
|
def get_user_summary(self, cdate):
|
||||||
|
self.calls.append(("summary", cdate))
|
||||||
|
raise RuntimeError("too many 429 error responses")
|
||||||
|
|
||||||
|
out = garmin_svc.sync_data(user["id"], CREDS, days=2, client=Throttled())
|
||||||
assert out["status"] == "rate_limited"
|
assert out["status"] == "rate_limited"
|
||||||
assert garmin_svc.get_sync_history(user["id"])[0]["status"] == "rate_limited"
|
assert garmin_svc.get_sync_history(user["id"])[0]["status"] == "rate_limited"
|
||||||
|
assert garmin_svc.rate_limited_until(user["id"]) is not None
|
||||||
|
|
||||||
def test_history_is_per_user(self, db, user, make_user):
|
def test_history_is_per_user(self, db, user, make_user):
|
||||||
garmin_svc.sync_data(user["id"], CREDS, days=1, client=StubClient())
|
garmin_svc.sync_data(user["id"], CREDS, days=1, client=StubClient())
|
||||||
|
|||||||
Reference in New Issue
Block a user