Simplified Airtable validation warning logging

apps/pg-sync-service/src/lib/db.ts

@@ -1,5 +1,6 @@
-import { PgAirtableDb, createTestDbClients } from '@bluedot/db';
+import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db';
 import { slackAlert } from '@bluedot/utils';
+import { logger } from '@bluedot/ui/src/api';
 import env from '../env';
 
 const isTest = env.VITEST === 'true';
@@ -17,20 +18,16 @@ export const db = new PgAirtableDb({
   airtableApiKey: env.AIRTABLE_PERSONAL_ACCESS_TOKEN,
   ...(isTest ? createTestDbClients() : {}),
   onWarning: async (warning: unknown) => {
-    const err = warning instanceof Error ? warning : new Error(String(warning));
-    const message = `Airtable validation warning encountered, attempting to proceed by setting the affected fields to undefined. Warning message: ${err.message}`;
+    const { message, messages, batchGroup } = formatAirtableWarning(warning);
 
-    // eslint-disable-next-line no-console
-    console.warn(message);
+    logger.warn(message);
 
-    await slackAlert(env, [
-      message,
-      ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : []),
-    ], {
+    await slackAlert(env, messages, {
       channelId: env.PG_SYNC_SLACK_CHANNEL_ID,
       batchKey: 'airtable-validation',
       spikeThreshold: VALIDATION_WARNING_SPIKE_THRESHOLD,
       escalationChannelId: env.ALERTS_SLACK_CHANNEL_ID,
+      batchGroup,
     });
   },
 });

apps/website/src/lib/api/db/index.ts

@@ -1,5 +1,6 @@
-import { PgAirtableDb, createTestDbClients } from '@bluedot/db';
+import { PgAirtableDb, createTestDbClients, formatAirtableWarning } from '@bluedot/db';
 import { slackAlert } from '@bluedot/utils';
+import { logger } from '@bluedot/ui/src/api';
 import env from '../env';
 
 const isTest = env.VITEST === 'true';
@@ -9,10 +10,9 @@ export default new PgAirtableDb({
   airtableApiKey: env.AIRTABLE_PERSONAL_ACCESS_TOKEN,
   ...(isTest ? createTestDbClients() : {}),
   async onWarning(warning: unknown) {
-    const err = warning instanceof Error ? warning : new Error(String(warning));
-    await slackAlert(env, [
-      `Airtable validation warning encountered, attempting to proceed by setting the affected fields to undefined. Warning message: ${err.message}`,
-      ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : []),
-    ], { batchKey: 'airtable-validation' });
+    const { message, messages, batchGroup } = formatAirtableWarning(warning);
+
+    logger.warn(message);
+    await slackAlert(env, messages, { batchKey: 'airtable-validation', batchGroup });
   },
 });

libraries/db/src/index.ts

@@ -117,5 +117,8 @@ export type { DeprecationSafeTable } from './lib/db-core';
 
 export { AirtableTsError, ErrorType } from 'airtable-ts/dist/AirtableTsError';
 
+export { formatAirtableWarning } from './lib/formatAirtableWarning';
+export type { FormattedAirtableWarning } from './lib/formatAirtableWarning';
+
 // TODO: restrict what's exported
 export * from 'drizzle-orm';

libraries/db/src/lib/db-core.ts

