Skip to content

Commit f0ce97d

Browse files
authored
Migrate src/frame console statements to structured logger (#61072)
1 parent 5488fd5 commit f0ce97d

9 files changed

Lines changed: 40 additions & 23 deletions

File tree

eslint.config.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ export default [
181181
'src/dev-toc/**/*.{ts,js}',
182182
'src/events/**/*.{ts,js}',
183183
'src/fixtures/**/*.{ts,js}',
184-
'src/frame/**/*.{ts,js}',
185184
'src/github-apps/**/*.{ts,js}',
186185
'src/journeys/**/*.{ts,js}',
187186
'src/languages/**/*.{ts,js}',
Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
import fs from 'fs'
22
import semver from 'semver'
33

4+
import { createLogger } from '@/observability/logger'
5+
const logger = createLogger(import.meta.url)
6+
47
export function checkNodeVersion() {
58
const packageFile = JSON.parse(fs.readFileSync('package.json', 'utf-8'))
69
const { engines } = packageFile
710

811
if (!semver.satisfies(process.version, engines.node)) {
9-
console.error(
10-
`\n\nYou're using Node.js ${process.version.replace(/^v/, '')} but this project requires ${
12+
logger.error(
13+
`You're using Node.js ${process.version.replace(/^v/, '')} but this project requires ${
1114
engines.node
1215
}`,
16+
{ currentVersion: process.version, requiredVersion: engines.node },
1317
)
14-
console.error('Visit nodejs.org to download an installer that meets these requirements.\n\n')
18+
logger.error('Visit nodejs.org to download an installer that meets these requirements.')
1519
process.exit(1)
1620
}
1721
}

src/frame/lib/create-tree.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import fs from 'fs/promises'
33

44
import PageClass from './page'
55
import type { UnversionedTree, Page } from '@/types'
6+
import { createLogger } from '@/observability/logger'
7+
const logger = createLogger(import.meta.url)
68

79
const isProduction = process.env.NODE_ENV === 'production'
810

@@ -50,7 +52,7 @@ export default async function createTree(
5052
originalPath === 'content/early-access' ||
5153
originalPath.startsWith('content/early-access/')
5254
) {
53-
console.warn(`Warning: ${msg}`)
55+
logger.warn(msg, { path: originalPath })
5456
return
5557
}
5658
throw new Error(msg)

src/frame/lib/page.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import { deprecated, supported } from '@/versions/lib/enterprise-server-releases
2020
import { allPlatforms } from '@/tools/lib/all-platforms'
2121
import type { Context, FrontmatterVersions, FeaturedLinksExpanded } from '@/types'
2222
import type { Product } from '@/products/lib/all-products'
23+
import { createLogger } from '@/observability/logger'
24+
const logger = createLogger(import.meta.url)
2325

2426
const isProduction = process.env.NODE_ENV === 'production'
2527

@@ -192,17 +194,18 @@ class Page {
192194
} as PageReadResult
193195
} catch (err) {
194196
if (err instanceof Error && (err as NodeJS.ErrnoException).code === 'ENOENT') return false
195-
console.error(err)
197+
logger.error('Failed to read page file', { error: err, fullPath })
196198
return false
197199
}
198200
}
199201

200202
constructor(opts: PageReadResult) {
201203
if (opts.frontmatterErrors && opts.frontmatterErrors.length) {
202-
console.error(
203-
`${opts.frontmatterErrors.length} frontmatter errors trying to load ${opts.fullPath}:`,
204-
)
205-
console.error(opts.frontmatterErrors)
204+
logger.error('Frontmatter errors loading page', {
205+
errorCount: opts.frontmatterErrors.length,
206+
fullPath: opts.fullPath,
207+
frontmatterErrors: opts.frontmatterErrors,
208+
})
206209
throw new FrontmatterErrorsError(
207210
`${opts.frontmatterErrors.length} frontmatter errors in ${opts.fullPath}`,
208211
opts.frontmatterErrors,

src/frame/lib/read-json-file.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import fs from 'fs'
22
import { brotliDecompressSync } from 'zlib'
33

4+
import { createLogger } from '@/observability/logger'
5+
const logger = createLogger(import.meta.url)
6+
47
export default function readJsonFile(xpath: string): unknown {
58
return JSON.parse(fs.readFileSync(xpath, 'utf8'))
69
}
@@ -68,10 +71,9 @@ export function readCompressedJsonFileFallbackLazily(xpath: string): () => unkno
6871
if (!cache.has(xpath)) {
6972
cache.set(xpath, readCompressedJsonFileFallback(xpath))
7073
if (globalCacheCounter[xpath]) {
71-
console.warn(
72-
"If this happens it's because the readCompressedJsonFileFallbackLazily " +
73-
'function has been called non-globally. Only use ' +
74-
'readCompressedJsonFileFallback once at module-level.',
74+
logger.warn(
75+
'readCompressedJsonFileFallbackLazily called non-globally. Only use readCompressedJsonFileFallback once at module-level.',
76+
{ xpath },
7577
)
7678
throw new Error(`Globally reading the same file more than once (${xpath})`)
7779
}

src/frame/middleware/api.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import express from 'express'
22
import { createProxyMiddleware } from 'http-proxy-middleware'
33

4+
import { createLogger } from '@/observability/logger'
45
import events from '@/events/middleware'
56
import anchorRedirect from '@/rest/api/anchor-redirect'
67
import aiSearch from '@/search/middleware/ai-search'
@@ -13,6 +14,7 @@ import { ExtendedRequest } from '@/types'
1314
import { noCacheControl } from './cache-control'
1415
import { STAFFONLY_COOKIE_NAME } from '@/frame/lib/constants'
1516

17+
const logger = createLogger(import.meta.url)
1618
const router = express.Router()
1719

1820
router.use('/events', events)
@@ -30,7 +32,7 @@ router.use('/article', article)
3032
if (process.env.CSE_COPILOT_ENDPOINT || process.env.NODE_ENV === 'test') {
3133
router.use('/ai-search', aiSearch)
3234
} else {
33-
console.log(
35+
logger.info(
3436
'Proxying AI Search requests to docs.github.com. To use the cse-copilot endpoint, set the CSE_COPILOT_ENDPOINT environment variable.',
3537
)
3638
router.use(aiSearchLocalProxy)

src/frame/middleware/cache-control.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import type { Response } from 'express'
22

3+
import { createLogger } from '@/observability/logger'
4+
const logger = createLogger(import.meta.url)
5+
36
interface CacheControlOptions {
47
key?: string
58
public_?: boolean
@@ -42,7 +45,7 @@ function cacheControlFactory(
4245
.join(', ')
4346
return (res: Response) => {
4447
if (process.env.NODE_ENV !== 'production' && res.hasHeader('set-cookie') && maxAge) {
45-
console.warn(
48+
logger.warn(
4649
"You can't set a >0 cache-control header AND set-cookie or else the CDN will never respect the cache-control.",
4750
)
4851
}

src/frame/middleware/render-page.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Response } from 'express'
22

33
import type { Failbot } from '@github/failbot'
44
import { get } from 'lodash-es'
5+
import { createLogger } from '@/observability/logger'
56

67
import { buildMiniTocFromCollected, type CollectedHeading } from '@/frame/lib/get-mini-toc-items'
78
import patterns from '@/frame/lib/patterns'
@@ -15,6 +16,7 @@ import { contentTypeCacheControl, defaultCacheControl } from './cache-control'
1516
import { isConnectionDropped } from './halt-on-dropped-connection'
1617
import { nextHandleRequest } from './next'
1718

19+
const logger = createLogger(import.meta.url)
1820
const STATSD_KEY_RENDER = 'middleware.render_page'
1921

2022
async function buildRenderedPage(req: ExtendedRequest): Promise<string> {
@@ -71,9 +73,9 @@ export default async function renderPage(req: ExtendedRequest, res: Response) {
7173
// render a 404 page
7274
if (!page) {
7375
if (process.env.NODE_ENV !== 'test' && context.redirectNotFound) {
74-
console.error(
75-
`\nTried to redirect to ${context.redirectNotFound}, but that page was not found.\n`,
76-
)
76+
logger.error('Tried to redirect to a page that was not found', {
77+
redirectNotFound: context.redirectNotFound,
78+
})
7779
}
7880

7981
// send minimal 404 at this point since we ran into hydration issues trying to pass

src/frame/start-server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ async function checkPortAvailability() {
3636
// Check that the development server is not already running
3737
const portInUse = await tcpPortUsed.check(port)
3838
if (portInUse) {
39-
console.log(`\n\n\nPort ${port} is not available. You may already have a server running.`)
40-
console.log(
41-
`Try running \`npx kill-port ${port}\` to shut down all your running node processes.\n\n\n`,
39+
logger.error('Port is not available. You may already have a server running.', { port })
40+
logger.error(
41+
`Try running \`npx kill-port ${port}\` to shut down all your running node processes.`,
4242
)
43-
console.log('\x07') // system 'beep' sound
43+
logger.info('\x07') // system 'beep' sound
4444
process.exit(1)
4545
}
4646
}

0 commit comments

Comments
 (0)