Skip to content

Commit a4e37b8

Browse files
authored
Replace got with fetch in 7 files (Phase 3A/3B) (#57193)
1 parent 3195047 commit a4e37b8

File tree

7 files changed

+115
-66
lines changed

7 files changed

+115
-66
lines changed

src/events/lib/hydro.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { createHmac } from 'crypto'
2-
import { Agent } from 'node:https'
3-
import got from 'got'
2+
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
43
import { isNil } from 'lodash-es'
54
import statsd from '@/observability/lib/statsd'
65
import { report } from '@/observability/lib/failbot'
@@ -15,7 +14,6 @@ const X_HYDRO_APP = 'docs-production'
1514
const CLUSTER = 'potomac' // We only have ability to publish externally to potomac cluster
1615
const TIMEOUT = MAX_REQUEST_TIMEOUT - 1000 // Limit because Express will terminate at MAX_REQUEST_TIMEOUT
1716
const RETRIES = 0 // We care about aggregate statistics; a few dropped events isn't a big deal
18-
const httpsAgent = new Agent({ keepAlive: true, maxSockets: 32 }) // keepAlive: https://gh.io/AAk2qio -- 32: https://bit.ly/3Tywd1U
1917
const { NODE_ENV, HYDRO_SECRET, HYDRO_ENDPOINT } = process.env
2018
const inProd = NODE_ENV === 'production'
2119

@@ -48,19 +46,27 @@ async function _publish(
4846
})
4947
const token = createHmac('sha256', secret).update(requestBody).digest('hex')
5048

