Skip to content

Commit ae38225

Browse files
authored
Merge pull request github#39868 from github/repo-sync
Repo sync
2 parents 7f001cf + 77ad892 commit ae38225

File tree

3 files changed

+37
-21
lines changed

3 files changed

+37
-21
lines changed

src/content-render/scripts/add-content-type.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,7 @@ function processFile(filePath: string, options: ScriptOptions) {
102102
// Remove the legacy type property if option is passed
103103
const removeLegacyType = Boolean(options.removeType && data.type)
104104

105-
// Skip if contentType already exists and we're not removing legacy type
106-
if (data.contentType && !removeLegacyType) {
107-
console.log(`contentType already set on ${relativePath}`)
108-
return { processed: true, updated: false }
109-
}
110-
111-
const newContentType = data.contentType || determineContentType(relativePath, data.type || '')
105+
const newContentType = determineContentType(relativePath, data.type || '')
112106

113107
if (options.dryRun) {
114108
console.log(`\n${relativePath}`)
@@ -121,9 +115,24 @@ function processFile(filePath: string, options: ScriptOptions) {
121115
return { processed: true, updated: false }
122116
}
123117

124-
// Set the contentType property if it doesn't exist
125-
if (!data.contentType) {
118+
// Check if we're actually changing an existing contentType
119+
const isChangingContentType = data.contentType && data.contentType !== newContentType
120+
const isAddingContentType = !data.contentType
121+
122+
if (isChangingContentType) {
123+
console.log(
124+
`Changing contentType from '${data.contentType}' to '${newContentType}' on ${relativePath}`,
125+
)
126+
} else if (isAddingContentType) {
127+
console.log(`Adding contentType '${newContentType}' on ${relativePath}`)
128+
}
129+
130+
// Only update if there's actually a change needed
131+
if (isChangingContentType || isAddingContentType) {
126132
data.contentType = newContentType
133+
} else {
134+
console.log(`contentType is already set to '${data.contentType}' on ${relativePath}`)
135+
return { processed: true, updated: false }
127136
}
128137

129138
let legacyTypeValue

src/events/lib/hydro.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import { isNil } from 'lodash-es'
55
import statsd from '@/observability/lib/statsd'
66
import { report } from '@/observability/lib/failbot'
77
import { MAX_REQUEST_TIMEOUT } from '@/frame/lib/constants'
8+
import { createLogger } from '@/observability/logger'
9+
10+
const logger = createLogger(import.meta.url)
811

912
const TIME_OUT_TEXT = 'ms has passed since batch creation'
1013
const SERVER_DISCONNECT_TEXT = 'The server disconnected before a response was received'
@@ -70,6 +73,13 @@ async function _publish(
7073
) {
7174
const error = new Error(`Failed to send event to Hydro (${statusCode})`)
7275
if (inProd) {
76+
logger.error('Failed to send event to Hydro', {
77+
error,
78+
statusCode,
79+
body,
80+
requestBody,
81+
})
82+
// Report the error to Failbot
7383
report(error, { statusCode, body, requestBody })
7484
} else {
7585
throw error

src/search/lib/helpers/external-search-analytics.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { ExtendedRequest } from '@/types'
12
import { publish } from '@/events/lib/hydro'
23
import { hydroNames } from '@/events/lib/schema'
34
import { createLogger } from '@/observability/logger'
@@ -9,7 +10,7 @@ const logger = createLogger(import.meta.url)
910
* Returns null if the request should continue, or an error response object if validation failed
1011
*/
1112
export async function handleExternalSearchAnalytics(
12-
req: any,
13+
req: ExtendedRequest,
1314
searchContext: string,
1415
): Promise<{ error: string; status: number } | null> {
1516
const host = req.headers['x-host'] || req.headers.host
@@ -73,12 +74,12 @@ export async function handleExternalSearchAnalytics(
7374
version: '1.0.0',
7475
created: new Date().toISOString(),
7576
hostname: normalizedHost,
76-
path: '',
77-
search: '',
77+
path: req?.context?.path || '',
78+
search: 'REDACTED',
7879
hash: '',
79-
path_language: 'en',
80-
path_version: '',
81-
path_product: '',
80+
path_language: req?.context?.language || 'en',
81+
path_version: req?.context?.version || '',
82+
path_product: req?.context?.product || '',
8283
path_article: '',
8384
},
8485
search_query: 'REDACTED',
@@ -90,7 +91,7 @@ export async function handleExternalSearchAnalytics(
9091
await publish(analyticsPayload)
9192
} catch (error) {
9293
// Don't fail the request if analytics fails
93-
console.error('Failed to send search analytics:', error)
94+
logger.error('Failed to send search analytics:', { error })
9495
}
9596

9697
return null
@@ -146,16 +147,12 @@ function stripPort(host: string): string {
146147
return hostname
147148
}
148149

149-
interface ExternalAPIRequestLike {
150-
headers: Record<string, string | undefined>
151-
}
152-
153150
/**
154151
* Determines if a request is likely from an external API client rather than a browser
155152
* Uses multiple heuristics to detect programmatic vs browser requests
156153
*/
157154
const userAgentRegex = /^(curl|wget|python-requests|axios|node-fetch|Go-http-client|okhttp)/i
158-
function isExternalAPIRequest(req: ExternalAPIRequestLike): boolean {
155+
function isExternalAPIRequest(req: ExtendedRequest): boolean {
159156
const headers = req.headers
160157

161158
// Browser security headers that APIs typically don't send

0 commit comments

Comments
 (0)