Skip to content

Commit e2a4216

Browse files
jdaltonjdalton
committed
Handle install errors in fix command
1 parent f7783fb commit e2a4216

File tree

3 files changed

+149
-53
lines changed

3 files changed

+149
-53
lines changed

‎src/commands/fix/npm-fix.mts‎

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -64,16 +64,19 @@ type InstallOptions = {
6464
async function install(
6565
arb: SafeArborist,
6666
options: InstallOptions,
67-
): Promise<SafeNode> {
67+
): Promise<SafeNode | null> {
6868
const { cwd = process.cwd() } = {
6969
__proto__: null,
7070
...options,
7171
} as InstallOptions
72-
const newArb = new Arborist({ path: cwd })
73-
newArb.idealTree = await arb.buildIdealTree()
74-
const actualTree = await newArb.reify()
75-
arb.actualTree = actualTree
76-
return actualTree
72+
try {
73+
const newArb = new Arborist({ path: cwd })
74+
newArb.idealTree = await arb.buildIdealTree()
75+
const actualTree = await newArb.reify()
76+
arb.actualTree = actualTree
77+
return actualTree
78+
} catch {}
79+
return null
7780
}
7881

7982
export async function npmFix(
@@ -124,7 +127,7 @@ export async function npmFix(
124127
const infoByPkgName = getCveInfoFromAlertsMap(alertsMap, { limit })
125128
if (!infoByPkgName) {
126129
spinner?.stop()
127-
logger.info('No fixable vulnerabilities found.')
130+
logger.info('No fixable vulns found.')
128131
return
129132
}
130133

@@ -148,6 +151,14 @@ export async function npmFix(
148151
pkgEnvDetails.editablePkgJson.filename!,
149152
]
150153

154+
const handleInstallFail = () => {
155+
logger.error(
156+
`Unexpected condition: ${pkgEnvDetails.agent} install failed.\n`,
157+
)
158+
logger.dedent()
159+
spinner?.dedent()
160+
}
161+
151162
spinner?.stop()
152163

153164
let count = 0
@@ -163,7 +174,7 @@ export async function npmFix(
163174
const isLastInfoEntry = i === length - 1
164175
const { 0: name, 1: infos } = sortedInfoEntries[i]!
165176

166-
logger.log(`Processing vulnerable package: ${name}`)
177+
logger.log(`Processing vulns for ${name}:`)
167178
logger.indent()
168179
spinner?.indent()
169180

@@ -258,7 +269,7 @@ export async function npmFix(
258269

259270
if (!(newVersion && newVersionPackument)) {
260271
warningsForAfter.add(
261-
`No update applied. ${oldId} needs >=${firstPatchedVersionIdentifier}.`,
272+
`No update applied: ${oldId} requires >=${firstPatchedVersionIdentifier}`,
262273
)
263274
continue infosLoop
264275
}
@@ -311,13 +322,18 @@ export async function npmFix(
311322
let errored = false
312323
try {
313324
// eslint-disable-next-line no-await-in-loop
314-
actualTree = await install(arb, { cwd })
315-
if (test) {
316-
spinner?.info(`Testing ${newId} in ${workspaceName}.`)
317-
// eslint-disable-next-line no-await-in-loop
318-
await runScript(testScript, [], { spinner, stdio: 'ignore' })
325+
const maybeActualTree = await install(arb, { cwd })
326+
if (maybeActualTree) {
327+
actualTree = maybeActualTree
328+
if (test) {
329+
spinner?.info(`Testing ${newId} in ${workspaceName}.`)
330+
// eslint-disable-next-line no-await-in-loop
331+
await runScript(testScript, [], { spinner, stdio: 'ignore' })
332+
}
333+
spinner?.success(`Fixed ${name} in ${workspaceName}.`)
334+
} else {
335+
errored = true
319336
}
320-
spinner?.success(`Fixed ${name} in ${workspaceName}.`)
321337
} catch (e) {
322338
errored = true
323339
error = e
@@ -382,7 +398,13 @@ export async function npmFix(
382398
// eslint-disable-next-line no-await-in-loop
383399
await gitResetAndClean(baseBranch, cwd)
384400
// eslint-disable-next-line no-await-in-loop
385-
actualTree = await install(arb, { cwd })
401+
const maybeActualTree = await install(arb, { cwd })
402+
if (!maybeActualTree) {
403+
// Exit early if install fails.
404+
handleInstallFail()
405+
return
406+
}
407+
actualTree = maybeActualTree
386408
continue infosLoop
387409
}
388410

@@ -447,10 +469,17 @@ export async function npmFix(
447469
}
448470

449471
if (isCi) {
472+
spinner?.start()
450473
// eslint-disable-next-line no-await-in-loop
451474
await gitResetAndClean(baseBranch, cwd)
452475
// eslint-disable-next-line no-await-in-loop
453-
actualTree = await install(arb, { cwd })
476+
const maybeActualTree = await install(arb, { cwd })
477+
spinner?.stop()
478+
if (maybeActualTree) {
479+
actualTree = maybeActualTree
480+
} else {
481+
errored = true
482+
}
454483
}
455484
if (errored) {
456485
if (!isCi) {
@@ -462,8 +491,14 @@ export async function npmFix(
462491
editablePkgJson.save({ ignoreWhitespace: true }),
463492
])
464493
// eslint-disable-next-line no-await-in-loop
465-
actualTree = await install(arb, { cwd })
494+
const maybeActualTree = await install(arb, { cwd })
466495
spinner?.stop()
496+
if (!maybeActualTree) {
497+
// Exit early if install fails.
498+
handleInstallFail()
499+
return
500+
}
501+
actualTree = maybeActualTree
467502
}
468503
logger.fail(
469504
`Update failed for ${oldId} in ${workspaceName}.`,

‎src/commands/fix/pnpm-fix.mts‎

Lines changed: 94 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -85,25 +85,28 @@ type InstallOptions = {
8585
async function install(
8686
pkgEnvDetails: EnvDetails,
8787
options: InstallOptions,
88-
): Promise<SafeNode> {
88+
): Promise<SafeNode | null> {
8989
const { args, cwd, spinner } = {
9090
__proto__: null,
9191
...options,
9292
} as InstallOptions
93-
await runAgentInstall(pkgEnvDetails, {
94-
args: [
95-
...(args ?? []),
96-
// Enable pnpm updates to pnpm-lock.yaml in CI environments.
97-
// https://pnpm.io/cli/install#--frozen-lockfile
98-
'--no-frozen-lockfile',
99-
// Enable a non-interactive pnpm install
100-
// https://github.com/pnpm/pnpm/issues/6778
101-
'--config.confirmModulesPurge=false',
102-
],
103-
spinner,
104-
stdio: isDebug() ? 'inherit' : 'ignore',
105-
})
106-
return await getActualTree(cwd)
93+
try {
94+
await runAgentInstall(pkgEnvDetails, {
95+
args: [
96+
...(args ?? []),
97+
// Enable pnpm updates to pnpm-lock.yaml in CI environments.
98+
// https://pnpm.io/cli/install#--frozen-lockfile
99+
'--no-frozen-lockfile',
100+
// Enable a non-interactive pnpm install
101+
// https://github.com/pnpm/pnpm/issues/6778
102+
'--config.confirmModulesPurge=false',
103+
],
104+
spinner,
105+
stdio: isDebug() ? 'inherit' : 'ignore',
106+
})
107+
return await getActualTree(cwd)
108+
} catch {}
109+
return null
107110
}
108111

109112
export async function pnpmFix(
@@ -135,8 +138,11 @@ export async function pnpmFix(
135138

136139
// If pnpm-lock.yaml does NOT exist then install with pnpm to create it.
137140
if (!lockfile) {
138-
actualTree = await install(pkgEnvDetails, { cwd, spinner })
139-
lockfile = await readPnpmLockfile(lockfilePath)
141+
const maybeActualTree = await install(pkgEnvDetails, { cwd, spinner })
142+
if (maybeActualTree) {
143+
actualTree = maybeActualTree
144+
lockfile = await readPnpmLockfile(lockfilePath)
145+
}
140146
}
141147
// Update pnpm-lock.yaml if its version is older than what the installed pnpm
142148
// produces.
@@ -145,12 +151,15 @@ export async function pnpmFix(
145151
pkgEnvDetails.agentVersion.major >= 10 &&
146152
parsePnpmLockfileVersion(lockfile.lockfileVersion).major <= 6
147153
) {
148-
actualTree = await install(pkgEnvDetails, {
154+
const maybeActualTree = await install(pkgEnvDetails, {
149155
args: ['--lockfile-only'],
150156
cwd,
151157
spinner,
152158
})
153-
lockfile = await readPnpmLockfile(lockfilePath)
159+
if (maybeActualTree) {
160+
actualTree = maybeActualTree
161+
lockfile = await readPnpmLockfile(lockfilePath)
162+
}
154163
}
155164

156165
// Exit early if pnpm-lock.yaml is not found.
@@ -179,7 +188,7 @@ export async function pnpmFix(
179188
const infoByPkgName = getCveInfoFromAlertsMap(alertsMap, { limit })
180189
if (!infoByPkgName) {
181190
spinner?.stop()
182-
logger.info('No fixable vulnerabilities found.')
191+
logger.info('No fixable vulns found.')
183192
return
184193
}
185194

@@ -203,6 +212,14 @@ export async function pnpmFix(
203212
pkgEnvDetails.editablePkgJson.filename!,
204213
]
205214

215+
const handleInstallFail = () => {
216+
logger.error(
217+
`Unexpected condition: ${pkgEnvDetails.agent} install failed.\n`,
218+
)
219+
logger.dedent()
220+
spinner?.dedent()
221+
}
222+
206223
spinner?.stop()
207224

208225
let count = 0
@@ -218,7 +235,7 @@ export async function pnpmFix(
218235
const isLastInfoEntry = i === length - 1
219236
const { 0: name, 1: infos } = sortedInfoEntries[i]!
220237

221-
logger.log(`Processing vulnerable package: ${name}`)
238+
logger.log(`Processing vulns for ${name}:`)
222239
logger.indent()
223240
spinner?.indent()
224241

@@ -254,11 +271,19 @@ export async function pnpmFix(
254271

255272
// actualTree may not be defined on the first iteration of pkgJsonPathsLoop.
256273
if (!actualTree) {
257-
actualTree = existsSync(path.join(rootPath, 'node_modules'))
274+
const maybeActualTree = existsSync(path.join(rootPath, 'node_modules'))
258275
? // eslint-disable-next-line no-await-in-loop
259276
await getActualTree(cwd)
260277
: // eslint-disable-next-line no-await-in-loop
261278
await install(pkgEnvDetails, { cwd, spinner })
279+
if (maybeActualTree) {
280+
actualTree = maybeActualTree
281+
}
282+
}
283+
if (!actualTree) {
284+
// Exit early if install fails.
285+
handleInstallFail()
286+
return
262287
}
263288

264289
const oldVersions = arrayUnique(
@@ -329,7 +354,7 @@ export async function pnpmFix(
329354

330355
if (!(newVersion && newVersionPackument)) {
331356
warningsForAfter.add(
332-
`No update applied. ${oldId} needs >=${firstPatchedVersionIdentifier}.`,
357+
`No update applied: ${oldId} requires >=${firstPatchedVersionIdentifier}`,
333358
)
334359
continue infosLoop
335360
}
@@ -414,13 +439,21 @@ export async function pnpmFix(
414439
let errored = false
415440
try {
416441
// eslint-disable-next-line no-await-in-loop
417-
actualTree = await install(pkgEnvDetails, { cwd, spinner })
418-
if (test) {
419-
spinner?.info(`Testing ${newId} in ${workspaceName}.`)
420-
// eslint-disable-next-line no-await-in-loop
421-
await runScript(testScript, [], { spinner, stdio: 'ignore' })
442+
const maybeActualTree = await install(pkgEnvDetails, {
443+
cwd,
444+
spinner,
445+
})
446+
if (maybeActualTree) {
447+
actualTree = maybeActualTree
448+
if (test) {
449+
spinner?.info(`Testing ${newId} in ${workspaceName}.`)
450+
// eslint-disable-next-line no-await-in-loop
451+
await runScript(testScript, [], { spinner, stdio: 'ignore' })
452+
}
453+
spinner?.success(`Fixed ${name} in ${workspaceName}.`)
454+
} else {
455+
errored = true
422456
}
423-
spinner?.success(`Fixed ${name} in ${workspaceName}.`)
424457
} catch (e) {
425458
error = e
426459
errored = true
@@ -483,7 +516,16 @@ export async function pnpmFix(
483516
// eslint-disable-next-line no-await-in-loop
484517
await gitResetAndClean(baseBranch, cwd)
485518
// eslint-disable-next-line no-await-in-loop
486-
actualTree = await install(pkgEnvDetails, { cwd, spinner })
519+
const maybeActualTree = await install(pkgEnvDetails, {
520+
cwd,
521+
spinner,
522+
})
523+
if (!maybeActualTree) {
524+
// Exit early if install fails.
525+
handleInstallFail()
526+
return
527+
}
528+
actualTree = maybeActualTree
487529
continue infosLoop
488530
}
489531

@@ -548,10 +590,20 @@ export async function pnpmFix(
548590
}
549591

550592
if (isCi) {
593+
spinner?.start()
551594
// eslint-disable-next-line no-await-in-loop
552595
await gitResetAndClean(baseBranch, cwd)
553596
// eslint-disable-next-line no-await-in-loop
554-
actualTree = await install(pkgEnvDetails, { cwd, spinner })
597+
const maybeActualTree = await install(pkgEnvDetails, {
598+
cwd,
599+
spinner,
600+
})
601+
spinner?.stop()
602+
if (maybeActualTree) {
603+
actualTree = maybeActualTree
604+
} else {
605+
errored = true
606+
}
555607
}
556608
if (errored) {
557609
if (!isCi) {
@@ -563,12 +615,21 @@ export async function pnpmFix(
563615
editablePkgJson.save({ ignoreWhitespace: true }),
564616
])
565617
// eslint-disable-next-line no-await-in-loop
566-
actualTree = await install(pkgEnvDetails, { cwd, spinner })
618+
const maybeActualTree = await install(pkgEnvDetails, {
619+
cwd,
620+
spinner,
621+
})
567622
spinner?.stop()
623+
if (!maybeActualTree) {
624+
// Exit early if install fails.
625+
handleInstallFail()
626+
return
627+
}
628+
actualTree = maybeActualTree
568629
}
569630
logger.fail(
570631
`Update failed for ${oldId} in ${workspaceName}.`,
571-
error,
632+
...(error ? [error] : []),
572633
)
573634
}
574635
if (++count >= limit) {

‎src/utils/socket-package-alert.mts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,13 +345,13 @@ export function getCveInfoFromAlertsMap(
345345
alertsMapLoop: for (const [pkgId, sockPkgAlerts] of alertsMap) {
346346
const purlObj = PackageURL.fromString(idToPurl(pkgId))
347347
const name = resolvePackageName(purlObj)
348-
for (const sockPkgAlert of sockPkgAlerts) {
348+
sockPkgAlertsLoop: for (const sockPkgAlert of sockPkgAlerts) {
349349
const alert = sockPkgAlert.raw
350350
if (
351351
alert.fix?.type !== ALERT_FIX_TYPE.cve ||
352352
(exclude.upgradable && getManifestData(NPM, name))
353353
) {
354-
continue
354+
continue sockPkgAlertsLoop
355355
}
356356
if (!infoByPkgName) {
357357
infoByPkgName = new Map()

0 commit comments

Comments
 (0)