Fix search index scrape crashing on records with empty titles (#59656)

Author: Ebonsignori

content/authentication/connecting-to-github-with-ssh/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent.md

@@ -162,6 +162,7 @@ Before adding a new SSH key to the ssh-agent to manage your keys, you should hav
    ```powershell
    ssh-add c:/Users/YOU/.ssh/id_ed25519
    ```
+
 {% data reusables.ssh.add-public-key-to-github %}
 
 > ### Troubleshooting SSH agent conflicts in Windows

src/search/scripts/scrape/lib/build-records-from-api.ts

@@ -416,6 +416,18 @@ export default async function buildRecordsFromApi(
       }
 
       if (result.record) {
+        // Validate required fields before adding to records
+        if (!result.record.title) {
+          failedPages.push({
+            url: permalink.href,
+            relativePath: permalink.relativePath,
+            error: 'Record has empty title',
+            errorType: 'Validation Error',
+          })
+          if (!noMarkers) process.stdout.write(chalk.red('✗'))
+          return null
+        }
+
         // Apply popularity
         const pathArticle = permalink.relativePath.replace('/index.md', '').replace('.md', '')
         let popularity = (hasPopularPages && popularPages[pathArticle]) || 0.0

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

@@ -1,7 +1,7 @@
 import path from 'path'
 import fsSync from 'fs'
 import fs from 'fs/promises'
-import assert from 'assert'
+import chalk from 'chalk'
 import { isArray, isString } from 'lodash-es'
 
 import type { Record } from '@/search/scripts/scrape/types'
@@ -11,9 +11,9 @@ export async function writeIndexRecords(
   records: Record[],
   outDirectory: string,
 ): Promise<string> {
-  validateRecords(name, records)
+  const validRecords = validateRecords(name, records)
 
-  const recordsObject = Object.fromEntries(records.map((record) => [record.objectID, record]))
+  const recordsObject = Object.fromEntries(validRecords.map((record) => [record.objectID, record]))
   const content = JSON.stringify(recordsObject, undefined, 0)
 
   // If the outDirectory doesn't exist, create it
@@ -27,30 +27,52 @@ export async function writeIndexRecords(
   return filePath
 }
 
-function validateRecords(name: string, records: Record[]): true {
-  assert(isString(name) && name.length, '`name` is required')
-  assert(isArray(records) && records.length, '`records` must be a non-empty array')
+function validateRecords(name: string, records: Record[]): Record[] {
+  if (!isString(name) || !name.length) {
+    throw new Error('`name` is required')
+  }
+  if (!isArray(records) || !records.length) {
+    throw new Error('`records` must be a non-empty array')
+  }
 
-  // each ID is unique
+  // each ID is unique — deduplicate rather than crash
   const objectIDs = records.map((record) => record.objectID)
   const dupes = countArrayValues(objectIDs)
     .filter(({ count }) => count > 1)
     .map(({ value }) => value)
-  assert(!dupes.length, `every objectID must be unique. dupes: ${dupes.join('; ')}`)
+  if (dupes.length) {
+    console.warn(
+      chalk.yellow(`⚠ Duplicate objectIDs found and will be deduplicated: ${dupes.join('; ')}`),
+    )
+  }
+
+  const seen = new Set<string>()
+  const validRecords: Record[] = []
 
   for (const record of records) {
-    assert(
-      isString(record.objectID) && record.objectID.length,
-      `objectID must be a string. received: ${record.objectID}, ${JSON.stringify(record)}`,
-    )
+    if (!isString(record.objectID) || !record.objectID.length) {
+      console.warn(
+        chalk.yellow(
+          `⚠ Skipping record with invalid objectID: ${JSON.stringify({ objectID: record.objectID, title: record.title })}`,
+        ),
+      )
+      continue
+    }
 
-    assert(
-      isString(record.title) && record.title.length,
-      `title must be a string. received: ${record.title}, ${JSON.stringify(record)}`,
-    )
+    if (!isString(record.title) || !record.title.length) {
+      console.warn(chalk.yellow(`⚠ Skipping record with empty title: ${record.objectID}`))
+      continue
+    }
+
+    if (seen.has(record.objectID)) {
+      continue
+    }
+    seen.add(record.objectID)
+
+    validRecords.push(record)
   }
 
-  return true
+  return validRecords
 }
 
 function countArrayValues(arr: string[]) {

src/search/scripts/scrape/tests/build-records-from-api.ts

@@ -0,0 +1,131 @@
+import { describe, test, expect } from 'vitest'
+
+import {
+  articleApiResponseToRecord,
+  extractFromMarkdown,
+  type ArticleApiResponse,
+} from '@/search/scripts/scrape/lib/build-records-from-api'
+
+describe('articleApiResponseToRecord', () => {
+  function makeApiResponse(overrides: Partial<ArticleApiResponse> = {}): ArticleApiResponse {
+    return {
+      meta: {
+        title: 'Test Page',
+        intro: 'An intro paragraph.',
+        product: 'test-product',
+        breadcrumbs: [
+          { href: '/en', title: 'Home' },
+          { href: '/en/test', title: 'Test' },
+          { href: '/en/test/page', title: 'Test Page' },
+        ],
+        ...overrides.meta,
+      },
+      body: overrides.body ?? '## Getting started\n\nSome content here.',
+    }
+  }
+
+  test('converts API response to a search record', () => {
+    const record = articleApiResponseToRecord('/en/test/page', makeApiResponse())
+
+    expect(record.objectID).toBe('/en/test/page')
+    expect(record.title).toBe('Test Page')
+    expect(record.intro).toBe('An intro paragraph.')
+    expect(record.breadcrumbs).toBe('Home / Test')
+    expect(record.toplevel).toBe('Home')
+    expect(record.headings).toContain('Getting started')
+    expect(record.content).toContain('Some content here.')
+  })
+
+  test('returns empty title when API response has empty title', () => {
+    const record = articleApiResponseToRecord(
+      '/en/test/page',
+      makeApiResponse({ meta: { title: '', intro: '', product: 'test' } }),
+    )
+
+    expect(record.title).toBe('')
+  })
+
+  test('excludes navigational headings', () => {
+    const record = articleApiResponseToRecord(
+      '/en/test/page',
+      makeApiResponse({
+        body: '## In this article\n\n## Real heading\n\n## Further reading\n\nContent.',
+      }),
+    )
+
+    expect(record.headings).toBe('Real heading')
+  })
+
+  test('handles missing breadcrumbs', () => {
+    const response = makeApiResponse()
+    response.meta.breadcrumbs = undefined
+    const record = articleApiResponseToRecord('/en/test/page', response)
+
+    expect(record.breadcrumbs).toBe('')
+    expect(record.toplevel).toBe('')
+  })
+
+  test('prepends intro to content when not already present', () => {
+    const record = articleApiResponseToRecord(
+      '/en/test/page',
+      makeApiResponse({
+        meta: { title: 'T', intro: 'Unique intro.', product: 'p' },
+        body: 'Body text only.',
+      }),
+    )
+
+    expect(record.content).toMatch(/^Unique intro\.\nBody text only\.$/)
+  })
+
+  test('does not duplicate intro when body already contains it', () => {
+    const record = articleApiResponseToRecord(
+      '/en/test/page',
+      makeApiResponse({
+        meta: { title: 'T', intro: 'Body text', product: 'p' },
+        body: 'Body text only.',
+      }),
+    )
+
+    const occurrences = record.content.split('Body text').length - 1
+    expect(occurrences).toBe(1)
+  })
+})
+
+describe('extractFromMarkdown', () => {
+  test('extracts h2 headings and plain text content', () => {
+    const md = '## Heading One\n\nParagraph text.\n\n## Heading Two\n\nMore text.'
+    const result = extractFromMarkdown(md)
+
+    expect(result.headings).toBe('Heading One\nHeading Two')
+    expect(result.content).toContain('Paragraph text.')
+    expect(result.content).toContain('More text.')
+  })
+
+  test('skips ignored heading slugs', () => {
+    const md = '## In this article\n\n## Prerequisites\n\n## Real heading'
+    const result = extractFromMarkdown(md)
+
+    expect(result.headings).toBe('Real heading')
+  })
+
+  test('skips ignored heading texts in non-English', () => {
+    const md = '## 本文内容\n\n## 先决条件\n\n## 真正的标题'
+    const result = extractFromMarkdown(md)
+
+    expect(result.headings).toBe('真正的标题')
+  })
+
+  test('ignores h1 and h3+ headings', () => {
+    const md = '# H1 Title\n\n## H2 Heading\n\n### H3 Subheading'
+    const result = extractFromMarkdown(md)
+
+    expect(result.headings).toBe('H2 Heading')
+  })
+
+  test('preserves code block text in content', () => {
+    const md = '## Setup\n\n```bash\nssh_url=git@github.com\n```'
+    const result = extractFromMarkdown(md)
+
+    expect(result.content).toContain('ssh_url=git@github.com')
+  })
+})

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

@@ -0,0 +1,123 @@
+import { describe, test, expect, vi, beforeEach } from 'vitest'
+import fs from 'fs/promises'
+import fsSync from 'fs'
+
+import { writeIndexRecords } from '@/search/scripts/scrape/lib/search-index-records'
+import type { Record } from '@/search/scripts/scrape/types'
+
+vi.mock('fs/promises')
+vi.mock('fs')
+
+function makeRecord(overrides: Partial<Record> = {}): Record {
+  return {
+    objectID: '/en/test-page',
+    breadcrumbs: 'Test',
+    title: 'Test Page',
+    headings: '',
+    content: 'Some content',
+    intro: '',
+    toplevel: 'Test',
+    ...overrides,
+  }
+}
+
+describe('writeIndexRecords', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    vi.mocked(fsSync.existsSync).mockReturnValue(true)
+    vi.mocked(fs.writeFile).mockResolvedValue()
+  })
+
+  test('writes valid records to JSON file', async () => {
+    const records = [makeRecord(), makeRecord({ objectID: '/en/other-page', title: 'Other Page' })]
+
+    const result = await writeIndexRecords('test-index', records, '/tmp/out')
+
+    expect(result).toBe('/tmp/out/test-index-records.json')
+    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(() => {})
+    const records = [makeRecord(), makeRecord({ objectID: '/en/bad-page', title: '' })]
+
+    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()
+  })
+
+  test('filters out records with missing objectID', async () => {
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    const records = [makeRecord(), makeRecord({ objectID: '', title: 'No ID' })]
+
+    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()
+  })
+
+  test('deduplicates records with the same objectID', async () => {
+    const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    const records = [
+      makeRecord({ title: 'First' }),
+      makeRecord({ title: 'Duplicate' }), // same objectID
+    ]
+
+    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(parsed['/en/test-page'].title).toBe('First')
+    expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Duplicate objectIDs'))
+    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',
+      }),
+    ]
+
+    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()
+  })
+
+  test('throws when name is empty', async () => {
+    await expect(writeIndexRecords('', [makeRecord()], '/tmp/out')).rejects.toThrow(
+      '`name` is required',
+    )
+  })
+
+  test('throws when records array is empty', async () => {
+    await expect(writeIndexRecords('test-index', [], '/tmp/out')).rejects.toThrow(
+      '`records` must be a non-empty array',
+    )
+  })
+
+  test('creates output directory if it does not exist', async () => {
+    vi.mocked(fsSync.existsSync).mockReturnValue(false)
+    vi.mocked(fs.mkdir).mockResolvedValue(undefined)
+
+    await writeIndexRecords('test-index', [makeRecord()], '/tmp/new-dir')
+
+    expect(fs.mkdir).toHaveBeenCalledWith('/tmp/new-dir', { recursive: true })
+  })
+})