Skip to content

Commit 689e7f8

Browse files
authored
Merge pull request #44643 from github/repo-sync
Repo sync
2 parents c6e9ae8 + dff19b1 commit 689e7f8

91 files changed

Lines changed: 22159 additions & 544 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
# ---------------------------------------------------------------
1111
# To update the sha:
1212
# https://github.com/github/gh-base-image/pkgs/container/gh-base-image%2Fgh-base-noble
13-
FROM ghcr.io/github/gh-base-image/gh-base-noble:20260527-203230-gabf2049e0@sha256:e6d4192acbdf566584c77f1306cd72cac3a381be6ff6174c85ee21bb164a8b46 AS base
13+
FROM ghcr.io/github/gh-base-image/gh-base-noble:20260603-101723-g62a660e20@sha256:216e9ea02bb1f000a3e9ed1beeb9127b33a7661ae61f041879e61fffd52e1bd2 AS base
1414

1515
# Install curl for Node install and determining the early access branch
1616
# Install git for cloning docs-early-access & translations repos

src/ai-tools/lib/spaces-utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* Copilot Space API response types
33
*/
4-
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
4+
import { fetchWithRetry, readBodyWithTimeout } from '@/frame/lib/fetch-utils'
55

66
export interface SpaceResource {
77
id: number
@@ -85,7 +85,7 @@ export async function fetchCopilotSpace(spaceUrl: string): Promise<SpaceData> {
8585
}
8686
}
8787

88-
return (await response.json()) as SpaceData
88+
return (await readBodyWithTimeout(response, () => response.json(), 30_000)) as SpaceData
8989
}
9090

9191
/**

src/archives/middleware/archived-enterprise-versions-assets.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
1+
import { fetchWithRetry, readBodyWithTimeout } from '@/frame/lib/fetch-utils'
22
import type { Response, NextFunction } from 'express'
33

44
import patterns from '@/frame/lib/patterns'
@@ -100,7 +100,7 @@ export default async function archivedEnterpriseVersionsAssets(
100100
},
101101
)
102102

103-
const body = await r.arrayBuffer()
103+
const body = await readBodyWithTimeout(r, () => r.arrayBuffer(), 8_000)
104104

105105
res.set('accept-ranges', 'bytes')
106106
const contentType = r.headers.get('content-type')

src/archives/middleware/archived-enterprise-versions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Response, NextFunction } from 'express'
2-
import { fetchWithRetry } from '@/frame/lib/fetch-utils'
2+
import { fetchWithRetry, readBodyWithTimeout } from '@/frame/lib/fetch-utils'
33

44
import statsd, { adaptForTimer } from '@/observability/lib/statsd'
55
import { createLogger } from '@/observability/logger'
@@ -275,7 +275,7 @@ export default async function archivedEnterpriseVersions(
275275
if (r.status !== 200) {
276276
let upstreamBody: string | undefined
277277
try {
278-
upstreamBody = await r.text()
278+
upstreamBody = await readBodyWithTimeout(r, () => r.text(), timeoutConfiguration.response)
279279
} catch {
280280
// ignore — body reading failure shouldn't affect error handling
281281
}
@@ -301,7 +301,7 @@ export default async function archivedEnterpriseVersions(
301301
}
302302

303303
if (r.status === 200) {
304-
const body = await r.text()
304+
const body = await readBodyWithTimeout(r, () => r.text(), timeoutConfiguration.response)
305305
const [, withoutLanguagePath] = splitByLanguage(req.path)
306306
const isDeveloperPage = withoutLanguagePath?.startsWith(
307307
`/enterprise/${requestedVersion}/developer`,

src/frame/lib/fetch-utils.ts

Lines changed: 116 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@ export interface FetchWithRetryOptions {
1111
retryDelay?: number
1212
timeout?: number
1313
throwHttpErrors?: boolean
14+
/**
15+
* How the `timeout` is enforced:
16+
* - 'full' (default): bounds the entire request, including body transfer.
17+
* The abort signal stays armed through body reads, so pair this with
18+
* `readBodyWithTimeout` to report body-phase timeouts consistently.
19+
* - 'ttfb': bounds only time-to-first-byte. The timer is cleared once the
20+
* response resolves, leaving body reads unbounded. Use for large, trusted,
21+
* well-cached payloads (e.g. multi-MB archived `redirects.json`) where a
22+
* short deadline should fail fast on an unresponsive server but must not
23+
* abort a legitimately long download.
24+
*/
25+
timeoutMode?: 'full' | 'ttfb'
1426
// Note: Custom HTTPS agents are not supported in native fetch
1527
// Consider using undici or node-fetch if custom agent support is critical
1628
}
@@ -30,45 +42,120 @@ function sleep(ms: number): Promise<void> {
3042
return new Promise((resolve) => setTimeout(resolve, ms))
3143
}
3244

