Report invalid search records as failures instead of crashing (#59776)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: heiskr

src/search/scripts/scrape/lib/scrape-into-index-json.ts

@@ -79,8 +79,42 @@ export default async function scrapeIntoIndexJson({
         })
       }
 
-      const fileWritten = await writeIndexRecords(indexName, records, outDirectory)
-      console.log(`wrote records to ${fileWritten}`)
+      const { filePath: fileWritten, skippedRecords } = await writeIndexRecords(
+        indexName,
+        records,
+        outDirectory,
+      )
+      if (fileWritten) {
+        console.log(`wrote records to ${fileWritten}`)
+      } else {
+        totalFailedPages += 1
+        allFailures.push({
+          indexName,
+          languageCode,
+          indexVersion,
+          failures: [
+            {
+              error: `No valid records to write for ${indexName}`,
+              errorType: 'no-valid-records',
+            },
+          ],
+        })
+      }
+
+      if (skippedRecords.length > 0) {
+        const skippedFailures = skippedRecords.map(({ objectID, reason }) => ({
+          url: objectID,
+          error: `Record skipped: ${reason}`,
+          errorType: 'invalid-record',
+        }))
+        totalFailedPages += skippedRecords.length
+        allFailures.push({
+          indexName,
+          languageCode,
+          indexVersion,
+          failures: skippedFailures,
+        })
+      }
     }
   }
   const t1 = new Date()

src/search/scripts/scrape/lib/search-index-records.ts

@@ -6,12 +6,26 @@ import { isArray, isString } from 'lodash-es'
 
 import type { Record } from '@/search/scripts/scrape/types'
 
