mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(measurements): name the batch validation refusal and the band an entry broke
The validation 422 from `POST /api/measurements/batch` now carries `meta.errorCode: "measurement.batch.invalid"`, beside the existing `too_large` and `source_not_permitted` codes, so a client can file the rows as permanently refused instead of holding its anchor on a guess. A `value_out_of_range` entry now also carries `convertedValue`, the reading after conversion into the stored unit, and `range` (`min`, `max`, `unit`), the band it was checked against. Both appear only on that skip. A fraction sent where a percent was expected is now visible in the response instead of looking like a sensor glitch. Status codes, the envelope and every existing field are unchanged.
This commit is contained in:
1 parent
74dedee645
commit
a7682a9fed
5 files changed
+320
-15
No files matched your search
+27
-3
@@ -3936,9 +3936,11 @@ paths:
|
||||
"401": *a1
|
||||
"409": *a7
|
||||
"422":
|
||||
description: The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation, or
|
||||
description: "The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation
|
||||
(`measurement.batch.invalid`, with every issue under `details.issues`), or
|
||||
(`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow
|
||||
`measurements:write` credential. Nothing was written in any of the three cases.
|
||||
`measurements:write` credential. Nothing was written in any of the three cases, and all three are permanent
|
||||
for the batch as sent: resending it unchanged is refused the same way."
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
@@ -30284,7 +30286,7 @@ components:
|
||||
labs.update.linkedFieldsImmutable, labs.update.numericExpected, labs.update.qualitativeExpected,
|
||||
labs.update.referenceRangeInvalid, labs.update.sourceReferenceRangeInvalid; managed_profile —
|
||||
managed_profile.guardian.duplicate, managed_profile.guardian.managed_invitee,
|
||||
managed_profile.guardian.required, managed_profile.not_found; measurement —
|
||||
managed_profile.guardian.required, managed_profile.not_found; measurement — measurement.batch.invalid,
|
||||
measurement.batch.source_not_permitted, measurement.batch.too_large, measurement.bulk-delete.invalid,
|
||||
measurement.create.invalid, measurement.create.source_not_permitted, measurement.delete.too_large,
|
||||
measurement.duplicate_timestamp, measurement.restore.invalid, measurement.update.server_owned_source;
|
||||
@@ -39736,6 +39738,28 @@ components:
|
||||
not advance the entry cursor; the response is marked `Cache-Control: no-store`."
|
||||
reason:
|
||||
type: string
|
||||
convertedValue:
|
||||
description: "Only on a `value_out_of_range` skip (since v1.39.1): the entry's value after conversion into the stored
|
||||
unit, which is the number the band was checked against. A HealthKit fraction that should have been a percent
|
||||
shows up here a hundred times off."
|
||||
type: number
|
||||
range:
|
||||
description: "Only on a `value_out_of_range` skip (since v1.39.1): the plausibility band that refused the entry, in the
|
||||
stored unit. Both edges are inclusive. A skip is permanent for the value as sent; resending it unchanged
|
||||
skips it again."
|
||||
type: object
|
||||
properties:
|
||||
min:
|
||||
type: number
|
||||
max:
|
||||
type: number
|
||||
unit:
|
||||
type: string
|
||||
required:
|
||||
- min
|
||||
- max
|
||||
- unit
|
||||
additionalProperties: false
|
||||
required:
|
||||
- index
|
||||
- status
|
||||
|
||||
@@ -0,0 +1,231 @@
|
||||
/**
|
||||
* `POST /api/measurements/batch` — what a refusal tells the client.
|
||||
*
|
||||
* Two gaps the native client reported against v1.39.0:
|
||||
*
|
||||
* - The whole-batch validation 422 carried no `meta.errorCode`, so the
|
||||
* client could not tell a permanent refusal from a transient one and had
|
||||
* to hold its anchor for several syncs before giving up on the rows.
|
||||
* The too-large and source refusals already had codes; this one now has
|
||||
* `measurement.batch.invalid`.
|
||||
* - A `value_out_of_range` skip said neither what the value became after
|
||||
* conversion nor which band it broke, so a unit-mapping slip (a fraction
|
||||
* sent where a percent was expected) looked the same as a sensor glitch.
|
||||
* The entry now carries `convertedValue` and `range`.
|
||||
*
|
||||
* Both are additive: status codes, the envelope and every existing field
|
||||
* are unchanged.
|
||||
*/
|
||||
import { describe, expect, it, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
user: { update: vi.fn() },
|
||||
measurement: { findMany: vi.fn() },
|
||||
$transaction: vi.fn(async (fn: unknown) => {
|
||||
if (typeof fn === "function") {
|
||||
return (fn as (tx: unknown) => unknown)(prisma);
|
||||
}
|
||||
}),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/measurements/reconcile-external-measurement", () => ({
|
||||
reconcileExternalMeasurement: vi.fn(
|
||||
async (_tx: unknown, input: Record<string, unknown>) => ({
|
||||
status: "inserted",
|
||||
row: { id: "row-1", ...input },
|
||||
}),
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() }));
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() }));
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn() }));
|
||||
vi.mock("@/lib/jobs/pr-detection", () => ({
|
||||
enqueuePrDetection: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/jobs/reminder-satisfy", () => ({
|
||||
enqueueReminderSatisfy: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/cache/invalidate", () => ({
|
||||
invalidateUserMeasurements: vi.fn(),
|
||||
}));
|
||||
vi.mock("@/lib/rollups/after-measurement-mutation", () => ({
|
||||
afterMeasurementMutation: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/daily/morning-refresh-trigger", () => ({
|
||||
maybeEnqueueMorningRefresh: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("@/lib/arrivals/emit-shared", () => ({
|
||||
emitDataArrival: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
vi.mock("next/headers", () => ({
|
||||
headers: vi.fn(async () => ({ get: () => null })),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: () => undefined,
|
||||
set: () => {},
|
||||
delete: () => {},
|
||||
})),
|
||||
}));
|
||||
|
||||
import { POST } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
user: { id: "user-1", username: "testuser", role: "USER" as const },
|
||||
};
|
||||
|
||||
function post(body: unknown) {
|
||||
return POST(
|
||||
new NextRequest("http://localhost/api/measurements/batch", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const AT = new Date(Date.now() - 3_600_000).toISOString();
|
||||
|
||||
function entry(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
hkIdentifier: "HKQuantityTypeIdentifierHeartRate",
|
||||
value: 64,
|
||||
unit: "count/min",
|
||||
startDate: AT,
|
||||
endDate: AT,
|
||||
externalId: "uuid-1",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
type EntryResult = {
|
||||
index: number;
|
||||
status: string;
|
||||
reason?: string;
|
||||
convertedValue?: number;
|
||||
range?: { min: number; max: number; unit: string };
|
||||
};
|
||||
|
||||
async function entriesFor(entries: unknown[]): Promise<EntryResult[]> {
|
||||
const res = await post({ entries });
|
||||
expect(res.status).toBe(200);
|
||||
return ((await res.json()) as { data: { entries: EntryResult[] } }).data
|
||||
.entries;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(checkRateLimit).mockResolvedValue({
|
||||
allowed: true,
|
||||
limit: 60,
|
||||
remaining: 60,
|
||||
resetAt: Date.now() + 60_000,
|
||||
});
|
||||
vi.mocked(prisma.measurement.findMany).mockResolvedValue([]);
|
||||
});
|
||||
|
||||
describe("POST /api/measurements/batch — validation refusal code", () => {
|
||||
it("names a malformed batch measurement.batch.invalid and keeps the 422 envelope", async () => {
|
||||
const res = await post({ entries: [entry({ startDate: "yesterday" })] });
|
||||
expect(res.status).toBe(422);
|
||||
const body = (await res.json()) as {
|
||||
data: null;
|
||||
error: string;
|
||||
details: { issues: unknown[] };
|
||||
meta?: { errorCode?: string };
|
||||
};
|
||||
expect(body.data).toBeNull();
|
||||
expect(typeof body.error).toBe("string");
|
||||
expect(body.details.issues.length).toBeGreaterThan(0);
|
||||
expect(body.meta?.errorCode).toBe("measurement.batch.invalid");
|
||||
});
|
||||
|
||||
it("names an empty entries array the same way", async () => {
|
||||
const res = await post({ entries: [] });
|
||||
expect(res.status).toBe(422);
|
||||
expect(
|
||||
((await res.json()) as { meta?: { errorCode?: string } }).meta?.errorCode,
|
||||
).toBe("measurement.batch.invalid");
|
||||
});
|
||||
|
||||
it("keeps measurement.batch.too_large for an over-cap batch", async () => {
|
||||
const entries = Array.from({ length: 501 }, (_, i) =>
|
||||
entry({ externalId: `uuid-${i}` }),
|
||||
);
|
||||
const res = await post({ entries });
|
||||
expect(res.status).toBe(422);
|
||||
expect(
|
||||
((await res.json()) as { meta?: { errorCode?: string } }).meta?.errorCode,
|
||||
).toBe("measurement.batch.too_large");
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /api/measurements/batch — value_out_of_range detail", () => {
|
||||
it("reports the converted value and the band it broke, in the stored unit", async () => {
|
||||
// HealthKit reports body fat as a fraction; 0.9 becomes 90 %, above 80.
|
||||
const [result] = await entriesFor([
|
||||
entry({
|
||||
hkIdentifier: "HKQuantityTypeIdentifierBodyFatPercentage",
|
||||
value: 0.9,
|
||||
unit: "%",
|
||||
}),
|
||||
]);
|
||||
expect(result).toEqual({
|
||||
index: 0,
|
||||
status: "skipped",
|
||||
reason: "value_out_of_range",
|
||||
convertedValue: 90,
|
||||
range: { min: 1, max: 80, unit: "%" },
|
||||
});
|
||||
});
|
||||
|
||||
it("reports the lower edge too", async () => {
|
||||
const [result] = await entriesFor([entry({ value: 12 })]);
|
||||
expect(result).toMatchObject({
|
||||
reason: "value_out_of_range",
|
||||
convertedValue: 12,
|
||||
range: { min: 20, max: 300, unit: "bpm" },
|
||||
});
|
||||
});
|
||||
|
||||
it("carries the same detail when the write-side gate is the one that refuses", async () => {
|
||||
vi.mocked(reconcileExternalMeasurement).mockResolvedValueOnce({
|
||||
status: "rejected_range",
|
||||
rejection: { type: "PULSE", direction: "above_max" },
|
||||
});
|
||||
const [result] = await entriesFor([entry({ value: 64 })]);
|
||||
expect(result).toMatchObject({
|
||||
status: "skipped",
|
||||
reason: "value_out_of_range",
|
||||
convertedValue: 64,
|
||||
range: { min: 20, max: 300, unit: "bpm" },
|
||||
});
|
||||
});
|
||||
|
||||
it("adds nothing to an entry that landed or was skipped for another reason", async () => {
|
||||
const results = await entriesFor([
|
||||
entry({ externalId: "uuid-ok" }),
|
||||
entry({ hkIdentifier: "HKQuantityTypeIdentifierNotAThing" }),
|
||||
]);
|
||||
expect(results[0]).toEqual({ index: 0, status: "inserted" });
|
||||
expect(results[1]).toEqual({
|
||||
index: 1,
|
||||
status: "skipped",
|
||||
reason: "unmappable_identifier",
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -71,7 +71,11 @@ import {
|
||||
UNSTABLE_EXTERNAL_ID_REASON,
|
||||
type UnstableExternalIdShape,
|
||||
} from "@/lib/validations/external-id";
|
||||
import { validateMeasurementRange } from "@/lib/validations/measurement";
|
||||
import {
|
||||
VALUE_RANGES,
|
||||
getUnitForType,
|
||||
validateMeasurementRange,
|
||||
} from "@/lib/validations/measurement";
|
||||
import { deviceTypeEnum } from "@/lib/validations/source-priority";
|
||||
import { checkRateLimit } from "@/lib/rate-limit";
|
||||
import { enqueuePrDetection } from "@/lib/jobs/pr-detection";
|
||||
@@ -216,6 +220,36 @@ interface EntryResult {
|
||||
index: number;
|
||||
status: EntryStatus;
|
||||
reason?: string;
|
||||
/**
|
||||
* Only on a `value_out_of_range` skip: the value after conversion into the
|
||||
* stored unit, which is what the band was checked against.
|
||||
*/
|
||||
convertedValue?: number;
|
||||
/** Only on a `value_out_of_range` skip: the band applied, in the stored unit. */
|
||||
range?: { min: number; max: number; unit: string };
|
||||
}
|
||||
|
||||
/**
|
||||
* The `value_out_of_range` skip, with what a client needs to say which limit
|
||||
* applied. The value is the entry's own reading after conversion, handed back
|
||||
* to the account that sent it; a fraction sent where a percent was expected
|
||||
* shows up here as a converted value a hundred times too small or too large.
|
||||
*/
|
||||
function outOfRangeResult(
|
||||
index: number,
|
||||
type: string,
|
||||
convertedValue: number,
|
||||
): EntryResult {
|
||||
const band = VALUE_RANGES[type];
|
||||
return {
|
||||
index,
|
||||
status: "skipped",
|
||||
reason: "value_out_of_range",
|
||||
...(Number.isFinite(convertedValue) ? { convertedValue } : {}),
|
||||
...(band
|
||||
? { range: { min: band.min, max: band.max, unit: getUnitForType(type) } }
|
||||
: {}),
|
||||
};
|
||||
}
|
||||
|
||||
export const POST = apiHandler(withIdempotency<[NextRequest]>(postBatch));
|
||||
@@ -276,10 +310,13 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
|
||||
const parsed = batchPayloadSchema.safeParse(rawBody);
|
||||
if (!parsed.success) {
|
||||
// The code is what lets a client tell this refusal from a transient one:
|
||||
// the batch as sent will never be accepted, so resending it is pointless.
|
||||
return apiValidationError(
|
||||
parsed.error.issues[0]?.message ?? "Invalid batch",
|
||||
sanitiseZodIssues(parsed.error.issues),
|
||||
422,
|
||||
{ errorCode: "measurement.batch.invalid" },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -363,11 +400,7 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
// sensor reading shouldn't poison an otherwise clean ingest.
|
||||
const rangeError = validateMeasurementRange(mapped.type, mapped.value);
|
||||
if (rangeError !== null) {
|
||||
results[index] = {
|
||||
index,
|
||||
status: "skipped",
|
||||
reason: "value_out_of_range",
|
||||
};
|
||||
results[index] = outOfRangeResult(index, mapped.type, mapped.value);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -666,11 +699,11 @@ async function postBatch(request: NextRequest): Promise<Response> {
|
||||
// already applied above, so reaching here means the two disagreed —
|
||||
// report it exactly as the earlier guard would have, and let the
|
||||
// wide-event tally record which end refused.
|
||||
results[p.index] = {
|
||||
index: p.index,
|
||||
status: "skipped",
|
||||
reason: "value_out_of_range",
|
||||
};
|
||||
results[p.index] = outOfRangeResult(
|
||||
p.index,
|
||||
p.row.type as string,
|
||||
p.row.value as number,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -198,6 +198,7 @@ export const ERROR_CODE_CATALOGUE: Readonly<Record<string, readonly string[]>> =
|
||||
"managed_profile.not_found",
|
||||
],
|
||||
measurement: [
|
||||
"measurement.batch.invalid",
|
||||
"measurement.batch.source_not_permitted",
|
||||
"measurement.batch.too_large",
|
||||
"measurement.bulk-delete.invalid",
|
||||
|
||||
@@ -147,6 +147,22 @@ const batchEntryResult = z
|
||||
"`inserted`/`duplicate` — the row landed (advance the cursor). `updated` — a `stats:` aggregate overwrote an existing row. `skipped` — validation no-op; see `reason`. `failed` — retryable database failure that must not advance the entry cursor; the response is marked `Cache-Control: no-store`.",
|
||||
),
|
||||
reason: z.string().optional(),
|
||||
convertedValue: z
|
||||
.number()
|
||||
.optional()
|
||||
.describe(
|
||||
"Only on a `value_out_of_range` skip (since v1.39.1): the entry's value after conversion into the stored unit, which is the number the band was checked against. A HealthKit fraction that should have been a percent shows up here a hundred times off.",
|
||||
),
|
||||
range: z
|
||||
.object({
|
||||
min: z.number(),
|
||||
max: z.number(),
|
||||
unit: z.string(),
|
||||
})
|
||||
.optional()
|
||||
.describe(
|
||||
"Only on a `value_out_of_range` skip (since v1.39.1): the plausibility band that refused the entry, in the stored unit. Both edges are inclusive. A skip is permanent for the value as sent; resending it unchanged skips it again.",
|
||||
),
|
||||
})
|
||||
.meta({ id: "AppleHealthBatchEntryResult" });
|
||||
|
||||
@@ -1011,7 +1027,7 @@ export const measurementPaths: NonNullable<ZodOpenApiObject["paths"]> = {
|
||||
...stdResponses,
|
||||
"422": {
|
||||
description:
|
||||
"The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation, or (`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow `measurements:write` credential. Nothing was written in any of the three cases.",
|
||||
"The batch exceeded the 500-entry limit (`measurement.batch.too_large`), failed validation (`measurement.batch.invalid`, with every issue under `details.issues`), or (`measurement.batch.source_not_permitted`) carried an entry naming `APPLE_HEALTH` under a narrow `measurements:write` credential. Nothing was written in any of the three cases, and all three are permanent for the batch as sent: resending it unchanged is refused the same way.",
|
||||
content: { "application/json": { schema: errorEnvelope } },
|
||||
},
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user