Skip to content

Commit 0bbf8b8

Browse files
pastelskypastelsky
committed
fix: map package-not-found errors from all package managers to PackageNotFoundError
Previously only npm E404 errors were caught; ETARGET (version range with no match), yarn classic YN0035, pnpm ERR_PNPM_NO_MATCHING_VERSION / ERR_PNPM_FETCH_404, and bun 'package not found' errors were all silently wrapped as InstallError and surfaced as HTTP 500 instead of HTTP 404. Add isPackageNotFound() helper that matches the canonical error strings emitted by npm, yarn (classic + berry), pnpm, and bun, and wire it into installWithClient's catch block. Add 11 mock-based fast unit tests that verify each pattern without doing any real network installs. Move the shell-metacharacter local-install integration test to slow/local.test.ts where exec is not mocked.
1 parent 7864791 commit 0bbf8b8

4 files changed

Lines changed: 214 additions & 65 deletions

File tree

‎.changeset/fix-pkg-not-found-all-pms.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'package-build-stats': patch
3+
---
4+
5+
Map "package / version not found" errors from npm, yarn (classic + berry), pnpm, and bun to `PackageNotFoundError` (HTTP 404) instead of the generic `InstallError`. Previously only npm `E404` was handled; `ETARGET`, yarn `YN0035`, pnpm `ERR_PNPM_NO_MATCHING_VERSION`/`ERR_PNPM_FETCH_404`, and bun "package not found" errors were all misclassified.

‎src/utils/installation.utils.ts‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,24 @@ function getInstallArgs(
8989
return args
9090
}
9191