@@ -224,6 +224,15 @@ export function getPgAirtableFromIds({ baseId, tableId }: { baseId: string; tabl
   return pgAirtableTableRegistry[key];
 }
 
+/**
+ * Airtable table ids are unique across bases, so a table id alone is enough to recover
+ * its base id. Useful when all we have is an error message from airtable-ts, which
+ * mentions the table id but not the base.
+ */
+export function getPgAirtableFromTableId(tableId: string) {
+  return Object.values(pgAirtableTableRegistry).find((table) => table.airtable.tableId === tableId);
+}
+
 export function pgAirtable<
   TTableName extends string,
   TColumnsMap extends Record<string, PgAirtableColumnInput>,

libraries/db/src/lib/formatAirtableWarning.test.ts

@@ -0,0 +1,105 @@
+import { describe, expect, test } from 'vitest';
+import { mapRecordFromAirtable } from 'airtable-ts/dist/mapping/recordMapper';
+import type { AirtableRecord } from 'airtable-ts/dist/types';
+import { formatAirtableWarning } from './formatAirtableWarning';
+import { selfServeCourseRegistrationTable } from '../schema';
+
+// A real schema entry, so the table id resolves to a base id and a column name.
+const TABLE_ID = 'tbla338CpAd0FF96g';
+const BASE_ID = 'appnJbsG1eWbAdEvf';
+const FIELD_ID = 'fldsS2lCVlk1WDSDw';
+const RECORD_ID = 'rec1ArmjQ8wUmBwDC';
+const RECORD_LINK = `<https://airtable.com/${BASE_ID}/${TABLE_ID}/${RECORD_ID}|${RECORD_ID}>`;
+
+describe('formatAirtableWarning', () => {
+  const typeMismatch = `Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: Failed to map field fullName (${FIELD_ID}) from Airtable: Cannot convert value from airtable type 'multipleLookupValues' to 'string | null', as the Airtable API provided a 'object'. Suggestion: Update the types in your table definition to compatible types for your Airtable base.`;
+
+  test('reduces a field-level warning to concise prose and batch metadata', () => {
+    const formatted = formatAirtableWarning(new Error(typeMismatch));
+
+    expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): can't map Airtable multipleLookupValues → string | null (got object). Value defaulted.`);
+    expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`);
+  });
+
+  test('links affected records into Airtable, using the base id from our schema', () => {
+    const formatted = formatAirtableWarning(new Error(typeMismatch));
+
+    // Only the Slack-bound copy carries mrkdwn; `message` stays plain for logs.
+    expect(formatted.messages[0]).toContain(RECORD_LINK);
+    expect(formatted.batchGroup.dedupeKeys).toEqual([RECORD_LINK]);
+  });
+
+  test('includes the stack trace as a reply message', () => {
+    const err = new Error(typeMismatch);
+    const formatted = formatAirtableWarning(err);
+
+    expect(formatted.messages).toHaveLength(2);
+    expect(formatted.messages[1]).toBe(`Stack:\n\`\`\`${err.stack}\`\`\``);
+  });
+
+  test('names the field from our schema when the warning carries no field prefix', () => {
+    // airtable-ts reports a field deleted from Airtable without its usual
+    // "Failed to map field ..." prefix, so the field name has to come from our schema.
+    const formatted = formatAirtableWarning(new Error(`Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: Field '${FIELD_ID}' does not exist in the table definition. This error should not happen in normal operation.`));
+
+    expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): Field '${FIELD_ID}' does not exist in the table definition. This error should not happen in normal operation. Value defaulted.`);
+    expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`);
+  });
+
+  test('formats a table we do not own, minus the record link', () => {
+    const formatted = formatAirtableWarning(new Error('Failed to map record from Airtable format for table \'someone_elses_table\' (tblXXXXXXXXXXXXXX) and record recYYYYYYYYYYYYYY: Failed to map field someField (fldZZZZZZZZZZZZZZ) from Airtable: Cannot convert value from airtable type \'formula\' to \'number\', as the Airtable API provided a \'string\'.'));
+
+    expect(formatted.message).toBe('Field `someField` on `someone_elses_table` (record recYYYYYYYYYYYYYY): can\'t map Airtable formula → number (got string). Value defaulted.');
+    // No base id available, so no link.
+    expect(formatted.batchGroup.dedupeKeys).toEqual(['recYYYYYYYYYYYYYY']);
+  });
+
+  test('keys record-level failures on the reason, with ids scrubbed', () => {
+    // Groups the same failure across records, without merging unrelated failures on one table.
+    const formatted = formatAirtableWarning(new Error(`Failed to map record from Airtable format for table 'self_serve_course_registration' (${TABLE_ID}) and record ${RECORD_ID}: something unexpected happened about ${RECORD_ID}`));
+
+    expect(formatted.message).toBe(`Record ${RECORD_ID} on \`self_serve_course_registration\`: something unexpected happened about ${RECORD_ID}.`);
+    expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/something unexpected happened about rec***`);
+  });
+
+  test('falls back to the raw message with no batch metadata when the shape is absent', () => {
+    const formatted = formatAirtableWarning(new Error('Some unrelated error with no airtable ids'));
+
+    expect(formatted.message).toBe('Some unrelated error with no airtable ids');
+    expect(formatted.batchGroup).toEqual({});
+  });
+
+  test('coerces non-Error warnings', () => {
+    const formatted = formatAirtableWarning('string warning');
+
+    expect(formatted.message).toBe('string warning');
+    expect(formatted.batchGroup).toEqual({});
+  });
+
+  test('parses what airtable-ts actually emits, not just our fixture strings', async () => {
+    // Guards the prose regexes against an airtable-ts wording change: without this, a
+    // version bump could silently degrade us to the ungrouped fallback with CI still green.
+    const record = {
+      id: RECORD_ID,
+      fields: { fullName: {} },
+      _table: { fields: [{ id: FIELD_ID, name: 'fullName', type: 'multipleLookupValues' }] },
+    } as unknown as AirtableRecord;
+
+    const warnings: unknown[] = [];
+    mapRecordFromAirtable(selfServeCourseRegistrationTable.airtable, record, {
+      readValidation: 'warning',
+      onWarning: (warning) => {
+        warnings.push(warning);
+      },
+    });
+    // onWarning is dispatched in a floating promise, so let the microtask queue drain.
+    await Promise.resolve();
+
+    const warning = warnings.find((w): w is Error => w instanceof Error && w.message.includes(FIELD_ID));
+    expect(warning).toBeDefined();
+
+    const formatted = formatAirtableWarning(warning);
+    expect(formatted.message).toBe(`Field \`fullName\` on \`self_serve_course_registration\` (record ${RECORD_ID}): can't map Airtable multipleLookupValues → string | null (got object). Value defaulted.`);
+    expect(formatted.batchGroup.signature).toBe(`${TABLE_ID}/${FIELD_ID}`);
+  });
+});