51-
const response = await got.post(endpoint, {
52-
body: requestBody,
53-
agent: { https: httpsAgent },
54-
headers: {
55-
Authorization: `Hydro ${token}`,
56-
'Content-Type': 'application/json',
57-
'X-Hydro-App': X_HYDRO_APP,
49+
// Note: Custom HTTPS agent (keepAlive, maxSockets) not supported with native fetch
50+
// Consider using undici.fetch() if custom agent behavior is critical
51+
const response = await fetchWithRetry(
52+
endpoint,
53+
{
54+
method: 'POST',
55+
body: requestBody,
56+
headers: {
57+
Authorization: `Hydro ${token}`,
58+
'Content-Type': 'application/json',
59+
'X-Hydro-App': X_HYDRO_APP,
60+
},
5861
},
59-
throwHttpErrors: false,
60-
retry: { limit: RETRIES },
61-
timeout: { request: TIMEOUT },
62-
})
63-
const { statusCode, body } = response
62+
{
63+
retries: RETRIES,
64+
timeout: TIMEOUT,
65+
throwHttpErrors: false,
66+
},
67+
)
68+
const statusCode = response.status
69+
const body = await response.text()
6470

6571
statsd.increment('hydro.response_code.all', 1, [`response_code:${statusCode}`])
6672

src/frame/lib/fetch-utils.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
* Utility functions for fetch with retry and timeout functionality
33
* to replace got library functionality
44
*/
5-
65
export interface FetchWithRetryOptions {
76
retries?: number
87
retryDelay?: number
98
timeout?: number
109
throwHttpErrors?: boolean
10+
// Note: Custom HTTPS agents are not supported in native fetch
11+
// Consider using undici or node-fetch if custom agent support is critical
1112
}
1213

1314
/**

src/observability/lib/failbot.ts

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,31 @@
1-
import got, { type OptionsOfTextResponseBody, type Method } from 'got'
1+
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
22
import { Failbot, HTTPBackend } from '@github/failbot'
33
import { getLoggerContext } from '@/observability/logger/lib/logger-context'
44

55
const HAYSTACK_APP = 'docs'
66

7-
async function retryingGot(input: RequestInfo | URL, init?: RequestInit): Promise<Response> {
7+
async function retryingFetch(input: RequestInfo | URL, init?: RequestInit): Promise<Response> {
88
const url = typeof input === 'string' ? input : input.toString()
99

10-
// Extract body from fetch init for got options
11-
const gotOptions: OptionsOfTextResponseBody = {
12-
method: (init?.method as Method) || 'GET',
13-
body: typeof init?.body === 'string' ? init.body : undefined,
14-
headers: init?.headers as Record<string, string> | undefined,
15-
// With the timeout at 3000 (milliseconds) and the retry.limit
16-
// at 4 (times), the total worst-case is:
17-
// 3000 * 4 + 1000 + 2000 + 3000 + 4000 + 8000 = 30 seconds
18-
timeout: {
19-
response: 3000,
10+
// Use fetchWithRetry with retry configuration matching got's behavior
11+
// With the timeout at 3000 (milliseconds) and the retry.limit
12+
// at 4 (times), the total worst-case is:
13+
// 3000 * 4 + 1000 + 2000 + 3000 + 4000 + 8000 = 30 seconds
14+
const response = await fetchWithRetry(
15+
url,
16+
{
17+
method: init?.method || 'GET',
18+
body: init?.body,
19+
headers: init?.headers,
2020
},
21-
retry: {
22-
// This means it will wait...
23-
// 1. 1000ms
24-
// 2. 2000ms
25-
// 3. 4000ms
26-
// 4. 8000ms
27-
// 5. give up!
28-
//
29-
// From the documentation:
30-
//
31-
// Delays between retries counts with function
32-
// 1000 * Math.pow(2, retry - 1) + Math.random() * 100,
33-
// where retry is attempt number (starts from 1).
34-
//
35-
limit: 4,
21+
{
22+
timeout: 3000,
23+
retries: 4,
24+
throwHttpErrors: false, // Let failbot handle HTTP errors
3625
},
37-
}
38-
39-
const gotResponse = await got(url, gotOptions)
26+
)
4027

41-
// Convert got response to fetch-compatible Response
42-
return new Response(gotResponse.body, {
43-
status: gotResponse.statusCode,
44-
statusText: gotResponse.statusMessage,
45-
headers: gotResponse.headers as HeadersInit,
46-
})
28+
return response
4729
}
4830

4931
export function report(error: Error, metadata?: Record<string, unknown>) {
@@ -55,7 +37,7 @@ export function report(error: Error, metadata?: Record<string, unknown>) {
5537
const backends = [
5638
new HTTPBackend({
5739
haystackURL: process.env.HAYSTACK_URL,
58-
fetchFn: retryingGot,
40+
fetchFn: retryingFetch,
5941
}),
6042
]
6143
const failbot = new Failbot({

src/search/middleware/general-search-middleware.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ This file & middleware is for when a user requests our /search page e.g. 'docs.g
66
When a user directly hits our API e.g. /api/search/v1?query=foo, they will hit the routes in ./search-routes.ts
77
*/
88

9-
import got from 'got'
9+
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
1010
import { Request, Response, NextFunction } from 'express'
1111
import { errors } from '@elastic/elasticsearch'
1212
import statsd from '@/observability/lib/statsd'
@@ -172,5 +172,10 @@ async function getProxySearch(
172172
// Add client_name for external API requests
173173
url.searchParams.set('client_name', 'docs.github.com-client')
174174
console.log(`Proxying search to ${url}`)
175-
return got(url).json<GeneralSearchResponse>()
175+
176+
const response = await fetchWithRetry(url.toString())
177+
if (!response.ok) {
178+
throw new Error(`HTTP ${response.status}: ${response.statusText}`)
179+
}
180+
return response.json() as Promise<GeneralSearchResponse>
176181
}

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import eventToPromise from 'event-to-promise'
22
import chalk from 'chalk'
33
import dotenv from 'dotenv'
44
import boxen from 'boxen'
5-
import { HTTPError } from 'got'
65

76
import languages from '@/languages/lib/languages'
87
import parsePageSectionsIntoRecords from '@/search/scripts/scrape/lib/parse-page-sections-into-records'
@@ -12,6 +11,23 @@ import { getAllVersionsKeyFromIndexVersion } from '@/search/lib/elasticsearch-ve
1211

1312
import type { Page, Permalink, Record, Config, Redirects } from '@/search/scripts/scrape/types'
1413

14+
// Custom error class to replace got's HTTPError
15+
class HTTPError extends Error {
16+
response: { ok: boolean; statusCode?: number }
17+
request: { requestUrl?: { pathname?: string } }
18+
19+
constructor(
20+
message: string,
21+
response: { ok: boolean; statusCode?: number },
22+
request: { requestUrl?: { pathname?: string } },
23+
) {
24+
super(message)
25+
this.name = 'HTTPError'
26+
this.response = response
27+
this.request = request
28+
}
29+
}
30+
1531
const pageMarker = chalk.green('|')
1632
const recordMarker = chalk.grey('.')
1733
const port = 4002

src/search/scripts/scrape/lib/domwaiter.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,27 @@
11
import { EventEmitter } from 'events'
22
import Bottleneck from 'bottleneck'
3-
import got from 'got'
3+
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
44
import cheerio from 'cheerio'
55

66
import type { Permalink } from '@/search/scripts/scrape/types'
77

8+
// Custom error class to match got's HTTPError interface
9+
class HTTPError extends Error {
10+
response: { ok: boolean; statusCode?: number }
11+
request: { requestUrl?: { pathname?: string } }
12+
13+
constructor(
14+
message: string,
15+
response: { ok: boolean; statusCode?: number },
16+
request: { requestUrl?: { pathname?: string } },
17+
) {
18+
super(message)
19+
this.name = 'HTTPError'
20+
this.response = response
21+
this.request = request
22+
}
23+
}
24+
825
interface DomWaiterOptions {
926
parseDOM?: boolean
1027
json?: boolean
@@ -45,15 +62,31 @@ async function getPage(page: Permalink, emitter: EventEmitter, opts: DomWaiterOp
4562

4663
if (opts.json) {
4764
try {
48-
const json = await got(page.url!).json()
65+
const response = await fetchWithRetry(page.url!)
66+
if (!response.ok) {
67+
throw new HTTPError(
68+
`HTTP ${response.status}: ${response.statusText}`,
69+
{ ok: response.ok, statusCode: response.status },
70+
{ requestUrl: { pathname: page.url } },
71+
)
72+
}
73+
const json = await response.json()
4974
const pageCopy = Object.assign({}, page, { json })
5075
emitter.emit('page', pageCopy)
5176
} catch (err) {
5277
emitter.emit('error', err)
5378
}
5479
} else {
5580
try {
56-
const body = (await got(page.url!)).body
81+
const response = await fetchWithRetry(page.url!)
82+
if (!response.ok) {
83+
throw new HTTPError(
84+
`HTTP ${response.status}: ${response.statusText}`,
85+
{ ok: response.ok, statusCode: response.status },
86+
{ requestUrl: { pathname: page.url } },
87+
)
88+
}
89+
const body = await response.text()
5790
const pageCopy = Object.assign({}, page, { body })
5891
if (opts.parseDOM) (pageCopy as any).$ = cheerio.load(body)
5992
emitter.emit('page', pageCopy)

src/workflows/experimental/readability-report.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import fs from 'fs'
3939
import path from 'path'
4040

4141
import cheerio from 'cheerio'
42-
import got from 'got'
42+
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
4343

4444
interface ReadabilityMetrics {
4545
fleschReadingEase: number
@@ -174,7 +174,12 @@ async function waitForServer(): Promise<void> {
174174

175175
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
176176
try {
177-
await got(makeURL('/'), { timeout: { request: 5000 } })
177+
const response = await fetchWithRetry(makeURL('/'), undefined, {
178+
timeout: 5000,
179+
})
180+
if (!response.ok) {
181+
throw new Error(`HTTP ${response.status}: ${response.statusText}`)
182+
}
178183
console.log('Server is ready!')
179184
return
180185
} catch (error) {
@@ -202,18 +207,19 @@ async function analyzeFile(filePath: string): Promise<PageReadability | null> {
202207

203208
try {
204209
// Fetch the rendered page
205-
const response = await got(makeURL(urlPath), {
206-
timeout: { request: 30000 },
210+
const response = await fetchWithRetry(makeURL(urlPath), undefined, {
211+
timeout: 30000,
207212
throwHttpErrors: false,
208213
})
209214

210-
if (response.statusCode !== 200) {
211-
console.warn(`Skipping ${urlPath}: HTTP ${response.statusCode}`)
215+
if (response.status !== 200) {
216+
console.warn(`Skipping ${urlPath}: HTTP ${response.status}`)
212217
return null
213218
}
214219

215220
// Parse HTML and extract content
216-
const $ = cheerio.load(response.body)
221+
const body = await response.text()
222+
const $ = cheerio.load(body)
217223

218224
// Get page title
219225
const title = $('h1').first().text().trim() || $('title').text().trim() || 'Untitled'

0 commit comments

Comments
 (0)