+export interface SkippedRecord {
+  objectID: string
+  reason: string
+}
+
+export interface WriteIndexRecordsResult {
+  filePath: string
+  skippedRecords: SkippedRecord[]
+}
+
 export async function writeIndexRecords(
   name: string,
   records: Record[],
   outDirectory: string,
-): Promise<string> {
-  const validRecords = validateRecords(name, records)
+): Promise<WriteIndexRecordsResult> {
+  const { validRecords, skippedRecords } = validateRecords(name, records)
+
+  if (validRecords.length === 0) {
+    return { filePath: '', skippedRecords }
+  }
 
   const recordsObject = Object.fromEntries(validRecords.map((record) => [record.objectID, record]))
   const content = JSON.stringify(recordsObject, undefined, 0)
@@ -24,15 +38,26 @@ export async function writeIndexRecords(
   const filePath = path.join(outDirectory, `${name}-records.json`)
   await fs.writeFile(filePath, content, { flag: 'w' })
 
-  return filePath
+  return { filePath, skippedRecords }
+}
+
+interface ValidateResult {
+  validRecords: Record[]
+  skippedRecords: SkippedRecord[]
 }
 
-function validateRecords(name: string, records: Record[]): Record[] {
+function validateRecords(name: string, records: Record[]): ValidateResult {
   if (!isString(name) || !name.length) {
-    throw new Error('`name` is required')
+    return {
+      validRecords: [],
+      skippedRecords: [{ objectID: '(unknown)', reason: 'name is required' }],
+    }
   }
   if (!isArray(records) || !records.length) {
-    throw new Error('`records` must be a non-empty array')
+    return {
+      validRecords: [],
+      skippedRecords: [{ objectID: '(unknown)', reason: 'records array is empty' }],
+    }
   }
 
   // each ID is unique — deduplicate rather than crash
@@ -48,19 +73,16 @@ function validateRecords(name: string, records: Record[]): Record[] {
 
   const seen = new Set<string>()
   const validRecords: Record[] = []
+  const skippedRecords: SkippedRecord[] = []
 
   for (const record of records) {
     if (!isString(record.objectID) || !record.objectID.length) {
-      console.warn(
-        chalk.yellow(
-          `⚠ Skipping record with invalid objectID: ${JSON.stringify({ objectID: record.objectID, title: record.title })}`,
-        ),
-      )
+      skippedRecords.push({ objectID: '(unknown)', reason: 'invalid objectID' })
       continue
     }
 
     if (!isString(record.title) || !record.title.length) {
-      console.warn(chalk.yellow(`⚠ Skipping record with empty title: ${record.objectID}`))
+      skippedRecords.push({ objectID: record.objectID, reason: 'empty title' })
       continue
     }
 
@@ -72,7 +94,7 @@ function validateRecords(name: string, records: Record[]): Record[] {
     validRecords.push(record)
   }
 
-  return validRecords
+  return { validRecords, skippedRecords }
 }
 
 function countArrayValues(arr: string[]) {

src/search/scripts/scrape/tests/search-index-records.ts

@@ -33,37 +33,34 @@ describe('writeIndexRecords', () => {
 
     const result = await writeIndexRecords('test-index', records, '/tmp/out')
 
-    expect(result).toBe('/tmp/out/test-index-records.json')
+    expect(result.filePath).toBe('/tmp/out/test-index-records.json')
+    expect(result.skippedRecords).toEqual([])
     expect(fs.writeFile).toHaveBeenCalledOnce()
     const writtenJson = vi.mocked(fs.writeFile).mock.calls[0][1] as string
     const parsed = JSON.parse(writtenJson)
     expect(Object.keys(parsed)).toEqual(['/en/test-page', '/en/other-page'])
   })
 
-  test('filters out records with empty titles', async () => {
-    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+  test('filters out records with empty titles and returns them as skipped', async () => {
     const records = [makeRecord(), makeRecord({ objectID: '/en/bad-page', title: '' })]
 
-    await writeIndexRecords('test-index', records, '/tmp/out')
+    const result = await writeIndexRecords('test-index', records, '/tmp/out')
 
     const writtenJson = vi.mocked(fs.writeFile).mock.calls[0][1] as string
     const parsed = JSON.parse(writtenJson)
     expect(Object.keys(parsed)).toEqual(['/en/test-page'])
-    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('empty title'))
-    warnSpy.mockRestore()
+    expect(result.skippedRecords).toEqual([{ objectID: '/en/bad-page', reason: 'empty title' }])
   })
 
-  test('filters out records with missing objectID', async () => {
-    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+  test('filters out records with missing objectID and returns them as skipped', async () => {
     const records = [makeRecord(), makeRecord({ objectID: '', title: 'No ID' })]
 
-    await writeIndexRecords('test-index', records, '/tmp/out')
+    const result = await writeIndexRecords('test-index', records, '/tmp/out')
 
     const writtenJson = vi.mocked(fs.writeFile).mock.calls[0][1] as string
     const parsed = JSON.parse(writtenJson)
     expect(Object.keys(parsed)).toEqual(['/en/test-page'])
-    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('invalid objectID'))
-    warnSpy.mockRestore()
+    expect(result.skippedRecords).toEqual([{ objectID: '(unknown)', reason: 'invalid objectID' }])
   })
 
   test('deduplicates records with the same objectID', async () => {
@@ -82,34 +79,22 @@ describe('writeIndexRecords', () => {
     warnSpy.mockRestore()
   })
 
-  test('does not log full record content for invalid objectID', async () => {
-    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
-    const records = [
-      makeRecord(),
-      makeRecord({
-        objectID: '',
-        title: 'No ID',
-        content: 'A very long content string that should not appear in logs',
-      }),
-    ]
+  test('returns empty filePath and skipped record when name is empty', async () => {
+    const result = await writeIndexRecords('', [makeRecord()], '/tmp/out')
 
-    await writeIndexRecords('test-index', records, '/tmp/out')
-
-    const warnMessage = warnSpy.mock.calls[0][0] as string
-    expect(warnMessage).not.toContain('very long content string')
-    warnSpy.mockRestore()
+    expect(result.filePath).toBe('')
+    expect(result.skippedRecords).toEqual([{ objectID: '(unknown)', reason: 'name is required' }])
+    expect(fs.writeFile).not.toHaveBeenCalled()
   })
 
-  test('throws when name is empty', async () => {
-    await expect(writeIndexRecords('', [makeRecord()], '/tmp/out')).rejects.toThrow(
-      '`name` is required',
-    )
-  })
+  test('returns empty filePath and skipped record when records array is empty', async () => {
+    const result = await writeIndexRecords('test-index', [], '/tmp/out')
 
-  test('throws when records array is empty', async () => {
-    await expect(writeIndexRecords('test-index', [], '/tmp/out')).rejects.toThrow(
-      '`records` must be a non-empty array',
-    )
+    expect(result.filePath).toBe('')
+    expect(result.skippedRecords).toEqual([
+      { objectID: '(unknown)', reason: 'records array is empty' },
+    ])
+    expect(fs.writeFile).not.toHaveBeenCalled()
   })
 
   test('creates output directory if it does not exist', async () => {