92+
// Returns true when the error indicates the package name or version doesn't
93+
// exist in the registry. Covers npm, yarn (classic + berry), pnpm, and bun.
94+
function isPackageNotFound(err: ProcessExecutionError): boolean {
95+
const output = `${err.stderr}\n${err.stdout}`
96+
97+
return (
98+
output.includes('code E404') ||
99+
output.includes('code ETARGET') ||
100+
output.includes("Couldn't find package") ||
101+
output.includes('YN0035') ||
102+
output.includes('ERR_PNPM_NO_MATCHING_VERSION') ||
103+
output.includes('ERR_PNPM_FETCH_404') ||
104+
/bun.*package not found/i.test(output) ||
105+
output.includes('404 Not Found') ||
106+
output.includes('No matching version found')
107+
)
108+
}
109+
92110
const InstallationUtils = {
93111
getInstallPath(packageName: string) {
94112
const id = randomUUID().slice(0, 8)
@@ -290,10 +308,7 @@ const InstallationUtils = {
290308
...installOptions,
291309
client: currentClient,
292310
})
293-
if (
294-
err instanceof ProcessExecutionError &&
295-
`${err.stderr}\n${err.stdout}`.includes('code E404')
296-
) {
311+
if (err instanceof ProcessExecutionError && isPackageNotFound(err)) {
297312
throw new PackageNotFoundError(err)
298313
} else {
299314
throw new InstallError(err)

‎tests/fast/installation.utils.test.ts‎

Lines changed: 134 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,59 +1,21 @@
1-
import fs from 'node:fs/promises'
2-
import os from 'node:os'
3-
import path from 'node:path'
41
import { vi } from 'vitest'
52

6-
import { BuildCancelledError } from '../../src/errors/CustomError.js'
3+
import {
4+
BuildCancelledError,
5+
InstallError,
6+
PackageNotFoundError,
7+
} from '../../src/errors/CustomError.js'
8+
import { exec, ProcessExecutionError } from '../../src/utils/common.utils.js'
79
import InstallationUtils from '../../src/utils/installation.utils.js'
810

9-
describe('InstallationUtils', () => {
10-
test('installs a local package whose path contains shell metacharacters', async () => {
11-
const temporaryDirectory = await fs.mkdtemp(
12-
path.join(os.tmpdir(), 'package-build-stats-install-'),
13-
)
14-
const packagePath = path.join(temporaryDirectory, 'fixture; not-a-command')
15-
const installPath = await InstallationUtils.preparePath('local-fixture')
16-
17-
try {
18-
await fs.mkdir(packagePath)
19-
await fs.writeFile(
20-
path.join(packagePath, 'package.json'),
21-
JSON.stringify({
22-
name: 'local-fixture',
23-
version: '1.0.0',
24-
main: 'index.js',
25-
}),
26-
)
27-
await fs.writeFile(
28-
path.join(packagePath, 'index.js'),
29-
'module.exports = 42\n',
30-
)
31-
32-
await InstallationUtils.installWithClient(
33-
packagePath,
34-
installPath,
35-
{ client: 'npm', isLocal: true, installTimeout: 10_000 },
36-
'npm',
37-
)
11+
function makeProcessError(stderr: string, stdout = '') {
12+
return new ProcessExecutionError('install failed', stdout, stderr, 1)
13+
}
3814

39-
const installedPackage = JSON.parse(
40-
await fs.readFile(
41-
path.join(
42-
installPath,
43-
'node_modules',
44-
'local-fixture',
45-
'package.json',
46-
),
47-
'utf8',
48-
),
49-
)
50-
expect(installedPackage.name).toBe('local-fixture')
51-
} finally {
52-
await InstallationUtils.cleanupPath(installPath)
53-
await fs.rm(temporaryDirectory, { recursive: true, force: true })
54-
}
55-
})
15+
// The local-package integration test (npm-pack + real install) lives in
16+
// tests/slow/local.test.ts – exec is mocked for the whole file below.
5617

18+
describe('InstallationUtils', () => {
5719
test('throws instead of terminating the host for an invalid client', async () => {
5820
await expect(
5921
InstallationUtils.installWithClient(
@@ -70,18 +32,129 @@ describe('InstallationUtils', () => {
7032
.spyOn(InstallationUtils, 'installWithClient')
7133
.mockRejectedValue(new BuildCancelledError())
7234

73-
await expect(
74-
InstallationUtils.installPackage('example', '/tmp/unused', {
75-
client: ['bun', 'npm'],
76-
}),
77-
).rejects.toBeInstanceOf(BuildCancelledError)
35+
try {
36+
await expect(
37+
InstallationUtils.installPackage('example', '/tmp/unused', {
38+
client: ['bun', 'npm'],
39+
}),
40+
).rejects.toBeInstanceOf(BuildCancelledError)
41+
42+
expect(installWithClient).toHaveBeenCalledTimes(1)
43+
expect(installWithClient).toHaveBeenCalledWith(
44+
'example',
45+
'/tmp/unused',
46+
expect.objectContaining({ client: 'bun' }),
47+
'bun',
48+
)
49+
} finally {
50+
installWithClient.mockRestore()
51+
}
52+
})
53+
})
54+
55+
// vi.mock is hoisted by Vitest, so exec is replaced for the whole file.
56+
vi.mock('../../src/utils/common.utils.js', async importOriginal => {
57+
const actual =
58+
await importOriginal<typeof import('../../src/utils/common.utils.js')>()
59+
return {
60+
...actual,
61+
exec: vi.fn(),
62+
}
63+
})
64+
65+
describe('installWithClient – package-not-found error classification', () => {
66+
afterEach(() => {
67+
vi.resetAllMocks()
68+
})
69+
70+
const cases: Array<{
71+
pm: 'npm' | 'yarn' | 'pnpm' | 'bun'
72+
label: string
73+
stderr: string
74+
stdout?: string
75+
}> = [
76+
{
77+
pm: 'npm',
78+
label: 'npm E404 (unknown package)',
79+
stderr:
80+
'npm error code E404\nnpm error 404 Not Found - GET https://registry.npmjs.org/no-such-pkg',
81+
},
82+
{
83+
pm: 'npm',
84+
label: 'npm ETARGET (version range has no match)',
85+
stderr:
86+
'npm error code ETARGET\nnpm error notarget No matching version found for lodash@999.0.0',
87+
},
88+
{
89+
pm: 'npm',
90+
label: 'npm "No matching version found" text',
91+
stderr:
92+
'npm ERR! code ETARGET\nnpm ERR! No matching version found for react@0.0.0-nonexistent.',
93+
},
94+
{
95+
pm: 'yarn',
96+
label: "yarn classic – Couldn't find package",
97+
stderr:
98+
'error Couldn\'t find package "no-such-pkg" on the "npm" registry',
99+
},
100+
{
101+
pm: 'yarn',
102+
label: 'yarn berry – YN0035',
103+
stderr:
104+
"YN0035: │ no-such-pkg@npm:^1.0.0 couldn't be resolved to a satisfying range",
105+
},
106+
{
107+
pm: 'pnpm',
108+
label: 'pnpm ERR_PNPM_NO_MATCHING_VERSION',
109+
stderr:
110+
'ERR_PNPM_NO_MATCHING_VERSION No matching version found for no-such-pkg@999.0.0',
111+
},
112+
{
113+
pm: 'pnpm',
114+
label: 'pnpm ERR_PNPM_FETCH_404',
115+
stderr:
116+
'ERR_PNPM_FETCH_404 GET https://registry.npmjs.org/no-such-pkg: Not Found - 404',
117+
},
118+
{
119+
pm: 'bun',
120+
label: 'bun – package not found',
121+
stderr: 'error: bun package not found no-such-pkg',
122+
},
123+
{
124+
pm: 'bun',
125+
label: 'bun – 404 Not Found',
126+
stderr:
127+
'404 Not Found\nGET https://registry.npmjs.org/no-such-pkg/-/no-such-pkg-1.0.0.tgz',
128+
},
129+
]
130+
131+
for (const { pm, label, stderr, stdout = '' } of cases) {
132+
test(`throws PackageNotFoundError for: ${label}`, async () => {
133+
vi.mocked(exec).mockRejectedValue(makeProcessError(stderr, stdout))
134+
135+
await expect(
136+
InstallationUtils.installWithClient(
137+
'no-such-pkg@999.0.0',
138+
'/tmp/unused-install-path',
139+
{ client: pm },
140+
pm,
141+
),
142+
).rejects.toBeInstanceOf(PackageNotFoundError)
143+
})
144+
}
78145

79-
expect(installWithClient).toHaveBeenCalledTimes(1)
80-
expect(installWithClient).toHaveBeenCalledWith(
81-
'example',
82-
'/tmp/unused',
83-
expect.objectContaining({ client: 'bun' }),
84-
'bun',
146+
test('throws InstallError for generic (non-404) failures', async () => {
147+
vi.mocked(exec).mockRejectedValue(
148+
makeProcessError('npm ERR! network timeout', ''),
85149
)
150+
151+
await expect(
152+
InstallationUtils.installWithClient(
153+
'some-pkg',
154+
'/tmp/unused-install-path',
155+
{ client: 'npm' },
156+
'npm',
157+
),
158+
).rejects.toBeInstanceOf(InstallError)
86159
})
87160
})

‎tests/slow/local.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
* @jest-environment node
33
*/
44

5+
import fs from 'node:fs/promises'
6+
import os from 'node:os'
57
import path from 'node:path'
68
import { getPackageStats } from '../../src'
9+
import InstallationUtils from '../../src/utils/installation.utils.js'
710
import 'dotenv/config'
811

912
describe('getPackageStats', () => {
@@ -40,3 +43,56 @@ describe('getPackageStats', () => {
4043
// - "module" field without "main"
4144
// - export * re-export chains across multiple files
4245
// - nested folder structures with complex re-exports
46+
47+
describe('InstallationUtils (integration)', () => {
48+
/**
49+
* Regression: local package paths containing shell metacharacters (e.g. `;`)
50+
* must never be passed through a shell – they are forwarded as argv arguments.
51+
*/
52+
test('installs a local package whose path contains shell metacharacters', async () => {
53+
const temporaryDirectory = await fs.mkdtemp(
54+
path.join(os.tmpdir(), 'package-build-stats-install-'),
55+
)
56+
const packagePath = path.join(temporaryDirectory, 'fixture; not-a-command')
57+
const installPath = await InstallationUtils.preparePath('local-fixture')
58+
59+
try {
60+
await fs.mkdir(packagePath)
61+
await fs.writeFile(
62+
path.join(packagePath, 'package.json'),
63+
JSON.stringify({
64+
name: 'local-fixture',
65+
version: '1.0.0',
66+
main: 'index.js',
67+
}),
68+
)
69+
await fs.writeFile(
70+
path.join(packagePath, 'index.js'),
71+
'module.exports = 42\n',
72+
)
73+
74+
await InstallationUtils.installWithClient(
75+
packagePath,
76+
installPath,
77+
{ client: 'npm', isLocal: true, installTimeout: 30_000 },
78+
'npm',
79+
)
80+
81+
const installedPackage = JSON.parse(
82+
await fs.readFile(
83+
path.join(
84+
installPath,
85+
'node_modules',
86+
'local-fixture',
87+
'package.json',
88+
),
89+
'utf8',
90+
),
91+
)
92+
expect(installedPackage.name).toBe('local-fixture')
93+
} finally {
94+
await InstallationUtils.cleanupPath(installPath)
95+
await fs.rm(temporaryDirectory, { recursive: true, force: true })
96+
}
97+
})
98+
})

0 commit comments

Comments
 (0)