libraries/db/src/lib/formatAirtableWarning.ts

@@ -0,0 +1,98 @@
+import { type BatchGroup } from '@bluedot/utils';
+import { getPgAirtableFromTableId } from './db-core';
+
+/**
+ * airtable-ts surfaces read-validation failures as one deeply-nested error string, e.g.:
+ *   Failed to map record from Airtable format for table 'self_serve_course_registration'
+ *   (tbl…) and record rec…: Failed to map field fullName (fld…) from Airtable: Cannot
+ *   convert value from airtable type 'multipleLookupValues' to 'string | null', as the
+ *   Airtable API provided a 'object'. Suggestion: Update the types...
+ *
+ * We peel off the prefixes airtable-ts prepends as the error bubbles up, keeping the innermost
+ * reason plus the ids. Our own table definitions are consulted only for what the message can't
+ * tell us: the base id needed to link the offending record, and the column name in the cases
+ * where airtable-ts reports a field by id alone.
+ */
+export type FormattedAirtableWarning = {
+  /** Plain text, for logs. */
+  message: string;
+  /** Slack-flavoured (record ids become links), plus a stack-trace reply. Pass straight to slackAlert. */
+  messages: string[];
+  batchGroup: BatchGroup;
+};
+
+const TABLE_ID = /\btbl[A-Za-z0-9]{10,}/;
+const FIELD_ID = /\bfld[A-Za-z0-9]{10,}/;
+const RECORD_ID = /\brec[A-Za-z0-9]{10,}/;
+
+// Prefixes airtable-ts prepends as an error bubbles up, plus the suggestion it appends.
+// Stripping them leaves the innermost reason, which we can quote without repeating the
+// location we've already stated.
+const RECORD_PREFIX = /^Failed to map record from Airtable format for table '([^']*)' \(tbl[A-Za-z0-9]{10,}\) and record rec[A-Za-z0-9]{10,}: /;
+const FIELD_PREFIX = /^Failed to map field (.+?)(?: \(fld[A-Za-z0-9]{10,}\))? from Airtable: /;
+const SUGGESTION = / Suggestion: [\s\S]*$/;
+const TYPE_MISMATCH = /from airtable type '([^']*)' to '([^']*)', as the Airtable API provided a '([^']*)'/;
+
+const scrubIds = (message: string) => message.replace(/\b(tbl|fld|rec)[A-Za-z0-9]{10,}/g, '$1***');
+
+export const formatAirtableWarning = (warning: unknown): FormattedAirtableWarning => {
+  const err = warning instanceof Error ? warning : new Error(String(warning));
+  const raw = err.message;
+  const withStack = (message: string) => [message, ...(err.stack ? [`Stack:\n\`\`\`${err.stack}\`\`\``] : [])];
+
+  const tableId = TABLE_ID.exec(raw)?.[0];
+  const recordId = RECORD_ID.exec(raw)?.[0];
+  if (!tableId || !recordId) {
+    return { message: raw, messages: withStack(raw), batchGroup: {} };
+  }
+
+  // Peel the prefixes off one at a time, capturing what each one carries as we go.
+  const recordPrefix = RECORD_PREFIX.exec(raw);
+  const afterRecord = recordPrefix ? raw.slice(recordPrefix[0].length) : raw;
+  const fieldPrefix = FIELD_PREFIX.exec(afterRecord);
+  const afterField = fieldPrefix ? afterRecord.slice(fieldPrefix[0].length) : afterRecord;
+  const innermost = afterField.replace(SUGGESTION, '');
+
+  const fieldId = FIELD_ID.exec(raw)?.[0];
+  const table = getPgAirtableFromTableId(tableId);
+  const columnName = fieldId
+    ? Array.from(table?.airtableFieldMap ?? []).find(([, airtableId]) => airtableId === fieldId)?.[0]
+    : undefined;
+
+  const tableName = recordPrefix?.[1] ?? tableId;
+  // airtable-ts usually names the field in its prefix. When it doesn't — e.g. a field deleted
+  // from Airtable, reported by id — recover the column name rather than print a bare id.
+  const fieldName = fieldPrefix?.[1] ?? columnName ?? fieldId;
+
+  const [, airtableType, expectedType, providedType] = TYPE_MISMATCH.exec(innermost) ?? [];
+  const reason = airtableType && expectedType
+    ? `can't map Airtable ${airtableType} → ${expectedType}${providedType ? ` (got ${providedType})` : ''}`
+    : innermost;
+
+  const buildMessage = (recordRef: string) => {
+    const location = fieldId
+      ? `Field \`${fieldName}\` on \`${tableName}\` (record ${recordRef})`
+      : `Record ${recordRef} on \`${tableName}\``;
+    // airtable-ts substitutes a type-appropriate default (null, '', 0, false or []), so
+    // don't promise a specific value here.
+    const outcome = fieldId ? ' Value defaulted.' : '';
+    return `${location}: ${reason.replace(/\.$/, '')}.${outcome}`;
+  };
+
+  const baseId = table?.airtable.baseId;
+  const recordLink = baseId
+    ? `<https://airtable.com/${baseId}/${tableId}/${recordId}|${recordId}>`
+    : recordId;
+
+  return {
+    message: buildMessage(recordId),
+    messages: withStack(buildMessage(recordLink)),
+    batchGroup: {
+      // Group by table/field, not by record, so the same failure across many records
+      // collapses into one batched alert. With no field to key on, fall back to the
+      // reason with ids scrubbed, so unrelated failures on one table stay separate.
+      signature: `${tableId}/${fieldId ?? scrubIds(innermost)}`,
+      dedupeKeys: [recordLink],
+    },
+  };
+};

libraries/utils/src/index.ts

@@ -1,5 +1,5 @@
 export { validateEnv } from './validateEnv';
-export { slackAlert } from './slackNotifications';
+export { slackAlert, type BatchGroup } from './slackNotifications';
 export { chunk } from './array';
 
 export {