Skip to content

Commit 3de5791

Browse files
EbonsignoriCopilot
andauthored
Migrate src/content-render console statements to structured logger (#60984)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 667ea50 commit 3de5791

11 files changed

Lines changed: 61 additions & 45 deletions

File tree

eslint.config.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ export default [
177177
'src/article-api/**/*.{ts,js}',
178178
'src/audit-logs/**/*.{ts,js}',
179179
'src/color-schemes/**/*.{ts,js}',
180-
'src/content-render/**/*.{ts,js}',
181180
'src/data-directory/**/*.{ts,js}',
182181
'src/dev-toc/**/*.{ts,js}',
183182
'src/events/**/*.{ts,js}',

src/content-render/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ import { renderLiquid } from './liquid/index'
22
import { renderMarkdown, renderUnified } from './unified/index'
33
import { engine } from './liquid/engine'
44
import type { Context } from '@/types'
5+
import { createLogger } from '@/observability/logger'
6+
7+
const logger = createLogger(import.meta.url)
58

69
interface RenderOptions {
710
cache?: boolean | ((template: string, context: Context) => string)
@@ -53,7 +56,7 @@ export async function renderContent(
5356
return html
5457
} catch (error) {
5558
if (options.filename) {
56-
console.error(`renderContent failed on file: ${options.filename}`)
59+
logger.error('renderContent failed on file', { filename: options.filename })
5760
}
5861
throw error
5962
}

src/content-render/liquid/data.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import type { TagToken, Liquid, Template } from 'liquidjs'
33

44
import { THROW_ON_EMPTY, DataReferenceError } from './error-handling'
55
import { getDataByLanguage } from '@/data-directory/lib/get-data'
6+
import { createLogger } from '@/observability/logger'
7+
8+
const logger = createLogger(import.meta.url)
69

710
const Syntax = /([a-z0-9/\\_.\-[\]]+)/i
811
const SyntaxHelp = "Syntax Error in 'data' - Valid syntax: data [path]"
@@ -42,7 +45,7 @@ export default {
4245
if (THROW_ON_EMPTY) {
4346
throw new DataReferenceError(message)
4447
}
45-
console.warn(message)
48+
logger.warn(message)
4649
}
4750
return
4851
}

src/content-render/liquid/ifversion.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import versionSatisfiesRange from '@/versions/lib/version-satisfies-range'
1414
import supportedOperators, {
1515
type IfversionSupportedOperator,
1616
} from './ifversion-supported-operators'
17+
import { createLogger } from '@/observability/logger'
18+
19+
const logger = createLogger(import.meta.url)
1720

1821
interface Branch {
1922
cond: string
@@ -174,17 +177,7 @@ export default class Ifversion extends Tag {
174177
}
175178

176179
if (!this.currentVersionObj) {
177-
console.warn(
178-
`
179-
If this happens, it means the context prepared for rendering Liquid
180-
did not supply an object called 'currentVersionObj'.
181-
To fix the error, find the code that prepares the context before
182-
calling 'liquid.parseAndRender' and make sure there's an object
183-
called 'currentVersionObj' included there.
184-
`
185-
.replace(/\n\s+/g, ' ')
186-
.trim(),
187-
)
180+
logger.warn('Context missing currentVersionObj for Liquid rendering')
188181
throw new Error('currentVersionObj not found in environment context.')
189182
}
190183

@@ -222,7 +215,7 @@ export default class Ifversion extends Tag {
222215

223216
handleVersionNames(resolvedBranchCond: string): string {
224217
if (!this.currentVersionObj) {
225-
console.warn('currentVersionObj not found in ifversion context.')
218+
logger.warn('currentVersionObj not found in ifversion context')
226219
return resolvedBranchCond
227220
}
228221

src/content-render/liquid/indented-data-reference.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import assert from 'assert'
33
import { type TagToken, type Liquid } from 'liquidjs'
44
import { THROW_ON_EMPTY, IndentedDataReferenceError } from './error-handling'
55
import { getDataByLanguage } from '@/data-directory/lib/get-data'
6+
import { createLogger } from '@/observability/logger'
7+
8+
const logger = createLogger(import.meta.url)
69

710
interface LiquidScope {
811
environments: {
@@ -55,7 +58,7 @@ const IndentedDataReference = {
5558
if (THROW_ON_EMPTY) {
5659
throw new IndentedDataReferenceError(message)
5760
}
58-
console.warn(message)
61+
logger.warn(message)
5962
}
6063
return
6164
}

src/content-render/tests/copilot-code-blocks.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
import { describe, it, expect, vi } from 'vitest'
22
import { renderContent } from '@/content-render/index'
33

4+
const { mockWarn } = vi.hoisted(() => ({ mockWarn: vi.fn() }))
5+
vi.mock('@/observability/logger', () => ({
6+
createLogger: () => ({
7+
info: vi.fn(),
8+
warn: mockWarn,
9+
error: vi.fn(),
10+
debug: vi.fn(),
11+
}),
12+
}))
13+
414
describe('code-header plugin', () => {
515
describe('copilot language code blocks', () => {
616
it('should render basic copilot code block without header (no copy meta)', async () => {
@@ -126,18 +136,17 @@ Improve the variable names in this function
126136

127137
describe('edge cases', () => {
128138
it('should handle missing reference gracefully and fall back to current code only', async () => {
129-
// Mock console.warn to capture warning
130-
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
139+
mockWarn.mockClear()
131140

132141
const markdown =
133142
'```copilot copy prompt ref=nonexistent-id\nImprove the variable names in this function\n```'
134143

135144
const html = await renderContent(markdown)
136145

137-
// Should warn about missing reference
138-
expect(consoleWarnSpy).toHaveBeenCalledWith(
139-
expect.stringContaining("Can't find referenced code block with id=nonexistent-id"),
140-
)
146+
// Should warn about missing reference via structured logger
147+
expect(mockWarn).toHaveBeenCalledWith('Cannot find referenced code block', {
148+
ref: 'nonexistent-id',
149+
})
141150

142151
// Should still render with prompt button using current code only
143152
expect(html).toContain('https://github.com/copilot?prompt=')
@@ -149,8 +158,7 @@ Improve the variable names in this function
149158
// Should not crash or fail
150159
expect(html).toContain('code-example')
151160

152-
// Restore console.warn
153-
consoleWarnSpy.mockRestore()
161+
mockWarn.mockClear()
154162
})
155163

156164
it('should not process annotated code blocks', async () => {

src/content-render/unified/alerts.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import { visit } from 'unist-util-visit'
66
import { h } from 'hastscript'
77
import octicons from '@primer/octicons'
88
import type { Element, Root, ElementContent } from 'hast'
9+
import { createLogger } from '@/observability/logger'
10+
11+
const logger = createLogger(import.meta.url)
912

1013
interface AlertType {
1114
icon: string
@@ -33,9 +36,7 @@ export default function alerts({ alertTitles = {} }: { alertTitles?: Record<stri
3336
return
3437
const key = getAlertKey(el)
3538
if (!(key in alertTypes)) {
36-
console.warn(
37-
`Alert key '${key}' should be all uppercase (change it to '${key.toUpperCase()}')`,
38-
)
39+
logger.warn('Alert key should be all uppercase', { key, expected: key.toUpperCase() })
3940
}
4041
const alertType = alertTypes[getAlertKey(el).toUpperCase()]
4142
el.tagName = 'div'

src/content-render/unified/annotate.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ import { toHast } from 'mdast-util-to-hast'
3838
import type { Root } from 'mdast'
3939
import { header } from './code-header'
4040
import findPage from '@/frame/lib/find-page'
41+
import { createLogger } from '@/observability/logger'
42+
43+
const logger = createLogger(import.meta.url)
4144

4245
interface LanguageConfig {
4346
comment: 'number' | 'slash' | 'xml' | 'percent' | 'hyphen'
@@ -280,10 +283,10 @@ function processAutotitleInMdast(mdast: Root, context: any): void {
280283
child.value = page.rawTitle || 'AUTOTITLE'
281284
} catch (error) {
282285
// Keep AUTOTITLE if we can't get the title
283-
console.warn(
284-
`Could not resolve AUTOTITLE for ${node.url}:`,
285-
error instanceof Error ? error.message : String(error),
286-
)
286+
logger.warn('Could not resolve AUTOTITLE', {
287+
url: node.url,
288+
error: error instanceof Error ? error.message : String(error),
289+
})
287290
}
288291
}
289292
}

src/content-render/unified/copilot-prompt.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ import octicons from '@primer/octicons'
88
import { parse } from 'parse5'
99
import { fromParse5 } from 'hast-util-from-parse5'
1010
import { getPreMeta } from './code-header'
11+
import { createLogger } from '@/observability/logger'
1112
import { generatePromptId } from '../lib/prompt-id'
1213
import type { Element, Root } from 'hast'
1314

15+
const logger = createLogger(import.meta.url)
16+
1417
export function getPrompt(
1518
node: Element,
1619
tree: Root,
@@ -55,7 +58,7 @@ function buildPromptData(
5558
// If the 'ref=<id>' meta is found, find a matching code block to include as context in the prompt link.
5659
const matchingCodeEl = findMatchingCode(ref as string, tree)
5760
if (!matchingCodeEl) {
58-
console.warn(`Can't find referenced code block with id=${ref}`)
61+
logger.warn('Cannot find referenced code block', { ref })
5962
return promptOnly(code)
6063
}
6164
// AST structure: element -> code -> text node with value property

src/content-render/unified/rewrite-asset-urls.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import fs from 'fs'
22
import type { Element, Node } from 'hast'
33
import { visit } from 'unist-util-visit'
4+
import { createLogger } from '@/observability/logger'
5+
6+
const logger = createLogger(import.meta.url)
47

58
// Process-level cache for stat results — file sizes don't change between deploys.
69
const statCache = new Map<string, number | null>()
@@ -59,9 +62,6 @@ function getNewSrc(node: Element): string | undefined {
5962
return split.join('/')
6063
} catch {
6164
statCache.set(filePath, null)
62-
console.warn(
63-
`Failed to get a hash for ${src} ` +
64-
'(This is mostly harmless and can happen with outdated translations).',
65-
)
65+
logger.warn('Failed to get a hash for asset URL', { src })
6666
}
6767
}

0 commit comments

Comments
 (0)