diff --git a/src/lib/integrations/__tests__/status.test.ts b/src/lib/integrations/__tests__/status.test.ts index f624e7ccb..f9a8dfe99 100644 --- a/src/lib/integrations/__tests__/status.test.ts +++ b/src/lib/integrations/__tests__/status.test.ts @@ -269,6 +269,68 @@ describe("recordSyncFailure — reauth_required", () => { }); }); +describe("recordSyncFailure — persistent (v1.4.42 W6)", () => { + it("marks state=error_transient (next sync still runs) and audits with kind=persistent", async () => { + // Contract-mismatch failures (Withings 293 invalid params) MUST + // surface in the audit log with kind=persistent so operations can + // grep for upstream contract bugs, but they MUST NOT park the + // integration — the next sync should still run because the + // mismatch can be one-sided (e.g. Withings introducing a new + // required field that the client release fixes). + vi.mocked(prisma.integrationStatus.upsert).mockResolvedValueOnce({ + consecutiveFailures: 1, + } as never); + await recordSyncFailure({ + userId: "u1", + integration: "withings", + kind: "persistent", + message: "Withings measure error: 293 - invalid params", + errorCode: "293", + }); + const upsertArgs = vi.mocked(prisma.integrationStatus.upsert).mock + .calls[0][0]; + expect(upsertArgs.update).toMatchObject({ state: "error_transient" }); + expect(auditLog).toHaveBeenCalledWith( + "integrations.sync.failed", + expect.objectContaining({ + details: expect.objectContaining({ + kind: "persistent", + errorCode: "293", + }), + }), + ); + }); + + it("at threshold, the admin alert message includes the 'persistent error' label", async () => { + vi.mocked(prisma.integrationStatus.upsert).mockResolvedValueOnce({ + consecutiveFailures: 3, + alertedAt: null, + } as never); + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + email: "user@example.com", + } as never); + vi.mocked(prisma.user.findMany).mockResolvedValueOnce([ + { id: "admin-1" }, + ] as never); + vi.mocked(prisma.integrationStatus.update).mockResolvedValueOnce( + {} as never, + ); + + await recordSyncFailure({ + userId: "u1", + integration: "withings", + kind: "persistent", + message: "Withings activity error: 293", + errorCode: "293", + }); + + expect(dispatchNotification).toHaveBeenCalledOnce(); + const payload = vi.mocked(dispatchNotification).mock.calls[0][0]; + expect(payload.message).toContain("persistent error"); + expect(payload.message).toContain("Action: investigate the upstream contract"); + }); +}); + describe("recordSyncFailure — admin Telegram skipped silently when no admins", () => { it("logs a wide-event warning but does NOT throw when no admin users exist", async () => { vi.mocked(prisma.integrationStatus.upsert).mockResolvedValueOnce({ diff --git a/src/lib/integrations/status.ts b/src/lib/integrations/status.ts index 537c770ea..bcec36c66 100644 --- a/src/lib/integrations/status.ts +++ b/src/lib/integrations/status.ts @@ -162,7 +162,26 @@ export async function recordSyncSuccess( }); } -export type FailureKind = "transient" | "reauth_required"; +/** + * Failure kinds carried into `recordSyncFailure`. + * + * - `transient` : retry on the next sync; user not blocked. + * - `reauth_required` : permanent revoke / invalid_grant; park at + * `error_reauth` until the user reconnects. + * - `persistent` : contract mismatch (invalid params, missing + * field, unknown action). Surfaces in the + * integration-status card AND audit log so an + * operator can investigate, but does NOT skip + * future sync attempts — those may succeed once + * the upstream side resolves. + * + * v1.4.42 W6 extended this union from `transient | reauth_required` to + * the three-state taxonomy above; the state-mapping function turns + * `persistent` into `error_transient` for now (a Withings 293 still + * lets the next sync run), but the audit detail carries the explicit + * kind so operations can filter. + */ +export type FailureKind = "transient" | "reauth_required" | "persistent"; export interface RecordSyncFailureInput { userId: string; @@ -193,6 +212,12 @@ export async function recordSyncFailure( ): Promise { const { userId, integration, kind, message, errorCode } = input; const now = new Date(); + // State mapping: + // reauth_required → error_reauth (sync entry-point short-circuits) + // transient → error_transient (next sync still runs) + // persistent → error_transient (next sync still runs, but the + // audit detail carries `kind: "persistent"` so + // operations can grep for contract-bug bursts) const newState: IntegrationState = kind === "reauth_required" ? "error_reauth" : "error_transient"; @@ -486,7 +511,11 @@ export function formatAdminAlertPayload(input: AlertInput): { input.integration === "withings" ? "Withings" : "moodLog"; const subjectLabel = input.subjectLabel ?? input.userId; const reasonLabel = - input.kind === "reauth_required" ? "re-auth required" : "transient error"; + input.kind === "reauth_required" + ? "re-auth required" + : input.kind === "persistent" + ? "persistent error" + : "transient error"; const codeLabel = input.errorCode ? ` (${input.errorCode})` : ""; const trimmed = input.message.length > 280 @@ -497,7 +526,13 @@ export function formatAdminAlertPayload(input: AlertInput): { const message = `${integrationLabel} sync has failed ${input.consecutiveFailures} times in a row for ${subjectLabel}.\n` + `Last error: ${reasonLabel}${codeLabel} — ${trimmed}\n` + - `Action: ${input.kind === "reauth_required" ? "ask the user to reconnect the integration." : "investigate the upstream service."}`; + `Action: ${ + input.kind === "reauth_required" + ? "ask the user to reconnect the integration." + : input.kind === "persistent" + ? "investigate the upstream contract — params/scope/action likely mismatched." + : "investigate the upstream service." + }`; return { title, diff --git a/src/lib/withings/__tests__/client.test.ts b/src/lib/withings/__tests__/client.test.ts index 5e3723b2f..20d71b219 100644 --- a/src/lib/withings/__tests__/client.test.ts +++ b/src/lib/withings/__tests__/client.test.ts @@ -2,11 +2,14 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { MEASURE_TYPE_MAP, WITHINGS_OAUTH_SCOPE, + exchangeCode, fetchMeasurements, hasActivityScope, parseWithingsScope, + refreshAccessToken, subscribeWebhook, } from "../client"; +import { WithingsApiError } from "../response-classifier"; import { WITHINGS_NOTIFY_APPLIS } from "../sync"; /** @@ -376,3 +379,142 @@ describe("fetchMeasurements — pulse-wave velocity + vascular age", () => { expect(out[0]).toMatchObject({ type: "VASCULAR_AGE", value: 42 }); }); }); + +// ── v1.4.42 W6 — off-response classifier wire-through ── +// +// Each client entrypoint must throw `WithingsApiError` carrying the +// classification verdict, NOT a plain `new Error("Withings ... error: +// ")`. Downstream catch-blocks read `err.classification`; the +// regex-fallback in `classifyError` covers serialised retries but the +// typed-throw is the contract. + +describe("client off-response handling — refreshAccessToken", () => { + const creds = { clientId: "id", clientSecret: "secret" }; + + it("throws WithingsApiError(classification: 'reauth_required') on Withings 101", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 101, error: "Invalid token" }), + })), + ); + + await expect(refreshAccessToken("rt", creds)).rejects.toMatchObject({ + name: "WithingsApiError", + classification: "reauth_required", + withingsStatus: 101, + }); + }); + + it("throws WithingsApiError(classification: 'transient') on HTTP 503", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 503, + json: async () => ({ status: 0 }), + })), + ); + + const err = await refreshAccessToken("rt", creds).catch((e) => e); + expect(err).toBeInstanceOf(WithingsApiError); + expect((err as WithingsApiError).classification).toBe("transient"); + expect((err as WithingsApiError).reason).toBe("http_503"); + }); +}); + +describe("client off-response handling — exchangeCode", () => { + const creds = { clientId: "id", clientSecret: "secret" }; + + it("classifies Withings 100 (Authentication failed) as reauth_required", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 100, error: "Authentication failed" }), + })), + ); + const err = await exchangeCode("auth-code", creds).catch((e) => e); + expect(err).toBeInstanceOf(WithingsApiError); + expect((err as WithingsApiError).classification).toBe("reauth_required"); + }); +}); + +describe("client off-response handling — fetchMeasurements", () => { + it("throws WithingsApiError(classification: 'transient') on Withings 601 (rate-limit)", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 601 }), + })), + ); + const err = await fetchMeasurements("token").catch((e) => e); + expect(err).toBeInstanceOf(WithingsApiError); + expect((err as WithingsApiError).classification).toBe("transient"); + expect((err as WithingsApiError).withingsStatus).toBe(601); + }); + + it("throws WithingsApiError(classification: 'persistent') on Withings 293 (invalid params)", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 293 }), + })), + ); + const err = await fetchMeasurements("token").catch((e) => e); + expect(err).toBeInstanceOf(WithingsApiError); + expect((err as WithingsApiError).classification).toBe("persistent"); + }); + + it("returns the empty list on a healthy but empty body (status 0, no measuregrps)", async () => { + // Off-response edge case: a healthy connection that has no data + // in the requested window is NOT a failure — it's success with + // zero rows. + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 0, body: { measuregrps: [] } }), + })), + ); + const out = await fetchMeasurements("token"); + expect(out).toEqual([]); + }); +}); + +describe("client off-response handling — subscribeWebhook 294 idempotency", () => { + it("does NOT throw when Withings replies status 294 (already-subscribed)", async () => { + // 294 at the subscribe call-site is documented idempotent success; + // every other endpoint sees it as `persistent`, but here we + // downgrade to success so reconnect flows don't fail. + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 294 }), + })), + ); + await expect( + subscribeWebhook("token", "https://example.com/webhook", 1), + ).resolves.toBeUndefined(); + }); + + it("throws WithingsApiError(classification: 'transient') when Withings replies status 2554 (notify-subscribe busy)", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + status: 200, + json: async () => ({ status: 2554 }), + })), + ); + const err = await subscribeWebhook( + "token", + "https://example.com/webhook", + 1, + ).catch((e) => e); + expect(err).toBeInstanceOf(WithingsApiError); + expect((err as WithingsApiError).classification).toBe("transient"); + }); +}); diff --git a/src/lib/withings/__tests__/response-classifier.test.ts b/src/lib/withings/__tests__/response-classifier.test.ts new file mode 100644 index 000000000..93ff0226d --- /dev/null +++ b/src/lib/withings/__tests__/response-classifier.test.ts @@ -0,0 +1,212 @@ +/** + * v1.4.42 W6 — Withings off-response classifier. + * + * The classifier turns a `(httpStatus, body)` pair into a verdict of + * `success / transient / reauth_required / persistent`. Every other + * Withings code path branches on the verdict; this suite pins the + * three-way taxonomy plus the "off-spec response" edge cases the + * legacy client treated as silent no-data. + */ +import { describe, expect, it } from "vitest"; +import { + WithingsApiError, + classifyError, + classifyWithingsResponse, +} from "../response-classifier"; + +describe("classifyWithingsResponse — success branch", () => { + it("treats HTTP 200 + status 0 as success", () => { + const v = classifyWithingsResponse(200, { status: 0 }); + expect(v.classification).toBe("success"); + expect(v.withingsStatus).toBe(0); + expect(v.reason).toBe("ok"); + }); +}); + +describe("classifyWithingsResponse — transient branch", () => { + it("HTTP 503 → transient (upstream outage)", () => { + const v = classifyWithingsResponse(503, null); + expect(v.classification).toBe("transient"); + expect(v.reason).toBe("http_503"); + }); + + it("HTTP 502 → transient", () => { + expect(classifyWithingsResponse(502, null).classification).toBe( + "transient", + ); + }); + + it("HTTP 429 → transient (rate-limit)", () => { + const v = classifyWithingsResponse(429, null); + expect(v.classification).toBe("transient"); + expect(v.reason).toBe("http_429"); + }); + + it("Withings status 503 (service unavailable) → transient", () => { + const v = classifyWithingsResponse(200, { status: 503 }); + expect(v.classification).toBe("transient"); + expect(v.withingsStatus).toBe(503); + }); + + it("Withings status 601 (rate-limited) → transient", () => { + expect( + classifyWithingsResponse(200, { status: 601 }).classification, + ).toBe("transient"); + }); + + it("Withings status 2554 (notify-subscribe transient) → transient", () => { + expect( + classifyWithingsResponse(200, { status: 2554 }).classification, + ).toBe("transient"); + }); + + it("off-spec body without a `status` field → transient (one-off CDN page)", () => { + const v = classifyWithingsResponse(200, {}); + expect(v.classification).toBe("transient"); + expect(v.reason).toBe("no_status_field"); + }); + + it("unknown Withings status defaults to transient (admin-alert ladder catches recurrence)", () => { + const v = classifyWithingsResponse(200, { status: 9999 }); + expect(v.classification).toBe("transient"); + expect(v.reason).toBe("withings_9999_unknown"); + }); +}); + +describe("classifyWithingsResponse — reauth_required branch", () => { + it.each([100, 101, 102])( + "Withings status %i → reauth_required (permanent revoke)", + (status) => { + const v = classifyWithingsResponse(200, { status }); + expect(v.classification).toBe("reauth_required"); + expect(v.withingsStatus).toBe(status); + }, + ); + + it("Withings 200 (invalid_grant boundary) → reauth_required", () => { + expect( + classifyWithingsResponse(200, { status: 200 }).classification, + ).toBe("reauth_required"); + }); + + it("Withings 250 (mid-range invalid_grant) → reauth_required", () => { + expect( + classifyWithingsResponse(200, { status: 250 }).classification, + ).toBe("reauth_required"); + }); + + it("Withings 299 (invalid_grant upper boundary) → reauth_required", () => { + expect( + classifyWithingsResponse(200, { status: 299 }).classification, + ).toBe("reauth_required"); + }); +}); + +describe("classifyWithingsResponse — persistent branch", () => { + it("Withings status 293 (invalid params) → persistent", () => { + const v = classifyWithingsResponse(200, { status: 293 }); + expect(v.classification).toBe("persistent"); + expect(v.withingsStatus).toBe(293); + }); + + it("Withings status 294 (already-subscribed) → persistent (callers downgrade for subscribe)", () => { + // 294 is idempotent-success at the subscribeWebhook call-site; + // the classifier still surfaces it as persistent so any OTHER + // endpoint that sees 294 is loud about the contract bug. + expect( + classifyWithingsResponse(200, { status: 294 }).classification, + ).toBe("persistent"); + }); + + it("HTTP 400 (unexpected from Withings) → persistent", () => { + const v = classifyWithingsResponse(400, null); + expect(v.classification).toBe("persistent"); + expect(v.reason).toBe("http_400"); + }); + + it("HTTP 404 → persistent", () => { + expect(classifyWithingsResponse(404, null).classification).toBe( + "persistent", + ); + }); +}); + +describe("WithingsApiError", () => { + it("carries the classification + status on the typed error", () => { + const err = new WithingsApiError({ + verb: "refresh", + classification: "reauth_required", + withingsStatus: 100, + reason: "withings_100", + upstreamError: "Authentication failed", + }); + expect(err.classification).toBe("reauth_required"); + expect(err.withingsStatus).toBe(100); + expect(err.reason).toBe("withings_100"); + expect(err.verb).toBe("refresh"); + expect(err.name).toBe("WithingsApiError"); + }); + + it("preserves the legacy `Withings error: - ` message shape", () => { + const err = new WithingsApiError({ + verb: "measure", + classification: "transient", + withingsStatus: 503, + reason: "withings_503", + upstreamError: "service unavailable", + }); + // Legacy regex consumers (extractWithingsStatus, etc.) expect + // exactly this shape. + expect(err.message).toBe( + "Withings measure error: 503 - service unavailable", + ); + expect(/Withings\s+\w+\s+error:\s*(\d+)/.exec(err.message)?.[1]).toBe( + "503", + ); + }); + + it("omits the dash-segment when upstreamError is absent", () => { + const err = new WithingsApiError({ + verb: "subscribe", + classification: "persistent", + withingsStatus: 293, + reason: "withings_293", + }); + expect(err.message).toBe("Withings subscribe error: 293"); + }); +}); + +describe("classifyError", () => { + it("reads the verdict directly off a WithingsApiError instance", () => { + const err = new WithingsApiError({ + verb: "refresh", + classification: "reauth_required", + withingsStatus: 101, + reason: "withings_101", + }); + expect(classifyError(err)).toBe("reauth_required"); + }); + + it("falls back to the message regex when the prototype was lost (pg-boss round-trip)", () => { + // pg-boss serializes job errors to JSON and re-throws as plain Error + // on retry — the WithingsApiError prototype does NOT survive. + const rehydrated = new Error("Withings refresh error: 102 - User does not exist"); + expect(classifyError(rehydrated)).toBe("reauth_required"); + }); + + it("classifies a recovered Withings 503 transient via the regex fallback", () => { + const rehydrated = new Error("Withings measure error: 503"); + expect(classifyError(rehydrated)).toBe("transient"); + }); + + it("classifies a recovered Withings 293 persistent via the regex fallback", () => { + const rehydrated = new Error("Withings activity error: 293"); + expect(classifyError(rehydrated)).toBe("persistent"); + }); + + it("defaults to transient for unrelated errors (network drop, timeout)", () => { + expect(classifyError(new Error("ECONNRESET"))).toBe("transient"); + expect(classifyError("string error")).toBe("transient"); + expect(classifyError(null)).toBe("transient"); + }); +}); diff --git a/src/lib/withings/client.ts b/src/lib/withings/client.ts index 82cfde9d0..daa685442 100644 --- a/src/lib/withings/client.ts +++ b/src/lib/withings/client.ts @@ -3,6 +3,10 @@ * Docs: https://developer.withings.com/api-reference */ import { getEvent } from "@/lib/logging/context"; +import { + WithingsApiError, + classifyWithingsResponse, +} from "./response-classifier"; const WITHINGS_OAUTH_URL = "https://account.withings.com/oauth2_user/authorize2"; @@ -112,15 +116,22 @@ export async function exchangeCode( }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: "exchangeCode", duration_ms: Math.round(performance.now() - start), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - throw new Error(`Withings token error: ${json.status} - ${json.error}`); + if (verdict.classification !== "success") { + throw new WithingsApiError({ + verb: "token", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } return json.body as WithingsTokenResponse; } @@ -148,15 +159,22 @@ export async function refreshAccessToken( }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: "refreshAccessToken", duration_ms: Math.round(performance.now() - start), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - throw new Error(`Withings refresh error: ${json.status} - ${json.error}`); + if (verdict.classification !== "success") { + throw new WithingsApiError({ + verb: "refresh", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } return json.body as WithingsTokenResponse; } @@ -261,15 +279,22 @@ export async function fetchMeasurements( }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: `fetchMeasurements(page=${pageCount})`, duration_ms: Math.round(performance.now() - pageStart), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - throw new Error(`Withings measure error: ${json.status}`); + if (verdict.classification !== "success") { + throw new WithingsApiError({ + verb: "measure", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } const body = json.body ?? {}; @@ -331,19 +356,29 @@ export async function subscribeWebhook( }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); + // 294 = already-subscribed is idempotent success at the subscribe + // call-site (Withings preserves the existing subscription). The + // classifier surfaces it as `persistent` because any OTHER endpoint + // receiving 294 is a contract bug; here we explicitly downgrade it. + const isAlreadySubscribed = json?.status === 294; + const treatAsSuccess = + verdict.classification === "success" || isAlreadySubscribed; getEvent()?.addExternalCall({ service: "withings", method: "subscribeWebhook", duration_ms: Math.round(performance.now() - start), status: res.status, - error: - json.status !== 0 && json.status !== 294 - ? `status=${json.status}` - : undefined, + error: treatAsSuccess ? undefined : verdict.reason, }); - // Status 0 = success, 294 = already subscribed (both OK) - if (json.status !== 0 && json.status !== 294) { - throw new Error(`Withings subscribe error: ${json.status}`); + if (!treatAsSuccess) { + throw new WithingsApiError({ + verb: "subscribe", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } } @@ -372,14 +407,21 @@ export async function unsubscribeWebhook( }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: "unsubscribeWebhook", duration_ms: Math.round(performance.now() - start), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - getEvent()?.addWarning(`Withings unsubscribe warning: ${json.status}`); + // Unsubscribe is best-effort — a stale subscription will eventually + // be garbage-collected by Withings, so we never throw here. We do + // bubble the verdict into a warning so the audit-log path still + // sees the off-response. + if (verdict.classification !== "success") { + getEvent()?.addWarning( + `Withings unsubscribe warning: ${verdict.reason}`, + ); } } diff --git a/src/lib/withings/response-classifier.ts b/src/lib/withings/response-classifier.ts new file mode 100644 index 000000000..210885515 --- /dev/null +++ b/src/lib/withings/response-classifier.ts @@ -0,0 +1,281 @@ +/** + * Off-response classifier for Withings API replies. + * + * The Withings API has three failure shapes the legacy client treated + * uniformly as "throw with a message": + * + * 1. HTTP 5xx — upstream outage, retry on the next sync. + * 2. HTTP 200 + `status: ` — Withings returns its semantic + * error code in the JSON body, with HTTP 200. The status code + * taxonomy is documented at + * https://developer.withings.com/api-reference/#section/Response-status . + * 3. HTTP 200 + `status: 0` + empty `measuregrps` — no data, but the + * connection is healthy. Not a failure at all. + * + * Pre-v1.4.42 the sync path inferred reauth-vs-transient from the error + * MESSAGE via a regex (`isWithingsRefreshReauthFailure` in sync.ts). + * That worked because the only failure shape the client threw was + * `Withings error: `, but it tightly coupled the + * classifier to the literal message format and meant rate-limit + * responses (601) and quota responses (293) were silently bucketed as + * transient errors that never escalated even when they kept recurring. + * + * This module introduces an explicit classifier: + * + * - `classifyWithingsResponse(httpStatus, body)` — pure function, + * returns a `WithingsClassification`. + * - `WithingsApiError` — typed Error subclass carrying the + * classification so downstream catch-blocks don't need to re-parse + * a string. + * + * Classification buckets: + * + * - `success` → HTTP 200 + body.status === 0 + * - `transient` → retryable: HTTP 5xx, 429, Withings status 503 + * (upstream unavailable), 601 (rate-limited), + * 2554 (too many subscriptions in flight), + * 3xx network/CDN glitches + * - `reauth_required` → permanent revoke: Withings 100, 101, 102, + * 200–299 (invalid grant family). The user has + * to redo OAuth. + * - `persistent` → operator-attention error: invalid params + * (293), missing scope (293 with body.scope_hint), + * unknown action (294 outside subscribeWebhook). + * These should NOT be retried silently — they + * indicate a contract mismatch. + * + * The classifier is deliberately conservative: anything it doesn't + * recognise defaults to `transient` so we don't hard-fail an integration + * over a Withings status we've never seen. The audit log + admin-alert + * ladder in `recordSyncFailure` already catches the "this keeps + * happening" case for unknown transients. + */ + +/** Outcome buckets for a Withings response. */ +export type WithingsClassification = + | "success" + | "transient" + | "reauth_required" + | "persistent"; + +export interface ClassifiedResponse { + classification: WithingsClassification; + /** The Withings body status code (or undefined if HTTP-only failure). */ + withingsStatus: number | undefined; + /** Short human-readable label for logs / audit details. */ + reason: string; +} + +/** + * The minimum body shape we read. Withings adds many additional fields + * (`body`, `error`, `scope_hint`, `more`, `measuregrps`, …); callers + * keep their own typed views — we only need `status` and (optionally) + * `error` for the reason string. + */ +export interface WithingsBodyLike { + status?: number; + error?: string; +} + +/** + * Withings status codes that map to a permanent-revoke (user MUST + * redo OAuth). See https://developer.withings.com/api-reference for + * the full table — these are the values both Withings' own SDKs and + * the existing HealthLog code treated as terminal. + */ +const REAUTH_CODES = new Set([100, 101, 102]); + +/** + * Withings status codes that map to a transient / retryable error. + * Conservative on purpose — 503 is the documented "service unavailable" + * response, 601 is rate-limit, 2554/2555/2556 are notify-subscribe + * transients. + */ +const TRANSIENT_CODES = new Set([503, 601, 2554, 2555, 2556]); + +/** + * Withings status codes that indicate a persistent contract mismatch — + * the request itself is malformed or the integration is missing a + * required field. Retrying without operator intervention will not + * succeed; the audit log surfaces these. + * + * - 293 = "invalid params" / "missing required field" + * - 294 = "already subscribed" (subscribeWebhook treats it as success; + * any OTHER endpoint receiving 294 is a contract bug) + * + * Note: 294 is intentionally listed here. `subscribeWebhook` overrides + * the classification for its own call site (already-subscribed is + * idempotent there); every other client call that sees 294 surfaces it + * as a persistent failure so the bug doesn't hide. + */ +const PERSISTENT_CODES = new Set([293, 294]); + +/** + * Classify a single Withings API response. + * + * @param httpStatus The HTTP status code from fetch (200 for the + * happy path, 5xx for upstream outages). + * @param body The parsed JSON body. Only `status` (and + * optionally `error`) are read. + */ +export function classifyWithingsResponse( + httpStatus: number, + body: WithingsBodyLike | null | undefined, +): ClassifiedResponse { + // HTTP layer first — a 503 / 502 / 504 from Withings means we never + // got to the JSON envelope. + if (httpStatus >= 500 && httpStatus < 600) { + return { + classification: "transient", + withingsStatus: undefined, + reason: `http_${httpStatus}`, + }; + } + if (httpStatus === 429) { + return { + classification: "transient", + withingsStatus: undefined, + reason: "http_429", + }; + } + // 4xx outside 429 is unexpected from Withings — they always reply 200 + // even on auth failures, embedding the real status in the body. We + // bucket as persistent so a regression to a 4xx-throwing upstream + // surfaces in audit logs rather than retrying forever. + if (httpStatus >= 400 && httpStatus < 500) { + return { + classification: "persistent", + withingsStatus: undefined, + reason: `http_${httpStatus}`, + }; + } + + // Body status — Withings' own taxonomy. + const status = body?.status; + if (status === 0) { + return { + classification: "success", + withingsStatus: 0, + reason: "ok", + }; + } + if (typeof status !== "number") { + // Off-spec body — no `status` field at all. The legacy client + // would have crashed reading `json.status`; we treat it as a + // transient so the retry catches a one-off CDN error page. + return { + classification: "transient", + withingsStatus: undefined, + reason: "no_status_field", + }; + } + + // Explicit lists take precedence over ranges so a contract-mismatch + // code (293, 294) that happens to live inside the 200..299 OAuth + // range stays classified as persistent rather than reauth. + if (PERSISTENT_CODES.has(status)) { + return { + classification: "persistent", + withingsStatus: status, + reason: `withings_${status}`, + }; + } + if (TRANSIENT_CODES.has(status)) { + return { + classification: "transient", + withingsStatus: status, + reason: `withings_${status}`, + }; + } + if (REAUTH_CODES.has(status)) { + return { + classification: "reauth_required", + withingsStatus: status, + reason: `withings_${status}`, + }; + } + // The 200..299 invalid_grant family — Withings uses this range for + // OAuth grant failures. Membership is by range rather than an + // explicit list because individual codes in there can shift between + // Withings firmware revisions. Explicit persistent/transient codes + // above this check are deliberate exemptions. + if (status >= 200 && status <= 299) { + return { + classification: "reauth_required", + withingsStatus: status, + reason: `withings_${status}`, + }; + } + + // Default: anything we don't recognise becomes transient. The + // 3-strike admin alert ladder in `recordSyncFailure` already catches + // the "keeps happening" case, and silently degrading to "no data" + // (the legacy behaviour) is strictly worse than a retry. + return { + classification: "transient", + withingsStatus: status, + reason: `withings_${status}_unknown`, + }; +} + +/** + * Typed Error subclass that carries the classification verdict so + * `try/catch` blocks downstream can branch without re-parsing the + * message. + * + * The `message` field still uses the legacy + * `"Withings error: - "` format so the existing + * regex-based `extractWithingsStatus` / `isWithingsRefreshReauthFailure` + * helpers in sync.ts continue to work for callers that haven't been + * migrated yet. Callers that import the new class can read + * `err.classification` directly. + */ +export class WithingsApiError extends Error { + readonly classification: WithingsClassification; + readonly withingsStatus: number | undefined; + readonly reason: string; + readonly verb: string; + + constructor(opts: { + verb: string; + classification: WithingsClassification; + withingsStatus: number | undefined; + reason: string; + upstreamError?: string; + }) { + const statusLabel = + typeof opts.withingsStatus === "number" ? opts.withingsStatus : "?"; + const errSegment = opts.upstreamError ? ` - ${opts.upstreamError}` : ""; + super(`Withings ${opts.verb} error: ${statusLabel}${errSegment}`); + this.name = "WithingsApiError"; + this.verb = opts.verb; + this.classification = opts.classification; + this.withingsStatus = opts.withingsStatus; + this.reason = opts.reason; + } +} + +/** + * Read the classification from any caught error. Returns + * `transient` for non-`WithingsApiError` inputs so existing call + * sites that surface a plain `Error` continue to retry rather than + * permanently disabling the integration. + * + * Falls back to message-regex parsing so callers that wrap + * `WithingsApiError` in `new Error(`... ${err.message}`)` (e.g. + * pg-boss job retries that lose the original prototype) still + * classify correctly. + */ +export function classifyError(err: unknown): WithingsClassification { + if (err instanceof WithingsApiError) return err.classification; + + // Best-effort regex fallback for unwrapped messages — same shape + // sync.ts's `extractWithingsStatus` reads. + const msg = err instanceof Error ? err.message : String(err); + const m = /Withings\s+\w+\s+error:\s*(\d+)/.exec(msg); + if (!m) return "transient"; + const status = Number.parseInt(m[1]!, 10); + if (!Number.isFinite(status)) return "transient"; + // Run the body classifier with a synthetic HTTP-200 envelope. + return classifyWithingsResponse(200, { status }).classification; +} diff --git a/src/lib/withings/sync-activity.ts b/src/lib/withings/sync-activity.ts index 3ce27fcb8..e58d2c96b 100644 --- a/src/lib/withings/sync-activity.ts +++ b/src/lib/withings/sync-activity.ts @@ -44,6 +44,10 @@ import { } from "@/lib/rollups/measurement-rollups"; import { hasActivityScope } from "./client"; +import { + WithingsApiError, + classifyWithingsResponse, +} from "./response-classifier"; import { extractWithingsStatus, isWithingsRefreshReauthFailure, @@ -157,15 +161,22 @@ export async function fetchWithingsActivity( body: params.toString(), }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: `fetchWithingsActivity(page=${pageCount})`, duration_ms: Math.round(performance.now() - pageStart), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - throw new Error(`Withings activity error: ${json.status}`); + if (verdict.classification !== "success") { + throw new WithingsApiError({ + verb: "activity", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } const body = json.body ?? {}; diff --git a/src/lib/withings/sync-sleep.ts b/src/lib/withings/sync-sleep.ts index 24a2f2d3d..f9d70fc85 100644 --- a/src/lib/withings/sync-sleep.ts +++ b/src/lib/withings/sync-sleep.ts @@ -42,6 +42,10 @@ import { } from "@/lib/rollups/measurement-rollups"; import { hasActivityScope } from "./client"; +import { + WithingsApiError, + classifyWithingsResponse, +} from "./response-classifier"; import { extractWithingsStatus, isWithingsRefreshReauthFailure, @@ -127,15 +131,22 @@ export async function fetchWithingsSleep( body: params.toString(), }); const json = await res.json(); + const verdict = classifyWithingsResponse(res.status, json); getEvent()?.addExternalCall({ service: "withings", method: "fetchWithingsSleep", duration_ms: Math.round(performance.now() - pageStart), status: res.status, - error: json.status !== 0 ? `status=${json.status}` : undefined, + error: verdict.classification === "success" ? undefined : verdict.reason, }); - if (json.status !== 0) { - throw new Error(`Withings sleep error: ${json.status}`); + if (verdict.classification !== "success") { + throw new WithingsApiError({ + verb: "sleep", + classification: verdict.classification, + withingsStatus: verdict.withingsStatus, + reason: verdict.reason, + upstreamError: typeof json?.error === "string" ? json.error : undefined, + }); } const series: WithingsSleepSegment[] = json.body?.series ?? []; return series; diff --git a/src/lib/withings/sync.ts b/src/lib/withings/sync.ts index d85076385..25e4ef4cb 100644 --- a/src/lib/withings/sync.ts +++ b/src/lib/withings/sync.ts @@ -17,7 +17,13 @@ import { isReauthRequired, recordSyncFailure, recordSyncSuccess, + type FailureKind, } from "@/lib/integrations/status"; +import { + classifyError, + WithingsApiError, + type WithingsClassification, +} from "./response-classifier"; import { collapseToTypeDayKeys, recomputeBucketsForMeasurement, @@ -109,19 +115,22 @@ export async function getValidToken(userId: string): Promise<{ }; } catch (err) { getEvent()?.addWarning(`Token refresh failed for user ${userId}: ${err}`); - // Withings refresh failures expose the upstream `status` code in - // the error message (see refreshAccessToken). We treat any of the - // documented permanent-revoke statuses (100, 101, 102, 200..299 - // for invalid_grant) as reauth-required and everything else as a - // transient retryable failure. + // v1.4.42 W6 — the typed `WithingsApiError` carries the + // classification verdict directly; the regex fallback in + // `classifyError` handles plain `Error` instances (e.g. a + // pg-boss job retry that lost the prototype during JSON + // round-trip). const message = err instanceof Error ? err.message : String(err); - const isReauth = isWithingsRefreshReauthFailure(message); + const classification = classifyError(err); await recordSyncFailure({ userId, integration: "withings", - kind: isReauth ? "reauth_required" : "transient", + kind: classificationToFailureKind(classification), message, - errorCode: extractWithingsStatus(message), + errorCode: + err instanceof WithingsApiError + ? err.withingsStatus?.toString() + : extractWithingsStatus(message), }); return null; } @@ -185,14 +194,16 @@ export async function syncUserMeasurements( measures = await fetchMeasurements(tokenInfo.accessToken, startDate); } catch (err) { const message = err instanceof Error ? err.message : String(err); + const classification = classifyError(err); await recordSyncFailure({ userId, integration: "withings", - kind: isWithingsRefreshReauthFailure(message) - ? "reauth_required" - : "transient", + kind: classificationToFailureKind(classification), message, - errorCode: extractWithingsStatus(message), + errorCode: + err instanceof WithingsApiError + ? err.withingsStatus?.toString() + : extractWithingsStatus(message), }); throw err; } @@ -287,6 +298,33 @@ export async function syncUserMeasurements( return imported; } +/** + * Map a Withings response classification onto the `FailureKind` carried + * into `recordSyncFailure`. Pure function — no IO, no state. + * + * `success` → not a failure; callers must not invoke this. + * `transient` → `transient` (retry next sync) + * `reauth_required` → `reauth_required` (park at `error_reauth`) + * `persistent` → `persistent` (audited; next sync still runs) + */ +export function classificationToFailureKind( + classification: WithingsClassification, +): FailureKind { + switch (classification) { + case "reauth_required": + return "reauth_required"; + case "persistent": + return "persistent"; + case "transient": + return "transient"; + case "success": + // Defensive: a caller asking for the FailureKind of a success + // is a contract bug. Default to `transient` so the audit log + // still surfaces the bug rather than silently ignoring it. + return "transient"; + } +} + /** * Withings status codes that indicate a permanent-revoke condition * (the refresh_token will not work again — the user has to redo OAuth). @@ -302,6 +340,13 @@ export async function syncUserMeasurements( * * The error message format from `refreshAccessToken()` is * "Withings refresh error: - ". + * + * v1.4.42 W6 — superseded for new code paths by + * `classifyError`/`classifyWithingsResponse` in `response-classifier.ts`. + * Kept for the W17b/c sync-activity + sync-sleep catch-blocks that + * haven't yet been migrated; both paths now also benefit from the + * typed-error throw upstream because `classifyError` falls back to the + * same regex. */ export function isWithingsRefreshReauthFailure(message: string): boolean { const status = extractWithingsStatus(message);