mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(ingest): treat upsert_failed as a retry in the nutrient and cycle batches
The nutrient batch docs told a client to log and drop an `upsert_failed` entry. The route means the opposite: a write group failed on the server's side and nothing was stored. The docs now say to send the entry again on a later sync, and keep the three validation skips terminal. A retry under the same Idempotency-Key used to replay the cached failure, because the 200 carrying `upsert_failed` was cached like any other. Both the nutrient and the cycle bulk routes now mark that response `no-store`, as the measurement batch already does for `failed`, so the retry writes again. The cycle bulk route already answered its validation refusal with `cycle.bulk.invalid` and an over-cap batch with `cycle.bulk.too_large`. Both are now pinned by tests and named in the route's 422 response.
This commit is contained in:
1 parent
a7682a9fed
commit
0cc73d3c88
8 files changed
+181
-10
No files matched your search
+26
-4
@@ -491,7 +491,15 @@ paths:
|
||||
"401": *a1
|
||||
"403": *a3
|
||||
"409": *a7
|
||||
"422": *a4
|
||||
"422":
|
||||
description: "The whole batch was refused and nothing was written: more than 500 entries (`meta.errorCode` =
|
||||
`cycle.bulk.too_large`, no issue list), or the body failed validation (`meta.errorCode` =
|
||||
`cycle.bulk.invalid`, every issue under `details.issues`). Both are permanent for the batch as sent. A
|
||||
problem with one entry never refuses the batch; it comes back as that entry's `skipped` status."
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
$ref: "#/components/schemas/ErrorEnvelope"
|
||||
"429": *a5
|
||||
/api/cycle/period:
|
||||
post:
|
||||
@@ -16810,8 +16818,11 @@ paths:
|
||||
Upsert key is (day, nutrient): a re-post replaces the stored total and reports `updated`. Post the current +
|
||||
previous local day per sync; backfill 30 days on first enable. Max 500 entries; Idempotency-Key supported; rate
|
||||
limit 60/min. Per-entry skips (`unit_mismatch` | `value_out_of_range` | `day_invalid`) are terminal — log and
|
||||
drop, do not retry the entry. Behind the opt-in `nutrients` module (403 `module.disabled` when off). Query
|
||||
HealthKit with exactly the listed unit per code (`ug` = microgram; do not convert to IU):
|
||||
drop, do not retry the entry. `upsert_failed` is not one of them: it is a server-side write failure for the
|
||||
entry's whole write group, nothing was stored, and the entry should be sent again on a later sync; a response
|
||||
carrying it is `Cache-Control: no-store` and is not replayed for its `Idempotency-Key`. Behind the opt-in
|
||||
`nutrients` module (403 `module.disabled` when off). Query HealthKit with exactly the listed unit per code (`ug`
|
||||
= microgram; do not convert to IU):
|
||||
|
||||
|
||||
| code | HealthKit identifier | unit |
|
||||
@@ -30760,6 +30771,12 @@ components:
|
||||
externalId:
|
||||
type: string
|
||||
reason:
|
||||
description: "On a `skipped` entry. `unstable_external_id` is a verdict on the entry and terminal: resending it
|
||||
unchanged is skipped again. `constraint` means the write met a database constraint (a key another
|
||||
write claimed at the same moment, or a reference that no longer exists). `upsert_failed` is a
|
||||
write failure on the server's side: nothing was stored, the entry should be sent again, and the
|
||||
response carrying it is `Cache-Control: no-store`, so a retry under the same `Idempotency-Key`
|
||||
writes again rather than replaying the failure."
|
||||
type: string
|
||||
required:
|
||||
- index
|
||||
@@ -64336,7 +64353,12 @@ components:
|
||||
- status
|
||||
additionalProperties: false
|
||||
description: "Per-entry ingest outcome. skipped reasons: unit_mismatch | value_out_of_range | day_invalid |
|
||||
upsert_failed. Log and drop a skipped entry — do not retry it."
|
||||
upsert_failed. The first three are verdicts on the entry itself and terminal: resending it unchanged is skipped
|
||||
again, so log and drop it. `upsert_failed` is a write failure on the server's side, not a verdict on the entry:
|
||||
nothing was stored for it, and it should be sent again on a later sync. A write failure takes out the whole
|
||||
group it was written in (every new day total, or every existing one), so several entries can carry it at once. A
|
||||
response carrying it is marked `Cache-Control: no-store` and is not kept for `Idempotency-Key` replay, so the
|
||||
retry writes again rather than replaying the failure."
|
||||
NutrientOverviewEnvelope:
|
||||
type: object
|
||||
properties:
|
||||
|
||||
@@ -203,3 +203,82 @@ describe("POST /api/cycle/day-logs/bulk — external-id stability floor", () =>
|
||||
expect(upsertCycleDayLog).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/cycle/day-logs/bulk — refusal codes", () => {
|
||||
/**
|
||||
* The native client files rows under a permanent-refusal register as soon
|
||||
* as it can tell the refusal is permanent. It can only tell from the code,
|
||||
* so both whole-batch 422s are pinned here, with the envelope they ride.
|
||||
*/
|
||||
async function refusal(body: unknown) {
|
||||
const res = await POST(postReq(body));
|
||||
return {
|
||||
status: res.status,
|
||||
body: (await res.json()) as {
|
||||
data: null;
|
||||
error: string;
|
||||
details?: { issues: unknown[] };
|
||||
meta?: { errorCode?: string };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("names a malformed batch cycle.bulk.invalid, with every issue listed", async () => {
|
||||
const { status, body } = await refusal({
|
||||
entries: [{ date: "not-a-day" }, { date: "2026-13-40" }],
|
||||
});
|
||||
expect(status).toBe(422);
|
||||
expect(body.data).toBeNull();
|
||||
expect(body.meta?.errorCode).toBe("cycle.bulk.invalid");
|
||||
expect(body.details?.issues.length).toBeGreaterThanOrEqual(2);
|
||||
expect(upsertCycleDayLog).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("names a batch with no entries array cycle.bulk.invalid", async () => {
|
||||
const { status, body } = await refusal({});
|
||||
expect(status).toBe(422);
|
||||
expect(body.meta?.errorCode).toBe("cycle.bulk.invalid");
|
||||
});
|
||||
|
||||
it("names an over-cap batch cycle.bulk.too_large", async () => {
|
||||
const entries = Array.from({ length: 501 }, (_, i) => ({
|
||||
date: "2026-01-01",
|
||||
externalId: `e${i}`,
|
||||
}));
|
||||
const { status, body } = await refusal({ entries });
|
||||
expect(status).toBe(422);
|
||||
expect(body.meta?.errorCode).toBe("cycle.bulk.too_large");
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/cycle/day-logs/bulk — a failed write is not cached", () => {
|
||||
it("marks a response carrying upsert_failed no-store and leaves a clean one alone", async () => {
|
||||
vi.mocked(upsertCycleDayLog).mockRejectedValueOnce(new Error("reset"));
|
||||
const failed = await POST(
|
||||
postReq({
|
||||
entries: [{ date: "2026-01-01", loggedAt: "2026-01-01T08:00:00.000Z" }],
|
||||
}),
|
||||
);
|
||||
expect(failed.status).toBe(200);
|
||||
const body = (await failed.json()) as {
|
||||
data: { entries: Array<{ status: string; reason?: string }> };
|
||||
};
|
||||
expect(body.data.entries[0]).toMatchObject({
|
||||
status: "skipped",
|
||||
reason: "upsert_failed",
|
||||
});
|
||||
expect(failed.headers.get("Cache-Control")).toBe("private, no-store");
|
||||
|
||||
vi.mocked(upsertCycleDayLog).mockResolvedValueOnce({
|
||||
id: "d1",
|
||||
existed: false,
|
||||
changed: true,
|
||||
} as never);
|
||||
const clean = await POST(
|
||||
postReq({
|
||||
entries: [{ date: "2026-01-02", loggedAt: "2026-01-02T08:00:00.000Z" }],
|
||||
}),
|
||||
);
|
||||
expect(clean.headers.get("Cache-Control")).not.toBe("private, no-store");
|
||||
});
|
||||
});
|
||||
@@ -249,7 +249,7 @@ async function postBulk(request: NextRequest): Promise<Response> {
|
||||
},
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
const response = apiSuccess({
|
||||
processed: entries.length,
|
||||
inserted,
|
||||
updated,
|
||||
@@ -257,4 +257,12 @@ async function postBulk(request: NextRequest): Promise<Response> {
|
||||
skipped,
|
||||
entries: results,
|
||||
});
|
||||
// `upsert_failed` is a write that did not happen, not a verdict on the
|
||||
// entry, and the client sends it again. `no-store` keeps the idempotency
|
||||
// layer from caching this answer, so a retry under the same key writes
|
||||
// instead of replaying the failure.
|
||||
if (results.some((r) => r.reason === "upsert_failed")) {
|
||||
response.headers.set("Cache-Control", "private, no-store");
|
||||
}
|
||||
return response;
|
||||
}
|
||||
@@ -69,6 +69,7 @@ import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { requireModuleEnabled } from "@/lib/modules/gate";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
import { invalidateUserDashboardSnapshot } from "@/lib/cache/invalidate";
|
||||
import { nutrientEntryResultSchema } from "@/lib/validations/nutrients";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
@@ -449,6 +450,46 @@ describe("POST /api/nutrients/batch — write semantics", () => {
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it("marks a response carrying upsert_failed no-store, so a retry with the same Idempotency-Key writes again", async () => {
|
||||
const spy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
vi.mocked(prisma.nutrientIntakeDay.createMany).mockRejectedValue(
|
||||
new Error("connection reset"),
|
||||
);
|
||||
const failed = await POST(
|
||||
postReq({
|
||||
entries: [
|
||||
{ day: recentDay(), nutrient: "zinc", unit: "mg", amount: 10 },
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(failed.status).toBe(200);
|
||||
expect(failed.headers.get("Cache-Control")).toBe("private, no-store");
|
||||
spy.mockRestore();
|
||||
|
||||
vi.mocked(prisma.nutrientIntakeDay.createMany).mockResolvedValue({
|
||||
count: 1,
|
||||
} as never);
|
||||
const clean = await POST(
|
||||
postReq({
|
||||
entries: [
|
||||
{ day: recentDay(), nutrient: "zinc", unit: "mg", amount: 10 },
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(clean.headers.get("Cache-Control")).not.toBe("private, no-store");
|
||||
});
|
||||
|
||||
it("documents upsert_failed as a transient write failure to retry, and the validation skips as terminal", () => {
|
||||
const description = String(
|
||||
(nutrientEntryResultSchema.meta() as { description?: string })
|
||||
.description,
|
||||
);
|
||||
expect(description).toMatch(/`upsert_failed` is a write failure/);
|
||||
expect(description).toMatch(/sent again on a later sync/);
|
||||
expect(description).not.toMatch(/log and drop a skipped entry/i);
|
||||
expect(description).toMatch(/unit_mismatch/);
|
||||
});
|
||||
|
||||
it("a race that drops one createMany row still counts consistently and is observable", async () => {
|
||||
vi.mocked(prisma.nutrientIntakeDay.createMany).mockResolvedValue({
|
||||
count: 1,
|
||||
|
||||
@@ -18,7 +18,10 @@
|
||||
* per-row updates (existing pairs), the measurements-batch shape. A write
|
||||
* failure marks its WHOLE group `skipped upsert_failed` rather than
|
||||
* isolating the one entry that would have caused it (unavoidable once the
|
||||
* write is batched) — the request itself never fails.
|
||||
* write is batched) — the request itself never fails. That reason is
|
||||
* transient, unlike the validation skips: the client sends those entries
|
||||
* again, and the response is marked `no-store` so an idempotent retry is not
|
||||
* answered from the cache.
|
||||
*
|
||||
* Module gate FIRST: the opt-in `nutrients` module (default off)
|
||||
* refuses ingest with the 403 `module.disabled` envelope, so a phone
|
||||
@@ -360,11 +363,19 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
},
|
||||
});
|
||||
|
||||
return apiSuccess({
|
||||
const response = apiSuccess({
|
||||
processed: entries.length,
|
||||
inserted,
|
||||
updated,
|
||||
skipped,
|
||||
entries: results,
|
||||
});
|
||||
// `upsert_failed` is transient: nothing was stored for those entries and the
|
||||
// client sends them again. `no-store` keeps the idempotency layer from
|
||||
// caching this answer, so a retry under the same key writes instead of
|
||||
// replaying the failure. The measurement batch does the same for `failed`.
|
||||
if (skippedByReason.upsert_failed) {
|
||||
response.headers.set("Cache-Control", "private, no-store");
|
||||
}
|
||||
return response;
|
||||
}
|
||||
@@ -420,7 +420,12 @@ const cycleBulkEntryResult = z.object({
|
||||
status: z.enum(["inserted", "duplicate", "updated", "skipped"]),
|
||||
id: z.string().optional(),
|
||||
externalId: z.string().optional(),
|
||||
reason: z.string().optional(),
|
||||
reason: z
|
||||
.string()
|
||||
.optional()
|
||||
.describe(
|
||||
"On a `skipped` entry. `unstable_external_id` is a verdict on the entry and terminal: resending it unchanged is skipped again. `constraint` means the write met a database constraint (a key another write claimed at the same moment, or a reference that no longer exists). `upsert_failed` is a write failure on the server's side: nothing was stored, the entry should be sent again, and the response carrying it is `Cache-Control: no-store`, so a retry under the same `Idempotency-Key` writes again rather than replaying the failure.",
|
||||
),
|
||||
});
|
||||
|
||||
const cycleBulkResponse = z.object({
|
||||
@@ -791,6 +796,11 @@ export const cyclePaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
},
|
||||
...cycleDisabledResponse,
|
||||
...stdResponses,
|
||||
"422": {
|
||||
description:
|
||||
"The whole batch was refused and nothing was written: more than 500 entries (`meta.errorCode` = `cycle.bulk.too_large`, no issue list), or the body failed validation (`meta.errorCode` = `cycle.bulk.invalid`, every issue under `details.issues`). Both are permanent for the batch as sent. A problem with one entry never refuses the batch; it comes back as that entry's `skipped` status.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -56,7 +56,7 @@ export const nutrientPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
tags: ["Nutrients"],
|
||||
summary: "Ingest micronutrient day totals (v1.28)",
|
||||
description:
|
||||
"Bulk upsert of supplement-style daily totals synced from HealthKit — vitamins, minerals, water, caffeine. Day totals only: compute HKStatisticsCollectionQuery cumulative sums day-anchored in the user's current IANA timezone; `day` is yyyy-MM-dd in that timezone (the same rule as the `stats:` day keys). Never post raw samples. Upsert key is (day, nutrient): a re-post replaces the stored total and reports `updated`. Post the current + previous local day per sync; backfill 30 days on first enable. Max 500 entries; Idempotency-Key supported; rate limit 60/min. Per-entry skips (`unit_mismatch` | `value_out_of_range` | `day_invalid`) are terminal — log and drop, do not retry the entry. Behind the opt-in `nutrients` module (403 `module.disabled` when off). Query HealthKit with exactly the listed unit per code (`ug` = microgram; do not convert to IU):\n\n| code | HealthKit identifier | unit |\n| --- | --- | --- |\n" +
|
||||
"Bulk upsert of supplement-style daily totals synced from HealthKit — vitamins, minerals, water, caffeine. Day totals only: compute HKStatisticsCollectionQuery cumulative sums day-anchored in the user's current IANA timezone; `day` is yyyy-MM-dd in that timezone (the same rule as the `stats:` day keys). Never post raw samples. Upsert key is (day, nutrient): a re-post replaces the stored total and reports `updated`. Post the current + previous local day per sync; backfill 30 days on first enable. Max 500 entries; Idempotency-Key supported; rate limit 60/min. Per-entry skips (`unit_mismatch` | `value_out_of_range` | `day_invalid`) are terminal — log and drop, do not retry the entry. `upsert_failed` is not one of them: it is a server-side write failure for the entry's whole write group, nothing was stored, and the entry should be sent again on a later sync; a response carrying it is `Cache-Control: no-store` and is not replayed for its `Idempotency-Key`. Behind the opt-in `nutrients` module (403 `module.disabled` when off). Query HealthKit with exactly the listed unit per code (`ug` = microgram; do not convert to IU):\n\n| code | HealthKit identifier | unit |\n| --- | --- | --- |\n" +
|
||||
catalogTable,
|
||||
requestBody: {
|
||||
required: true,
|
||||
|
||||
@@ -64,7 +64,7 @@ export const nutrientEntryResultSchema = z
|
||||
.meta({
|
||||
id: "NutrientEntryResult",
|
||||
description:
|
||||
"Per-entry ingest outcome. skipped reasons: unit_mismatch | value_out_of_range | day_invalid | upsert_failed. Log and drop a skipped entry — do not retry it.",
|
||||
"Per-entry ingest outcome. skipped reasons: unit_mismatch | value_out_of_range | day_invalid | upsert_failed. The first three are verdicts on the entry itself and terminal: resending it unchanged is skipped again, so log and drop it. `upsert_failed` is a write failure on the server's side, not a verdict on the entry: nothing was stored for it, and it should be sent again on a later sync. A write failure takes out the whole group it was written in (every new day total, or every existing one), so several entries can carry it at once. A response carrying it is marked `Cache-Control: no-store` and is not kept for `Idempotency-Key` replay, so the retry writes again rather than replaying the failure.",
|
||||
});
|
||||
|
||||
export const nutrientBatchResponseSchema = z
|
||||
|
||||
Reference in new issue
Block a user