Skip to content

Commit 8c6194b

Browse files
committed
fix: drain packagist seed response body on error
Signed-off-by: anilb <epipav@gmail.com>
1 parent 9b0aaf5 commit 8c6194b

3 files changed

Lines changed: 42 additions & 6 deletions

File tree

services/apps/packages_worker/src/packagist/__tests__/listPackages.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,25 @@ describe('fetchPackagistPackageList', () => {
115115
)
116116
expect(await fetchPackagistPackageList()).toMatchObject({ kind: 'TRANSIENT', statusCode: 503 })
117117
})
118+
119+
// Regression: the seed's own error paths share the same 10-connection dispatcher as
120+
// the stats/p2 fetches, so an undrained body here risks the same socket exhaustion.
121+
it.each([
122+
['404', 404, 'NOT_FOUND'],
123+
['429', 429, 'RATE_LIMIT'],
124+
['503', 503, 'TRANSIENT'],
125+
])('drains the response body on a %s error', async (_status, statusCode, kind) => {
126+
const cancel = vi.fn()
127+
vi.stubGlobal(
128+
'fetch',
129+
vi.fn().mockResolvedValue({
130+
status: statusCode,
131+
ok: false,
132+
json: async () => ({}),
133+
body: { cancel },
134+
}),
135+
)
136+
expect(await fetchPackagistPackageList()).toMatchObject({ kind })
137+
expect(cancel).toHaveBeenCalled()
138+
})
118139
})

services/apps/packages_worker/src/packagist/fetchPackage.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ export function describeFetchFailure(err: unknown): string {
3636
// An error-path response whose body is never read can pin its socket instead of
3737
// returning it to packagistDispatcher's pool (capped at 10 connections per origin) —
3838
// canceling it releases the connection for reuse. Best-effort: a failure here must
39-
// never mask the real error already being returned.
40-
async function discardBody(res: Response): Promise<void> {
39+
// never mask the real error already being returned. Exported for listPackages.ts,
40+
// which shares the same dispatcher and error-path risk.
41+
export async function discardBody(res: Response): Promise<void> {
4142
try {
4243
await res.body?.cancel()
4344
} catch {

services/apps/packages_worker/src/packagist/listPackages.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import type { Dispatcher } from 'undici'
22

3-
import { buildPackagistUserAgent, describeFetchFailure, packagistDispatcher } from './fetchPackage'
3+
import {
4+
buildPackagistUserAgent,
5+
describeFetchFailure,
6+
discardBody,
7+
packagistDispatcher,
8+
} from './fetchPackage'
49
import type { FetchError } from './types'
510

611
const PACKAGIST_LIST = 'https://packagist.org/packages/list.json'
@@ -96,9 +101,18 @@ export async function fetchPackagistPackageList(): Promise<unknown | FetchError>
96101
}
97102

98103
// Status classification: 404 NOT_FOUND, 429 RATE_LIMIT, other non-ok TRANSIENT
99-
if (res.status === 404) return { kind: 'NOT_FOUND', message: 'list not found', statusCode: 404 }
100-
if (res.status === 429) return { kind: 'RATE_LIMIT', message: 'rate limited', statusCode: 429 }
101-
if (!res.ok) return { kind: 'TRANSIENT', message: `HTTP ${res.status}`, statusCode: res.status }
104+
if (res.status === 404) {
105+
await discardBody(res)
106+
return { kind: 'NOT_FOUND', message: 'list not found', statusCode: 404 }
107+
}
108+
if (res.status === 429) {
109+
await discardBody(res)
110+
return { kind: 'RATE_LIMIT', message: 'rate limited', statusCode: 429 }
111+
}
112+
if (!res.ok) {
113+
await discardBody(res)
114+
return { kind: 'TRANSIENT', message: `HTTP ${res.status}`, statusCode: res.status }
115+
}
102116

103117
let json: unknown
104118
try {

0 commit comments

Comments
 (0)