mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(google-health): one broken collection no longer kills its siblings
The live 400 on fetchSteps suppressed fetchDistance / fetchFloors / fetchActiveEnergy entirely: handleCollectionFetchError rethrew anything that was not a per-class 403, and the rethrow escaped the rollup loop in sync-activity, emptying the whole cumulative group after the first bad collection. - handleCollectionFetchError records the classified failure, warns with the per-collection detail, and returns instead of rethrowing; every call site already sits in a per-collection catch, so the sibling collections keep fetching. - A per-cycle hard-fail ledger (AsyncLocalStorage, mirroring the 403 soft-skip tracker) still fails the cycle: the run counts as a partial error, the watermark is not stamped, and the next tick refetches the window - a broken collection cannot silently advance past its gap. - The orchestrator's per-resource warning names the resource explicitly; fn.name minifies to a single letter in the production bundle (the live log read "google-health c failed"). - Tests pin the no-rethrow contract per classification and the activity loop reaching all four rollup types plus VO2 max after a steps failure.
This commit is contained in:
1 parent
1c3c733fab
commit
de86ee26f2
3 files changed
+230
-19
No files matched your search
@@ -0,0 +1,84 @@
|
||||
/**
|
||||
* Pins the cumulative-group independence contract: each of the four rollup
|
||||
* types (steps / distance / active-energy / floors) and the VO2-max summary
|
||||
* fetches independently inside `syncUserActivity`. A hard failure on ONE type
|
||||
* routes through `handleCollectionFetchError` and the loop continues — the
|
||||
* siblings still fetch (live regression: steps' 400 killed the whole group).
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const { fetchDailyRollUpMock, fetchDataPointsMock, handleErrorMock } =
|
||||
vi.hoisted(() => ({
|
||||
fetchDailyRollUpMock: vi.fn(),
|
||||
fetchDataPointsMock: vi.fn(async () => []),
|
||||
handleErrorMock: vi.fn(async () => 0),
|
||||
}));
|
||||
|
||||
vi.mock("../client", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("../client")>();
|
||||
return {
|
||||
...actual,
|
||||
fetchDailyRollUp: fetchDailyRollUpMock,
|
||||
fetchDataPoints: fetchDataPointsMock,
|
||||
};
|
||||
});
|
||||
vi.mock("../sync", () => ({
|
||||
getValidToken: vi.fn(async () => ({
|
||||
accessToken: "token",
|
||||
connection: { id: "c1", googleUserId: "g1" },
|
||||
})),
|
||||
handleCollectionFetchError: handleErrorMock,
|
||||
upsertGoogleHealthMeasurements: vi.fn(async () => ({
|
||||
imported: 0,
|
||||
touched: [],
|
||||
})),
|
||||
}));
|
||||
vi.mock("@/lib/tz/resolver", () => ({
|
||||
resolveUserTimezone: vi.fn(async () => "UTC"),
|
||||
}));
|
||||
|
||||
import { syncUserActivity } from "../sync-activity";
|
||||
import { GoogleHealthApiError } from "../response-classifier";
|
||||
|
||||
beforeEach(() => {
|
||||
fetchDailyRollUpMock.mockReset();
|
||||
fetchDataPointsMock.mockClear();
|
||||
handleErrorMock.mockClear();
|
||||
});
|
||||
|
||||
describe("syncUserActivity — one rollup failure never suppresses siblings", () => {
|
||||
it("keeps fetching distance / active-energy / floors / vo2max after steps 400s", async () => {
|
||||
fetchDailyRollUpMock.mockImplementation(
|
||||
async (_dt: unknown, _token: string, verb: string) => {
|
||||
if (verb === "fetchSteps") {
|
||||
throw new GoogleHealthApiError({
|
||||
verb,
|
||||
classification: "persistent",
|
||||
httpStatus: 400,
|
||||
reason: "HTTP 400",
|
||||
});
|
||||
}
|
||||
return [];
|
||||
},
|
||||
);
|
||||
|
||||
await expect(syncUserActivity("user-1")).resolves.toBe(0);
|
||||
|
||||
const verbs = fetchDailyRollUpMock.mock.calls.map((c) => c[2]);
|
||||
expect(verbs).toEqual([
|
||||
"fetchSteps",
|
||||
"fetchDistance",
|
||||
"fetchActiveEnergy",
|
||||
"fetchFloors",
|
||||
]);
|
||||
// VO2 max (list read) still ran too.
|
||||
expect(fetchDataPointsMock).toHaveBeenCalledTimes(1);
|
||||
// The steps failure was routed through the shared handler exactly once.
|
||||
expect(handleErrorMock).toHaveBeenCalledTimes(1);
|
||||
expect(handleErrorMock).toHaveBeenCalledWith(
|
||||
"fetchSteps",
|
||||
"user-1",
|
||||
expect.any(GoogleHealthApiError),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,91 @@
|
||||
/**
|
||||
* Pins the fail-soft contract of `handleCollectionFetchError`: a hard failure
|
||||
* (anything but the per-class 403 soft-skip) records the classified sync
|
||||
* failure and RETURNS — it must not rethrow, because every call site sits in a
|
||||
* per-collection catch and a rethrow would abort the sibling collections
|
||||
* (live: steps 400ing suppressed distance / floors / active-energy entirely).
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const { recordSyncFailureMock } = vi.hoisted(() => ({
|
||||
recordSyncFailureMock: vi.fn(async () => {}),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({ prisma: {} }));
|
||||
vi.mock("@/lib/crypto", () => ({ encrypt: vi.fn(), decrypt: vi.fn() }));
|
||||
vi.mock("@/lib/integrations/status", () => ({
|
||||
isReauthRequired: vi.fn(async () => false),
|
||||
recordSyncFailure: recordSyncFailureMock,
|
||||
recordSyncSuccess: vi.fn(async () => {}),
|
||||
}));
|
||||
vi.mock("@/lib/rollups/measurement-rollups", () => ({
|
||||
collapseToTypeDayKeys: vi.fn(() => []),
|
||||
recomputeBucketsForMeasurement: vi.fn(async () => {}),
|
||||
recomputeUserRollups: vi.fn(async () => {}),
|
||||
}));
|
||||
vi.mock("@/lib/insights/comprehensive-generate", () => ({
|
||||
invalidateStatusInsightsForTypes: vi.fn(async () => {}),
|
||||
}));
|
||||
vi.mock("../credentials", () => ({
|
||||
getUserGoogleHealthCredentials: vi.fn(async () => null),
|
||||
}));
|
||||
vi.mock("../client", () => ({ refreshAccessToken: vi.fn() }));
|
||||
|
||||
import { handleCollectionFetchError } from "../sync";
|
||||
import { GoogleHealthApiError } from "../response-classifier";
|
||||
|
||||
beforeEach(() => {
|
||||
recordSyncFailureMock.mockClear();
|
||||
});
|
||||
|
||||
describe("handleCollectionFetchError — fail-soft grouping", () => {
|
||||
it("soft-skips a per-class 403 without recording a failure", async () => {
|
||||
const err = new GoogleHealthApiError({
|
||||
verb: "fetchSteps",
|
||||
classification: "reauth_required",
|
||||
httpStatus: 403,
|
||||
reason: "HTTP 403",
|
||||
});
|
||||
await expect(
|
||||
handleCollectionFetchError("fetchSteps", "user-1", err),
|
||||
).resolves.toBe(0);
|
||||
expect(recordSyncFailureMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("records a 400 as a persistent failure and returns instead of rethrowing", async () => {
|
||||
const err = new GoogleHealthApiError({
|
||||
verb: "fetchSteps",
|
||||
classification: "persistent",
|
||||
httpStatus: 400,
|
||||
reason: "HTTP 400",
|
||||
upstreamError: "INVALID_ARGUMENT: Invalid argument in request.",
|
||||
});
|
||||
await expect(
|
||||
handleCollectionFetchError("fetchSteps", "user-1", err),
|
||||
).resolves.toBe(0);
|
||||
expect(recordSyncFailureMock).toHaveBeenCalledTimes(1);
|
||||
expect(recordSyncFailureMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
userId: "user-1",
|
||||
integration: "google-health",
|
||||
kind: "persistent",
|
||||
errorCode: "400",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("records a transient failure (5xx) and returns as well", async () => {
|
||||
const err = new GoogleHealthApiError({
|
||||
verb: "fetchDistance",
|
||||
classification: "transient",
|
||||
httpStatus: 503,
|
||||
reason: "HTTP 503",
|
||||
});
|
||||
await expect(
|
||||
handleCollectionFetchError("fetchDistance", "user-1", err),
|
||||
).resolves.toBe(0);
|
||||
expect(recordSyncFailureMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ kind: "transient" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -190,6 +190,20 @@ interface SoftSkipTracker {
|
||||
}
|
||||
const softSkipStorage = new AsyncLocalStorage<SoftSkipTracker>();
|
||||
|
||||
/**
|
||||
* Per-cycle ledger of collection fetches that HARD-failed (non-403). A hard
|
||||
* failure on one collection must not abort its siblings — steps 400ing must
|
||||
* not suppress distance / floors / active-energy — so
|
||||
* `handleCollectionFetchError` records the failure here and returns instead of
|
||||
* rethrowing. The orchestrator reads the ledger to keep the cycle's verdict
|
||||
* honest: any entry ⇒ the run counts as failed (partial error), the watermark
|
||||
* is NOT stamped, and the next tick refetches the window.
|
||||
*/
|
||||
interface HardFailTracker {
|
||||
failures: string[];
|
||||
}
|
||||
const hardFailStorage = new AsyncLocalStorage<HardFailTracker>();
|
||||
|
||||
/**
|
||||
* Per-cycle accumulator of the `(type, day)` keys every resource's writes
|
||||
* touched. Only populated on a `fullSync` backfill (when the inline per-day
|
||||
@@ -204,10 +218,17 @@ const rollupDeferStorage = new AsyncLocalStorage<RollupDeferTracker>();
|
||||
|
||||
/**
|
||||
* Single-source the per-resource collection-fetch error handling. A 403 on one
|
||||
* data class soft-skips it (warn + return 0) so sibling resources still sync;
|
||||
* anything else records a classified sync failure and rethrows. A soft-skip
|
||||
* increments the ambient tracker so `syncUserGoogleHealth` can refuse to stamp
|
||||
* success on an all-403 grant-revoke cycle that imported nothing.
|
||||
* data class soft-skips it (warn + return 0) so sibling resources still sync; a
|
||||
* soft-skip increments the ambient tracker so `syncUserGoogleHealth` can refuse
|
||||
* to stamp success on an all-403 grant-revoke cycle that imported nothing.
|
||||
*
|
||||
* Anything else records a classified sync failure, warns with the per-type
|
||||
* detail, notes the collection on the ambient hard-fail ledger, and RETURNS —
|
||||
* it does not rethrow. Every call site sits in a per-collection catch, so
|
||||
* returning lets the sibling collections in the same resource keep fetching
|
||||
* (one 400 on steps must not kill distance / floors / active-energy), while
|
||||
* the ledger still fails the cycle so the watermark is not stamped past the
|
||||
* broken collection.
|
||||
*/
|
||||
export async function handleCollectionFetchError(
|
||||
resource: string,
|
||||
@@ -223,7 +244,12 @@ export async function handleCollectionFetchError(
|
||||
return 0;
|
||||
}
|
||||
await recordGoogleHealthSyncFailure(userId, err);
|
||||
throw err;
|
||||
getEvent()?.addWarning(
|
||||
`google-health ${resource} failed for ${userId}: ${err}`,
|
||||
);
|
||||
const hardTracker = hardFailStorage.getStore();
|
||||
if (hardTracker) hardTracker.failures.push(resource);
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -562,31 +588,41 @@ export async function syncUserGoogleHealth(
|
||||
import("./sync-workout"),
|
||||
]);
|
||||
|
||||
// Explicit labels — the production bundle minifies `fn.name` into a useless
|
||||
// single letter ("google-health c failed"), so the warning names the resource
|
||||
// itself.
|
||||
const resources = [
|
||||
syncUserMetrics,
|
||||
syncUserActivity,
|
||||
syncUserSleep,
|
||||
syncUserWorkout,
|
||||
{ name: "metrics", fn: syncUserMetrics },
|
||||
{ name: "activity", fn: syncUserActivity },
|
||||
{ name: "sleep", fn: syncUserSleep },
|
||||
{ name: "workout", fn: syncUserWorkout },
|
||||
];
|
||||
|
||||
const tracker: SoftSkipTracker = { count: 0 };
|
||||
const hardFailTracker: HardFailTracker = { failures: [] };
|
||||
const deferTracker: RollupDeferTracker = { keys: [] };
|
||||
let total = 0;
|
||||
let anyFailed = false;
|
||||
await softSkipStorage.run(tracker, async () => {
|
||||
await rollupDeferStorage.run(deferTracker, async () => {
|
||||
for (const fn of resources) {
|
||||
try {
|
||||
total += await fn(userId, resourceOpts);
|
||||
} catch (err) {
|
||||
anyFailed = true;
|
||||
getEvent()?.addWarning(
|
||||
`google-health ${fn.name} failed for ${userId}: ${err}`,
|
||||
);
|
||||
await hardFailStorage.run(hardFailTracker, async () => {
|
||||
await rollupDeferStorage.run(deferTracker, async () => {
|
||||
for (const { name, fn } of resources) {
|
||||
try {
|
||||
total += await fn(userId, resourceOpts);
|
||||
} catch (err) {
|
||||
anyFailed = true;
|
||||
getEvent()?.addWarning(
|
||||
`google-health ${name} sync failed for ${userId}: ${err}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
// A collection that hard-failed inside a resource (recorded on the ledger by
|
||||
// `handleCollectionFetchError` without aborting its siblings) still fails the
|
||||
// cycle: partial error, no watermark stamp.
|
||||
if (hardFailTracker.failures.length > 0) anyFailed = true;
|
||||
|
||||
// On a `fullSync` backfill the per-write inline rollup hook was deferred; the
|
||||
// accumulated touched type-days collapse into ONE range-recompute spanning the
|
||||
|
||||
Reference in new issue
Block a user