Skip to content

Commit 30acfe6

Browse files
committed
feat: tighten v2 payload diagnostics
1 parent 2f8cb21 commit 30acfe6

File tree

3 files changed

+253
-6
lines changed

3 files changed

+253
-6
lines changed

src/controllers/settingsController.ts

Lines changed: 156 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import fs from 'fs/promises';
2+
import os from 'os';
23
import path from 'path';
34
import { Reader } from 'comapeocat'; // Added Reader import
45
import { buildSettingsV1 } from '../services/settingsBuilder';
56
import { buildComapeoCatV2 } from '../services/comapeocatBuilder';
67
import { ValidationError, ProcessingError } from '../types/errors';
7-
import type { BuildRequestV2 } from '../types/v2';
8+
import type { BuildRequestV2, IconInput } from '../types/v2';
89
import { config } from '../config/app';
910

1011
/**
@@ -143,7 +144,7 @@ export async function handleBuildSettingsV2(payload: BuildRequestV2) {
143144

144145
return new Response(cleanupStream, { headers });
145146
} catch (error) {
146-
logV2PayloadForFailure(payload, error);
147+
await logV2PayloadForFailure(payload, error);
147148
throw error;
148149
}
149150
}
@@ -183,11 +184,160 @@ function formatWarningsHeader(warnings: string[]) {
183184
return joined.length > 1024 ? `${joined.slice(0, 1021)}...` : joined;
184185
}
185186

186-
function logV2PayloadForFailure(payload: BuildRequestV2, error: unknown) {
187+
const PAYLOAD_LOG_PREFIX = 'comapeo-v2-payload-';
188+
const SUMMARY_SAMPLE_LIMIT = 8;
189+
const ISSUE_SAMPLE_LIMIT = 5;
190+
191+
async function logV2PayloadForFailure(payload: BuildRequestV2, error: unknown) {
187192
try {
188-
const serializedPayload = JSON.stringify(payload, null, 2);
189-
console.error('[COMAPEO-API][ERROR] /v2 build failed. Payload dump follows:', serializedPayload, error);
193+
const summary = buildPayloadSummary(payload);
194+
const payloadPath = await persistPayloadSnapshot(payload);
195+
console.error(
196+
`[COMAPEO-API][ERROR] /v2 build failed. Summary: ${JSON.stringify(summary)}. Full payload saved to: ${payloadPath}`,
197+
error
198+
);
190199
} catch (serializationError) {
191-
console.error('[COMAPEO-API][ERROR] /v2 build failed and payload serialization threw an error:', serializationError, error);
200+
console.error(
201+
'[COMAPEO-API][ERROR] /v2 build failed and payload logging also failed:',
202+
serializationError,
203+
error
204+
);
192205
}
193206
}
207+
208+
function buildPayloadSummary(payload: BuildRequestV2) {
209+
const categories = Array.isArray(payload.categories) ? payload.categories : [];
210+
const fields = Array.isArray(payload.fields) ? payload.fields : [];
211+
const icons = Array.isArray(payload.icons) ? payload.icons : [];
212+
const translationLocales = payload.translations && typeof payload.translations === 'object'
213+
? Object.keys(payload.translations)
214+
: [];
215+
216+
const categoryIds = categories.map((c) => c.id).filter(Boolean);
217+
const fieldIds = fields.map((f) => f.id).filter(Boolean);
218+
const localesSample = truncateList(translationLocales, SUMMARY_SAMPLE_LIMIT);
219+
const categoriesWithoutFields = categories
220+
.filter((category) => !Array.isArray(category.fields) && !Array.isArray(category.defaultFieldIds))
221+
.map((category) => category.id);
222+
223+
const iconIds = new Set(icons.map((icon) => icon.id).filter(Boolean));
224+
const referencedIconIds = categories
225+
.map((category) => {
226+
const directIcon = (category as any).icon;
227+
if (typeof directIcon === 'string' && directIcon.trim()) {
228+
return directIcon.trim();
229+
}
230+
if (typeof category.iconId === 'string' && category.iconId.trim()) {
231+
return category.iconId.trim();
232+
}
233+
return undefined;
234+
})
235+
.filter((value): value is string => Boolean(value));
236+
237+
const missingIconRefs = iconIds.size > 0
238+
? Array.from(new Set(referencedIconIds.filter((ref) => !iconIds.has(ref))))
239+
: [];
240+
241+
const potentialIssues: string[] = [];
242+
243+
if (categoriesWithoutFields.length > 0) {
244+
potentialIssues.push(
245+
formatIssue(
246+
'Categories without field references',
247+
categoriesWithoutFields,
248+
ISSUE_SAMPLE_LIMIT
249+
)
250+
);
251+
}
252+
253+
if (missingIconRefs.length > 0) {
254+
potentialIssues.push(
255+
formatIssue('Categories referencing missing icon ids', missingIconRefs, ISSUE_SAMPLE_LIMIT)
256+
);
257+
}
258+
259+
const iconStats = summarizeIconStats(icons);
260+
261+
return {
262+
metadata: {
263+
name: payload.metadata?.name || 'unknown',
264+
version: payload.metadata?.version || 'n/a',
265+
},
266+
counts: {
267+
categories: categories.length,
268+
fields: fields.length,
269+
icons: icons.length,
270+
translations: translationLocales.length,
271+
},
272+
samples: {
273+
categoryIds: truncateList(categoryIds, SUMMARY_SAMPLE_LIMIT),
274+
fieldIds: truncateList(fieldIds, SUMMARY_SAMPLE_LIMIT),
275+
locales: localesSample,
276+
},
277+
potentialIssues,
278+
iconStats,
279+
};
280+
}
281+
282+
function summarizeIconStats(icons: IconInput[]) {
283+
if (!icons || icons.length === 0) {
284+
return undefined;
285+
}
286+
287+
let inlineCount = 0;
288+
let dataUriCount = 0;
289+
let remoteCount = 0;
290+
let missingSourceCount = 0;
291+
let inlineBytesTotal = 0;
292+
let inlineBytesMax = 0;
293+
294+
for (const icon of icons) {
295+
if (typeof icon.svgData === 'string') {
296+
inlineCount += 1;
297+
const size = Buffer.byteLength(icon.svgData, 'utf-8');
298+
inlineBytesTotal += size;
299+
inlineBytesMax = Math.max(inlineBytesMax, size);
300+
} else if (typeof icon.svgUrl === 'string') {
301+
if (icon.svgUrl.startsWith('data:image/svg+xml')) {
302+
dataUriCount += 1;
303+
} else {
304+
remoteCount += 1;
305+
}
306+
} else {
307+
missingSourceCount += 1;
308+
}
309+
}
310+
311+
const inlineAvg = inlineCount > 0 ? Math.round(inlineBytesTotal / inlineCount) : 0;
312+
313+
return {
314+
inlineCount,
315+
dataUriCount,
316+
remoteCount,
317+
missingSourceCount,
318+
inlineBytesTotal,
319+
inlineBytesMax,
320+
inlineBytesAvg: inlineAvg,
321+
};
322+
}
323+
324+
function formatIssue(label: string, values: string[], limit: number) {
325+
const uniqueValues = Array.from(new Set(values));
326+
const sample = truncateList(uniqueValues, limit);
327+
const excess = uniqueValues.length - sample.length;
328+
return excess > 0 ? `${label}: ${sample.join(', ')} (+${excess} more)` : `${label}: ${sample.join(', ')}`;
329+
}
330+
331+
function truncateList<T>(values: T[], limit: number): T[] {
332+
if (!Array.isArray(values) || values.length <= limit) {
333+
return values.slice();
334+
}
335+
return values.slice(0, limit);
336+
}
337+
338+
async function persistPayloadSnapshot(payload: BuildRequestV2) {
339+
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), PAYLOAD_LOG_PREFIX));
340+
const filePath = path.join(tempDir, `payload-${Date.now()}.json`);
341+
await fs.writeFile(filePath, JSON.stringify(payload, null, 2), 'utf-8');
342+
return filePath;
343+
}

src/services/comapeocatBuilder.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { config } from '../config/app';
1010
import { ProcessingError, ValidationError } from '../types/errors';
1111
import type {
1212
BuildRequestV2,
13+
CategoryInput,
1314
ComapeoFieldType,
1415
FieldInput,
1516
MappedCategory,
@@ -42,6 +43,12 @@ export async function buildComapeoCatV2(payload: BuildRequestV2): Promise<BuildR
4243
// Layer 1 enforces size during streaming in app.ts onParse hook
4344
enforcePayloadSize(payload);
4445
enforceEntryCap(payload);
46+
enforceUniqueIds(payload.fields, 'field');
47+
enforceUniqueIds(payload.categories, 'category');
48+
if (Array.isArray(payload.icons) && payload.icons.length > 0) {
49+
enforceUniqueIds(payload.icons, 'icon');
50+
}
51+
validateCategoryFieldReferences(payload.categories, payload.fields);
4552

4653
const mapped = await transformPayload(payload);
4754
const warnings: string[] = [...mapped.warnings];
@@ -845,6 +852,63 @@ function normalizeToNodeStream(streamLike: any) {
845852
throw new ProcessingError('Writer outputStream is not readable');
846853
}
847854

855+
function enforceUniqueIds<T extends { id?: string }>(items: T[], entityName: string) {
856+
const seen = new Set<string>();
857+
858+
for (const item of items) {
859+
const rawId = typeof item?.id === 'string' ? item.id.trim() : '';
860+
if (!rawId) {
861+
throw new ValidationError(`${entityName} id must be a non-empty string`);
862+
}
863+
864+
if (seen.has(rawId)) {
865+
throw new ValidationError(`Duplicate ${entityName} id detected: ${rawId}`);
866+
}
867+
868+
seen.add(rawId);
869+
}
870+
}
871+
872+
function validateCategoryFieldReferences(categories: CategoryInput[], fields: FieldInput[]) {
873+
const fieldIds = new Set(fields.map((field) => field.id).filter((id): id is string => Boolean(id)));
874+
const issues: string[] = [];
875+
876+
for (const category of categories) {
877+
const refs = new Set<string>();
878+
879+
if (Array.isArray(category.fields)) {
880+
for (const ref of category.fields) {
881+
if (typeof ref === 'string' && ref.trim()) {
882+
refs.add(ref.trim());
883+
}
884+
}
885+
}
886+
887+
if (Array.isArray(category.defaultFieldIds)) {
888+
for (const ref of category.defaultFieldIds) {
889+
if (typeof ref === 'string' && ref.trim()) {
890+
refs.add(ref.trim());
891+
}
892+
}
893+
}
894+
895+
if (refs.size === 0) {
896+
continue;
897+
}
898+
899+
const missing = Array.from(refs).filter((ref) => !fieldIds.has(ref));
900+
if (missing.length > 0) {
901+
issues.push(`${category.id}: ${missing.join(', ')}`);
902+
}
903+
}
904+
905+
if (issues.length > 0) {
906+
throw new ValidationError(
907+
`Categories reference unknown field ids. Missing references -> ${issues.join(' | ')}`
908+
);
909+
}
910+
}
911+
848912
/**
849913
* Sanitizes a string to be safe for use as a path component.
850914
* Rejects inputs containing path separators or traversal patterns to prevent attacks.
@@ -889,6 +953,8 @@ export const __test__ = {
889953
normalizeTags,
890954
enforcePayloadSize,
891955
enforceEntryCap,
956+
enforceUniqueIds,
957+
validateCategoryFieldReferences,
892958
sanitizePathComponent,
893959
decodeDataUri,
894960
normalizeIconId,

src/tests/unit/services/comapeocatBuilder.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,30 @@ describe('comapeocatBuilder helpers', () => {
2020
expect(() => __test__.mapFieldType(badField.type as any)).toThrow(ValidationError);
2121
});
2222

23+
it('throws when duplicate field ids are provided', async () => {
24+
const payload = createBasePayload();
25+
payload.fields.push({ ...payload.fields[0] });
26+
27+
await expect(buildComapeoCatV2(payload)).rejects.toThrow('Duplicate field id');
28+
});
29+
30+
it('throws when duplicate category ids are provided', async () => {
31+
const payload = createBasePayload();
32+
payload.categories.push({ ...payload.categories[0] });
33+
34+
await expect(buildComapeoCatV2(payload)).rejects.toThrow('Duplicate category id');
35+
});
36+
37+
it('throws when duplicate icon ids are provided', async () => {
38+
const payload = createBasePayload();
39+
payload.icons = [
40+
{ id: 'icon-1', svgData: '<svg />' },
41+
{ id: 'icon-1', svgData: '<svg />' },
42+
];
43+
44+
await expect(buildComapeoCatV2(payload)).rejects.toThrow('Duplicate icon id');
45+
});
46+
2347
it('fails when appliesTo is missing', () => {
2448
expect(() => __test__.mapCategory({ id: 'cat', name: 'Cat' }, 0)).toThrow(ValidationError);
2549
});
@@ -72,6 +96,13 @@ describe('comapeocatBuilder helpers', () => {
7296
expect(() => __test__.mapCategory(category, 0)).toThrow('appliesTo must only contain "observation" or "track"');
7397
});
7498

99+
it('throws when a category references unknown field ids', async () => {
100+
const payload = createBasePayload();
101+
payload.categories[0].fields = ['missing-field'];
102+
103+
await expect(buildComapeoCatV2(payload)).rejects.toThrow('unknown field ids');
104+
});
105+
75106
it('throws when appliesTo contains mix of valid and invalid values', () => {
76107
const category = { id: 'cat', name: 'Cat', appliesTo: ['observation', 'invalid', 'track'] };
77108
expect(() => __test__.mapCategory(category, 0)).toThrow(ValidationError);

0 commit comments

Comments
 (0)