mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(whoop): degrade a per-resource 403 gracefully instead of parking the connection
A 403 on a single WHOOP collection endpoint was classified reauth_required, which parked the whole connection at error_reauth and short-circuited every future sync. A lower-tier user missing one gated data class would brick the entire integration. Treat a per-resource collection 403 as a soft skip of that data class: log, return 0, and continue so sibling resources still sync. Reserve connection-wide reauth for a 401 (token rejected) and for a 403 on the token-refresh / profile path, which run outside the per-resource catch blocks and remain unchanged. Add isCollectionForbidden as the shared gate and cover both branches in tests.
This commit is contained in:
1 parent
e07527e680
commit
c8e8499472
6 files changed
+132
No files matched your search
@@ -87,10 +87,13 @@ vi.mock("@/lib/logging/context", () => ({
|
||||
import {
|
||||
getValidToken,
|
||||
incrementalStart,
|
||||
isCollectionForbidden,
|
||||
upsertWhoopMeasurements,
|
||||
WHOOP_DEFAULT_OVERLAP_MS,
|
||||
WHOOP_RECOVERY_SLEEP_OVERLAP_MS,
|
||||
} from "../sync";
|
||||
import { WhoopApiError } from "../response-classifier";
|
||||
import { syncUserRecovery } from "../sync-recovery";
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -247,3 +250,81 @@ describe("upsertWhoopMeasurements — idempotent upsert", () => {
|
||||
expect(a).toEqual(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isCollectionForbidden — tier degradation gate", () => {
|
||||
it("is true only for a WhoopApiError carrying HTTP 403", () => {
|
||||
const forbidden = new WhoopApiError({
|
||||
verb: "fetchRecoveries",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 403,
|
||||
reason: "http_403",
|
||||
});
|
||||
expect(isCollectionForbidden(forbidden)).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for a 401 (genuine token reject → connection-wide reauth)", () => {
|
||||
const unauthorized = new WhoopApiError({
|
||||
verb: "fetchRecoveries",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 401,
|
||||
reason: "http_401",
|
||||
});
|
||||
expect(isCollectionForbidden(unauthorized)).toBe(false);
|
||||
});
|
||||
|
||||
it("is false for a non-WhoopApiError", () => {
|
||||
expect(isCollectionForbidden(new Error("network down"))).toBe(false);
|
||||
expect(isCollectionForbidden("boom")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("per-resource 403 soft-skip vs reauth", () => {
|
||||
beforeEach(() => {
|
||||
prismaMock.whoopConnection.findUnique.mockResolvedValue({
|
||||
id: "conn1",
|
||||
whoopUserId: "42",
|
||||
accessToken: "enc(live-access)",
|
||||
refreshToken: "enc(live-refresh)",
|
||||
tokenExpiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
lastSyncedAt: new Date("2026-06-01T00:00:00Z"),
|
||||
});
|
||||
prismaMock.whoopConnection.update.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it("a collection 403 soft-skips: returns 0, records NO failure (connection stays connected)", async () => {
|
||||
fetchRecoveriesMock.mockRejectedValue(
|
||||
new WhoopApiError({
|
||||
verb: "fetchRecoveries",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 403,
|
||||
reason: "http_403",
|
||||
}),
|
||||
);
|
||||
|
||||
const imported = await syncUserRecovery("user1");
|
||||
|
||||
expect(imported).toBe(0);
|
||||
// No failure recorded → recordSyncFailure never parks the row at
|
||||
// error_reauth, so the next syncUserWhoop does not short-circuit.
|
||||
expect(recordSyncFailure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("a collection 401 still records a reauth failure and rethrows", async () => {
|
||||
fetchRecoveriesMock.mockRejectedValue(
|
||||
new WhoopApiError({
|
||||
verb: "fetchRecoveries",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 401,
|
||||
reason: "http_401",
|
||||
}),
|
||||
);
|
||||
|
||||
await expect(syncUserRecovery("user1")).rejects.toThrow();
|
||||
expect(recordSyncFailure).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
integration: "whoop",
|
||||
kind: "reauth_required",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -9,12 +9,14 @@ import { fetchCycles, mapCycle } from "./client";
|
||||
import {
|
||||
getValidToken,
|
||||
incrementalStart,
|
||||
isCollectionForbidden,
|
||||
markSynced,
|
||||
recordWhoopSyncFailure,
|
||||
upsertWhoopMeasurements,
|
||||
type WhoopMeasurementUpsert,
|
||||
} from "./sync";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
|
||||
export async function syncUserCycle(
|
||||
userId: string,
|
||||
@@ -37,6 +39,14 @@ export async function syncUserCycle(
|
||||
try {
|
||||
records = await fetchCycles(tokenInfo.accessToken, { start });
|
||||
} catch (err) {
|
||||
// A per-resource 403 soft-skips this data class rather than parking the
|
||||
// whole connection — sibling resources still sync.
|
||||
if (isCollectionForbidden(err)) {
|
||||
getEvent()?.addWarning(
|
||||
`whoop cycle sync skipped for ${userId}: collection 403 (soft-skip)`,
|
||||
);
|
||||
return 0;
|
||||
}
|
||||
await recordWhoopSyncFailure(userId, err);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import { fetchRecoveries, mapRecovery } from "./client";
|
||||
import {
|
||||
getValidToken,
|
||||
incrementalStart,
|
||||
isCollectionForbidden,
|
||||
markSynced,
|
||||
recordWhoopSyncFailure,
|
||||
upsertWhoopMeasurements,
|
||||
@@ -20,6 +21,7 @@ import {
|
||||
type WhoopMeasurementUpsert,
|
||||
} from "./sync";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
|
||||
export async function syncUserRecovery(
|
||||
userId: string,
|
||||
@@ -43,6 +45,14 @@ export async function syncUserRecovery(
|
||||
try {
|
||||
records = await fetchRecoveries(tokenInfo.accessToken, { start });
|
||||
} catch (err) {
|
||||
// A per-resource 403 soft-skips this data class rather than parking the
|
||||
// whole connection — sibling resources still sync.
|
||||
if (isCollectionForbidden(err)) {
|
||||
getEvent()?.addWarning(
|
||||
`whoop recovery sync skipped for ${userId}: collection 403 (soft-skip)`,
|
||||
);
|
||||
return 0;
|
||||
}
|
||||
await recordWhoopSyncFailure(userId, err);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import { fetchSleeps, mapSleep } from "./client";
|
||||
import {
|
||||
getValidToken,
|
||||
incrementalStart,
|
||||
isCollectionForbidden,
|
||||
markSynced,
|
||||
recordWhoopSyncFailure,
|
||||
upsertWhoopMeasurements,
|
||||
@@ -18,6 +19,7 @@ import {
|
||||
type WhoopMeasurementUpsert,
|
||||
} from "./sync";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getEvent } from "@/lib/logging/context";
|
||||
|
||||
export async function syncUserSleep(
|
||||
userId: string,
|
||||
@@ -41,6 +43,14 @@ export async function syncUserSleep(
|
||||
try {
|
||||
records = await fetchSleeps(tokenInfo.accessToken, { start });
|
||||
} catch (err) {
|
||||
// A per-resource 403 soft-skips this data class rather than parking the
|
||||
// whole connection — sibling resources still sync.
|
||||
if (isCollectionForbidden(err)) {
|
||||
getEvent()?.addWarning(
|
||||
`whoop sleep sync skipped for ${userId}: collection 403 (soft-skip)`,
|
||||
);
|
||||
return 0;
|
||||
}
|
||||
await recordWhoopSyncFailure(userId, err);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ import { fetchWorkouts, KJ_TO_KCAL } from "./client";
|
||||
import {
|
||||
getValidToken,
|
||||
incrementalStart,
|
||||
isCollectionForbidden,
|
||||
markSynced,
|
||||
recordWhoopSyncFailure,
|
||||
} from "./sync";
|
||||
@@ -52,6 +53,14 @@ export async function syncUserWorkout(
|
||||
try {
|
||||
records = await fetchWorkouts(tokenInfo.accessToken, { start });
|
||||
} catch (err) {
|
||||
// A per-resource 403 soft-skips this data class rather than parking the
|
||||
// whole connection — sibling resources still sync.
|
||||
if (isCollectionForbidden(err)) {
|
||||
getEvent()?.addWarning(
|
||||
`whoop workout sync skipped for ${userId}: collection 403 (soft-skip)`,
|
||||
);
|
||||
return 0;
|
||||
}
|
||||
await recordWhoopSyncFailure(userId, err);
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -43,6 +43,18 @@ import {
|
||||
type WhoopClassification,
|
||||
} from "./response-classifier";
|
||||
|
||||
/**
|
||||
* True when a caught error is a per-resource collection 403 (forbidden). A 403
|
||||
* on a single data class is a tier/scope gate on THAT class — the right
|
||||
* response is to soft-skip the class and keep the connection connected, NOT to
|
||||
* park the whole integration at `error_reauth`. Reserve connection-wide reauth
|
||||
* for a 401 (token rejected) and for a 403 on the token-refresh / profile path
|
||||
* (a genuine grant revoke), which run outside the per-resource catch blocks.
|
||||
*/
|
||||
export function isCollectionForbidden(err: unknown): boolean {
|
||||
return err instanceof WhoopApiError && err.httpStatus === 403;
|
||||
}
|
||||
|
||||
/** Refresh the access token this many ms before `tokenExpiresAt`. */
|
||||
const TOKEN_REFRESH_BUFFER_MS = 5 * 60 * 1000;
|
||||
|
||||
|
||||
Reference in new issue
Block a user