From 042c4d52be6114c96b9f0a5576e8e286179ce295 Mon Sep 17 00:00:00 2001 From: ericwyuan Date: Fri, 28 Aug 2026 14:20:43 +0800 Subject: [PATCH] =?UTF-8?q?fix(sync):=20=E8=A2=AB=20Garmin=20=E9=99=90?= =?UTF-8?q?=E6=B5=81=E6=97=B6=E8=AF=B4=E4=BA=BA=E8=AF=9D=EF=BC=8C=E5=B9=B6?= =?UTF-8?q?=E5=81=9C=E6=AD=A2=E6=8A=8A=E9=99=90=E6=B5=81=E8=B6=8A=E6=92=9E?= =?UTF-8?q?=E8=B6=8A=E6=B7=B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 服务器上的数据停在 8-25，而同步状态是 error、卡在「连接 Garmin」， 记录的原因是 `JSONDecodeError: Expecting value: line 1 column 1`。 真相：Garmin 返回的是 HTTP 429，响应体是纯文本 "Rate limited"（12 字节）， garth 把它交给 json.loads，于是限流被伪装成了解析错误。令牌本身好好的， oauth1 有效期到 2027-08-23，账号也没问题。 - 新增 RateLimited 异常并识别 429（按状态码或响应体），同步状态里显示 「Garmin 暂时限制了请求频率…令牌本身没有失效」，不再是一句解析报错。 - 命中限流后整个进程对该账号退避 30 分钟。重试正是把限流撞得更深的原因， 原来失败后每小时还接着试。 - 只在 oauth2 令牌确实过期时才 refresh_oauth2()。原先每次 _connect 都无条件 刷新一次，白白消耗配额——正是这个把账号一步步推到了 429。 排查过程中我一度误判：先以为是数据库迁移把令牌 base64 包了一层， 改了解码逻辑反而把能用的令牌弄坏（garth.loads 本来就要 base64 形式）， 已还原。真正的定位靠拦截 requests.Session 打印出原始响应。 Co-Authored-By: Claude Haiku 4.5 --- backend/services/garmin.py | 57 ++++++++++++++++++++++- backend/tests/test_garmin_sync.py | 75 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 2 deletions(-) diff --git a/backend/services/garmin.py b/backend/services/garmin.py index e8d4339..8e099c4 100644 --- a/backend/services/garmin.py +++ b/backend/services/garmin.py @@ -85,6 +85,39 @@ def reset_stale_syncs(): ) +class RateLimited(RuntimeError): + """Garmin answered 429. + + It reaches us disguised: the body is the plain text "Rate limited", and + garth feeds that to json.loads, so the exception surfaced as + `JSONDecodeError: Expecting value: line 1 column 1` — which reads like a + parsing bug rather than "stop asking". Naming it means the sync status + says what is actually wrong. + """ + + +# Retrying while rate limited is what deepens the limit, so once Garmin says +# 429 the whole process stands down until this passes. +RATE_LIMIT_BACKOFF = datetime.timedelta(minutes=30) +_rate_limited_until = {} + + +def rate_limited_until(user_id): + return _rate_limited_until.get(user_id) + + +def _note_rate_limit(user_id): + _rate_limited_until[user_id] = datetime.datetime.utcnow() + RATE_LIMIT_BACKOFF + + +def _is_rate_limited(e): + """429 from Garmin, however it happens to be dressed.""" + response = getattr(e, "response", None) + if response is not None and getattr(response, "status_code", None) == 429: + return True + return "rate limit" in str(e).lower() + + class MFARequired(RuntimeError): """Raised when a password login needs a code this process cannot obtain.""" @@ -198,8 +231,28 @@ def _connect(creds, user_id=None): if token: client.garth.loads(token) _use_api_user_agent(client) - # Proves the token still works, and refreshes it if near expiry. - client.garth.refresh_oauth2() + + blocked = _rate_limited_until.get(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 + # quota for nothing, and that is what walked the account into a 429. + oauth2 = getattr(client.garth, "oauth2_token", None) + if oauth2 is None or getattr(oauth2, "expired", True): + try: + client.garth.refresh_oauth2() + except Exception as e: # noqa: BLE001 - re-raised, just named better + if _is_rate_limited(e): + _note_rate_limit(user_id) + raise RateLimited( + "Garmin 暂时限制了请求频率。这通常是短时间内连接过于频繁，" + "等待约半小时后会自动恢复，令牌本身没有失效。" + ) from e + raise # garminconnect builds most of its URLs from display_name, so leaving # it unset sends every request to ".../None". client.display_name = client.garth.profile["displayName"] diff --git a/backend/tests/test_garmin_sync.py b/backend/tests/test_garmin_sync.py index 5181526..8f9c0c6 100644 --- a/backend/tests/test_garmin_sync.py +++ b/backend/tests/test_garmin_sync.py @@ -602,3 +602,78 @@ class TestBackgroundSync: ) client.post("/api/garmin/sync", headers=auth, json={"days": 99999}) assert seen["days"] == 730 + + +class TestRateLimiting: + """Garmin answers 429 with the plain text "Rate limited". + + garth feeds that body to json.loads, so it surfaced as + `JSONDecodeError: Expecting value: line 1 column 1` — indistinguishable + from a parsing bug. The sync status said nothing useful while the real + problem was that we were asking too often, and every retry deepened it. + """ + + def setup_method(self): + garmin_svc._rate_limited_until.clear() + + def teardown_method(self): + garmin_svc._rate_limited_until.clear() + + def test_plain_text_body_is_recognised(self): + assert garmin_svc._is_rate_limited(ValueError("Rate limited")) + + def test_status_code_is_recognised(self): + class Resp: + status_code = 429 + + err = RuntimeError("nope") + err.response = Resp() + assert garmin_svc._is_rate_limited(err) + + def test_an_ordinary_error_is_not_mistaken_for_one(self): + assert not garmin_svc._is_rate_limited(ValueError("boom")) + + def test_backoff_is_recorded_and_reported(self, db, user): + garmin_svc._note_rate_limit(user["id"]) + until = garmin_svc.rate_limited_until(user["id"]) + assert until is not None and until > datetime.datetime.utcnow() + + def test_a_blocked_account_does_not_call_garmin_again(self, db, user, monkeypatch): + """The retry is what deepens the limit, so it must not happen.""" + garmin_svc.save_token(user["id"], "token-blob") + garmin_svc._note_rate_limit(user["id"]) + + class Stub: + def __init__(self, *a, **k): + self.garth = type("G", (), { + "loads": lambda *a: None, + "refresh_oauth2": lambda *a: (_ for _ in ()).throw( + AssertionError("must not reach Garmin while backing off")), + "oauth2_token": None, + "sess": type("S", (), {"headers": {}})(), + })() + + monkeypatch.setattr(garmin_svc, "_import_garmin", lambda: Stub) + with pytest.raises(garmin_svc.RateLimited): + garmin_svc._connect({}, user_id=user["id"]) + + def test_a_valid_token_is_not_refreshed(self, db, user, monkeypatch): + """Refreshing on every connect spends quota for nothing — and that is + what walked the account into a 429 in the first place.""" + garmin_svc.save_token(user["id"], "token-blob") + calls = [] + + class Stub: + def __init__(self, *a, **k): + unexpired = type("T", (), {"expired": False})() + self.garth = type("G", (), { + "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"