45+
function getHost(url: string | URL): string {
46+
try {
47+
return new URL(typeof url === 'string' ? url : url.toString()).host
48+
} catch {
49+
return 'unknown'
50+
}
51+
}
52+
3353
/**
34-
* Fetch with timeout support
54+
* Fetch with timeout support.
55+
*
56+
* `timeoutMode` controls what the deadline bounds:
57+
*
58+
* - 'full' (default): enforced with `AbortSignal.timeout()`, whose signal
59+
* stays armed after this function returns. Callers typically read the body
60+
* (`r.json()`, `r.arrayBuffer()`) after the response resolves; because the
61+
* signal is never cleared, that body read is aborted by the same deadline.
62+
* So the timeout bounds the full request (time-to-first-byte AND body
63+
* transfer). Use `readBodyWithTimeout` to consume the body so a body-phase
64+
* timeout reports the same way as a TTFB timeout.
65+
*
66+
* - 'ttfb': enforced with a manual `AbortController` whose timer is cleared as
67+
* soon as the response resolves. Only time-to-first-byte is bounded; the
68+
* body read that follows is left unbounded. Use for large, trusted,
69+
* well-cached payloads where a short deadline should fail fast on an
70+
* unresponsive server but not abort a legitimately long download.
3571
*/
3672
async function fetchWithTimeout(
3773
url: string | URL,
3874
init?: RequestInit,
3975
timeout?: number,
76+
timeoutMode: 'full' | 'ttfb' = 'full',
4077
): Promise<Response> {
4178
if (!timeout) {
4279
return fetch(url, init)
4380
}
4481

45-
const controller = new AbortController()
46-
const timeoutId = setTimeout(() => controller.abort(), timeout)
82+
if (timeoutMode === 'ttfb') {
83+
// Abort if headers don't arrive in time, but clear the timer once the
84+
// response resolves so the subsequent body read isn't bounded by the same
85+
// deadline.
86+
const controller = new AbortController()
87+
const signal = init?.signal
88+
? AbortSignal.any([init.signal, controller.signal])
89+
: controller.signal
90+
let timedOut = false
91+
const timer = setTimeout(() => {
92+
timedOut = true
93+
controller.abort()
94+
}, timeout)
95+
96+
try {
97+
return await fetch(url, { ...init, signal })
98+
} catch (error) {
99+
// Only our own timer firing counts as a timeout; a caller-provided
100+
// signal aborting is left untouched so it isn't misreported.
101+
if (timedOut) {
102+
statsd.increment(STATSD_FETCH_TIMEOUT, 1, [`host:${getHost(url)}`])
103+
throw new Error(`Request timed out after ${timeout}ms`)
104+
}
105+
throw error
106+
} finally {
107+
clearTimeout(timer)
108+
}
109+
}
110+
111+
const timeoutSignal = AbortSignal.timeout(timeout)
112+
// Honor a caller-provided signal too, rather than overwriting it.
113+
const signal = init?.signal ? AbortSignal.any([init.signal, timeoutSignal]) : timeoutSignal
47114

48115
try {
49-
const response = await fetch(url, {
50-
...init,
51-
signal: controller.signal,
52-
})
53-
clearTimeout(timeoutId)
54-
return response
116+
return await fetch(url, { ...init, signal })
55117
} catch (error) {
56-
clearTimeout(timeoutId)
57-
if (error instanceof Error && error.name === 'AbortError') {
58-
const host = (() => {
59-
try {
60-
return new URL(typeof url === 'string' ? url : url.toString()).host
61-
} catch {
62-
return 'unknown'
63-
}
64-
})()
65-
statsd.increment(STATSD_FETCH_TIMEOUT, 1, [`host:${host}`])
118+
// `AbortSignal.timeout()` aborts with a `TimeoutError`; a caller-provided
119+
// signal aborts with its own reason (e.g. `AbortError`), which we leave
120+
// untouched so caller cancellations aren't misreported as timeouts.
121+
if (error instanceof Error && error.name === 'TimeoutError') {
122+
statsd.increment(STATSD_FETCH_TIMEOUT, 1, [`host:${getHost(url)}`])
66123
throw new Error(`Request timed out after ${timeout}ms`)
67124
}
68125
throw error
69126
}
70127
}
71128

129+
/**
130+
* Read a response body, reporting a timeout the same way `fetchWithTimeout`
131+
* does for time-to-first-byte.
132+
*
133+
* The body read is already bounded by the deadline set on the originating
134+
* `fetchWithRetry`/`fetchWithTimeout` call (the abort signal stays armed
135+
* through body transfer). This wrapper just translates the resulting
136+
* `TimeoutError` into the friendly "Request timed out" error and emits the
137+
* `fetch.timeout` metric, so body-phase timeouts are observable.
138+
*
139+
* @param response The response whose body to read (used for the metric host).
140+
* @param read A callback that performs the body read, e.g. `() => r.json()`.
141+
* @param timeout The deadline that bounded the request, for the error message.
142+
*/
143+
export async function readBodyWithTimeout<T>(
144+
response: Response,
145+
read: () => Promise<T>,
146+
timeout?: number,
147+
): Promise<T> {
148+
try {
149+
return await read()
150+
} catch (error) {
151+
if (error instanceof Error && error.name === 'TimeoutError') {
152+
statsd.increment(STATSD_FETCH_TIMEOUT, 1, [`host:${getHost(response.url)}`])
153+
throw new Error(timeout ? `Request timed out after ${timeout}ms` : 'Request timed out')
154+
}
155+
throw error
156+
}
157+
}
158+
72159
/**
73160
* Fetch with retry logic matching got's behavior
74161
*/
@@ -77,13 +164,13 @@ export async function fetchWithRetry(
77164
init?: RequestInit,
78165
options: FetchWithRetryOptions = {},
79166
): Promise<Response> {
80-
const { retries = 0, timeout, throwHttpErrors = true } = options
167+
const { retries = 0, timeout, throwHttpErrors = true, timeoutMode = 'full' } = options
81168

82169
let lastError: Error | null = null
83170

84171
for (let attempt = 0; attempt <= retries; attempt++) {
85172
try {
86-
const response = await fetchWithTimeout(url, init, timeout)
173+
const response = await fetchWithTimeout(url, init, timeout, timeoutMode)
87174

88175
// Check if we should retry based on status code
89176
if (response.status >= 500 && attempt < retries) {
@@ -128,15 +215,21 @@ export async function fetchWithRetry(
128215
/**
129216
* Create a streaming fetch request that returns a ReadableStream
130217
* This replaces got.stream functionality
218+
*
219+
* Defaults to `timeoutMode: 'ttfb'` because streaming callers consume the body
220+
* incrementally over a `reader.read()` loop that can legitimately run far longer
221+
* than the connect deadline. A `'full'` default would keep `AbortSignal.timeout()`
222+
* armed through that loop and abort a valid long answer mid-stream. Callers that
223+
* want the deadline to bound the whole transfer can pass `timeoutMode: 'full'`.
131224
*/
132225
export async function fetchStream(
133226
url: string | URL,
134227
init?: RequestInit,
135228
options: FetchWithRetryOptions = {},
136229
): Promise<Response> {
137-
const { timeout, throwHttpErrors = true } = options
230+
const { timeout, throwHttpErrors = true, timeoutMode = 'ttfb' } = options
138231

139-
const response = await fetchWithTimeout(url, init, timeout)
232+
const response = await fetchWithTimeout(url, init, timeout, timeoutMode)
140233

141234
// Check for HTTP errors if throwHttpErrors is enabled
142235
if (throwHttpErrors && !response.ok && response.status >= 400) {

src/frame/lib/get-remote-json.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ export default async function getRemoteJSON(
113113
retries,
114114
timeout,
115115
throwHttpErrors: true,
116+
// The `redirects.json` files are large (5-10MB) but well-cached, and
117+
// the configured timeout is deliberately a short time-to-first-byte
118+
// budget (got's `timeout.response` semantics). Bound only TTFB so a
119+
// slow server fails fast without aborting a legitimately long body
120+
// download mid-transfer.
121+
timeoutMode: 'ttfb',
116122
},
117123
)
118124

0 commit comments

Comments
 (0)