mirror of
https://github.com/pnpm/pnpm.git
synced 2026-05-12 10:11:42 -04:00
fix(config): gracefully handle configDependencies errors during config commands (#11362)
Closes #10684. Running `pnpm config set` triggers `resolveAndInstallConfigDeps` before the new config (e.g. auth token) is saved to `.npmrc`, causing a 401 crash when the config dependency is hosted in a private registry. This patch adds a `tolerateConfigDependenciesErrors` flag to `installConfigDepsAndLoadHooks`. For `cmd === 'config'`, `'set'` and `'get'`, installation failures are now caught and logged at debug level so the command can proceed and actually write / read the auth token. `pnpm set` and `pnpm get` are separate top-level commands whose handlers delegate to the `config` command internally, so they are explicitly listed alongside `'config'`; users can hit #10684 via any of the three entry points. Plugin pnpmfile paths from `configDependencies` are resolved by checking each file's existence on disk, so previously-installed hooks survive a transient install failure and `requireHooks` won't throw `PNPMFILE_NOT_FOUND` when nothing has been installed yet. Covers the fix with: - Unit tests in `pnpm/test/getConfig.test.ts` exercising the `installConfigDepsAndLoadHooks` contract (success, tolerated error, rethrown error, store creation/close errors propagating) and the on-disk pnpmfile resolution behavior. - Four e2e tests in `pnpm/test/configurationalDependencies.test.ts` that spawn the pnpm CLI against a bogus `configDependency` and assert each entrypoint (`pnpm config set`, `pnpm config get`, `pnpm set`, `pnpm get`) still works. --------- Co-authored-by: Zoltan Kochan <z@kochan.io>
This commit is contained in:
committed by
GitHub
parent
75f4af9451
commit
0c137bbbe0
5
.changeset/major-hats-press.md
Normal file
5
.changeset/major-hats-press.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pnpm": patch
|
||||
---
|
||||
|
||||
Prevent crashes during `pnpm config`, `pnpm set`, and `pnpm get` by tolerating `configDependencies` install failures. For these commands, a failure to install `configDependencies` (for example because the registry auth token has not been written yet) is now logged at debug level and the command proceeds. All other commands still surface the install error [#10684](https://github.com/pnpm/pnpm/issues/10684).
|
||||
@@ -1,11 +1,13 @@
|
||||
import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import util from 'node:util'
|
||||
|
||||
import { formatWarn } from '@pnpm/cli.default-reporter'
|
||||
import { packageManager } from '@pnpm/cli.meta'
|
||||
import { type CliOptions, type Config, type ConfigContext, getConfig as _getConfig } from '@pnpm/config.reader'
|
||||
import { requireHooks } from '@pnpm/hooks.pnpmfile'
|
||||
import { resolveAndInstallConfigDeps } from '@pnpm/installing.env-installer'
|
||||
import { logger } from '@pnpm/logger'
|
||||
import { createStoreController } from '@pnpm/store.connection-manager'
|
||||
import type { ConfigDependencies } from '@pnpm/types'
|
||||
import { lexCompare } from '@pnpm/util.lex-comparator'
|
||||
@@ -42,7 +44,10 @@ export async function getConfig (
|
||||
|
||||
export async function installConfigDepsAndLoadHooks (
|
||||
config: Config,
|
||||
context: ConfigContext
|
||||
context: ConfigContext,
|
||||
opts?: {
|
||||
tolerateConfigDependenciesErrors?: boolean
|
||||
}
|
||||
): Promise<{ config: Config, context: ConfigContext }> {
|
||||
if (config.configDependencies) {
|
||||
const store = await createStoreController({ ...config, ...context })
|
||||
@@ -55,6 +60,15 @@ export async function installConfigDepsAndLoadHooks (
|
||||
rootDir: config.lockfileDir ?? context.rootProjectManifestDir,
|
||||
frozenLockfile: config.frozenLockfile,
|
||||
})
|
||||
} catch (err: unknown) {
|
||||
if (!opts?.tolerateConfigDependenciesErrors) {
|
||||
throw err
|
||||
}
|
||||
const errorMessage = util.types.isNativeError(err) ? err.message : String(err)
|
||||
logger.debug({
|
||||
message: `Failed to install configDependencies. This is expected if authentication is not yet configured. Proceeding. Error: ${errorMessage}`,
|
||||
err,
|
||||
})
|
||||
} finally {
|
||||
await store.ctrl.close()
|
||||
}
|
||||
@@ -87,12 +101,18 @@ export async function installConfigDepsAndLoadHooks (
|
||||
export function * calcPnpmfilePathsOfPluginDeps (configModulesDir: string, configDependencies: ConfigDependencies): Generator<string> {
|
||||
for (const configDepName of Object.keys(configDependencies).sort(lexCompare)) {
|
||||
if (isPluginName(configDepName)) {
|
||||
const mjsPath = path.join(configModulesDir, configDepName, 'pnpmfile.mjs')
|
||||
const pluginDir = path.join(configModulesDir, configDepName)
|
||||
// If the plugin directory itself is missing, the install didn't run
|
||||
// (or hasn't run yet) — skip silently. If the plugin directory exists
|
||||
// but contains no pnpmfile, fall through to yield the .cjs path so
|
||||
// requireHooks surfaces PNPMFILE_NOT_FOUND for the misconfigured plugin.
|
||||
if (!fs.existsSync(pluginDir)) continue
|
||||
const mjsPath = path.join(pluginDir, 'pnpmfile.mjs')
|
||||
if (fs.existsSync(mjsPath)) {
|
||||
yield mjsPath
|
||||
} else {
|
||||
yield path.join(configModulesDir, configDepName, 'pnpmfile.cjs')
|
||||
continue
|
||||
}
|
||||
yield path.join(pluginDir, 'pnpmfile.cjs')
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -125,7 +125,13 @@ export async function main (inputArgv: string[]): Promise<void> {
|
||||
}
|
||||
}
|
||||
}
|
||||
;({ config, context } = await installConfigDepsAndLoadHooks(config, context) as { config: typeof config, context: ConfigContext })
|
||||
// `pnpm set` / `pnpm get` are separate top-level commands whose handlers
|
||||
// delegate to the `config` command internally. They are not rewritten to
|
||||
// `cmd === 'config'` at this layer, so list them explicitly — users can
|
||||
// hit the #10684 crash via any of these three entry points.
|
||||
;({ config, context } = await installConfigDepsAndLoadHooks(config, context, {
|
||||
tolerateConfigDependenciesErrors: cmd === 'config' || cmd === 'set' || cmd === 'get',
|
||||
}) as { config: typeof config, context: ConfigContext })
|
||||
if (isDlxOrCreateCommand || cmd === 'sbom' || cmd === 'with') {
|
||||
config.useStderr = true
|
||||
}
|
||||
|
||||
@@ -212,3 +212,68 @@ test('installing a new configurational dependency', async () => {
|
||||
getIntegrity('@pnpm.e2e/foo', '100.0.0')
|
||||
)
|
||||
})
|
||||
|
||||
// Regression tests for https://github.com/pnpm/pnpm/issues/10684 — if the user
|
||||
// has a configDependency stored in a registry that needs auth, the config
|
||||
// commands must not crash when pnpm tries to fetch the configDependency before
|
||||
// the new setting is written. We reference a non-existent package version so
|
||||
// the install errors out fast; the real-world scenario is a 401 from the
|
||||
// private registry.
|
||||
//
|
||||
// All four entry points are tested: `pnpm config set`, `pnpm config get`,
|
||||
// `pnpm set`, and `pnpm get`. The latter two are shortcuts that delegate to
|
||||
// the config handler internally but are separate top-level commands, so they
|
||||
// need their own coverage at the main.ts guard level.
|
||||
function writeFailingConfigDep () {
|
||||
// Clean specifier for a version that does not exist on the mock registry.
|
||||
// fetchRetries: 0 keeps the failure fast so the test does not time out.
|
||||
writeYamlFileSync('pnpm-workspace.yaml', {
|
||||
configDependencies: {
|
||||
'@pnpm.e2e/foo': '999.999.999',
|
||||
},
|
||||
fetchRetries: 0,
|
||||
})
|
||||
}
|
||||
|
||||
test('pnpm config set succeeds even when configDependencies fail to install', async () => {
|
||||
prepare()
|
||||
writeFailingConfigDep()
|
||||
|
||||
// Use an auth-style key so the setting lands in ./.npmrc (project scope).
|
||||
const authKey = '//example.com/:_authToken'
|
||||
await execPnpm(['config', 'set', '--location=project', authKey, 'my-secret-token'])
|
||||
|
||||
const npmrc = fs.readFileSync('.npmrc', 'utf8')
|
||||
expect(npmrc).toContain(`${authKey}=my-secret-token`)
|
||||
})
|
||||
|
||||
test('pnpm config get succeeds even when configDependencies fail to install', async () => {
|
||||
prepare()
|
||||
writeFailingConfigDep()
|
||||
const authKey = '//example.com/:_authToken'
|
||||
fs.writeFileSync('.npmrc', `${authKey}=my-secret-token\n`, 'utf8')
|
||||
|
||||
const result = execPnpmSync(['config', 'get', '--location=project', authKey], { expectSuccess: true })
|
||||
expect(result.stdout.toString()).toContain('my-secret-token')
|
||||
})
|
||||
|
||||
test('pnpm set succeeds even when configDependencies fail to install', async () => {
|
||||
prepare()
|
||||
writeFailingConfigDep()
|
||||
|
||||
const authKey = '//example.com/:_authToken'
|
||||
await execPnpm(['set', '--location=project', authKey, 'my-secret-token'])
|
||||
|
||||
const npmrc = fs.readFileSync('.npmrc', 'utf8')
|
||||
expect(npmrc).toContain(`${authKey}=my-secret-token`)
|
||||
})
|
||||
|
||||
test('pnpm get succeeds even when configDependencies fail to install', async () => {
|
||||
prepare()
|
||||
writeFailingConfigDep()
|
||||
const authKey = '//example.com/:_authToken'
|
||||
fs.writeFileSync('.npmrc', `${authKey}=my-secret-token\n`, 'utf8')
|
||||
|
||||
const result = execPnpmSync(['get', '--location=project', authKey], { expectSuccess: true })
|
||||
expect(result.stdout.toString()).toContain('my-secret-token')
|
||||
})
|
||||
|
||||
@@ -3,12 +3,46 @@ import fs from 'node:fs'
|
||||
import path from 'node:path'
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, jest, test } from '@jest/globals'
|
||||
import type { Config, ConfigContext } from '@pnpm/config.reader'
|
||||
import { prepare } from '@pnpm/prepare'
|
||||
|
||||
import { calcPnpmfilePathsOfPluginDeps, getConfig } from '../src/getConfig.js'
|
||||
jest.unstable_mockModule('@pnpm/installing.env-installer', () => ({
|
||||
resolveAndInstallConfigDeps: jest.fn(),
|
||||
}))
|
||||
|
||||
jest.unstable_mockModule('@pnpm/store.connection-manager', () => ({
|
||||
createStoreController: jest.fn(),
|
||||
}))
|
||||
|
||||
const loggerMock = {
|
||||
debug: jest.fn(),
|
||||
info: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
error: jest.fn(),
|
||||
}
|
||||
jest.unstable_mockModule('@pnpm/logger', () => ({
|
||||
logger: Object.assign(jest.fn(() => loggerMock), loggerMock),
|
||||
globalWarn: jest.fn(),
|
||||
globalInfo: jest.fn(),
|
||||
globalError: jest.fn(),
|
||||
}))
|
||||
|
||||
const { resolveAndInstallConfigDeps } = await import('@pnpm/installing.env-installer')
|
||||
const { logger } = await import('@pnpm/logger')
|
||||
const { createStoreController } = await import('@pnpm/store.connection-manager')
|
||||
const { calcPnpmfilePathsOfPluginDeps, getConfig, installConfigDepsAndLoadHooks } = await import('../src/getConfig.js')
|
||||
|
||||
const storeCloseMock = jest.fn<() => Promise<void>>()
|
||||
|
||||
beforeEach(() => {
|
||||
jest.spyOn(console, 'warn')
|
||||
loggerMock.debug.mockClear()
|
||||
jest.mocked(resolveAndInstallConfigDeps).mockReset()
|
||||
storeCloseMock.mockReset().mockResolvedValue(undefined)
|
||||
jest.mocked(createStoreController).mockReset().mockResolvedValue({
|
||||
ctrl: { close: storeCloseMock },
|
||||
dir: '/tmp/store',
|
||||
} as unknown as Awaited<ReturnType<typeof createStoreController>>)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -59,6 +93,29 @@ describe('calcPnpmfilePathsOfPluginDeps', () => {
|
||||
fs.rmSync(tmpDir, { recursive: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('skips plugins whose directory is missing (e.g. config dep install never ran)', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-'))
|
||||
try {
|
||||
const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })]
|
||||
expect(paths).toEqual([])
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('yields pnpmfile.cjs path even if missing when the plugin directory exists (so requireHooks reports the misconfiguration)', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(import.meta.dirname, '.tmp-'))
|
||||
try {
|
||||
const pluginDir = path.join(tmpDir, 'pnpm-plugin-foo')
|
||||
fs.mkdirSync(pluginDir, { recursive: true })
|
||||
|
||||
const paths = [...calcPnpmfilePathsOfPluginDeps(tmpDir, { 'pnpm-plugin-foo': '1.0.0' })]
|
||||
expect(paths).toEqual([path.join(pluginDir, 'pnpmfile.cjs')])
|
||||
} finally {
|
||||
fs.rmSync(tmpDir, { recursive: true })
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
test('hoist: false removes hoistPattern', async () => {
|
||||
@@ -74,3 +131,101 @@ test('hoist: false removes hoistPattern', async () => {
|
||||
expect(config.hoist).toBe(false)
|
||||
expect(config.hoistPattern).toBeUndefined()
|
||||
})
|
||||
|
||||
describe('installConfigDepsAndLoadHooks', () => {
|
||||
test('proceeds normally when configDependencies install succeeds', async () => {
|
||||
prepare()
|
||||
|
||||
jest.mocked(resolveAndInstallConfigDeps).mockResolvedValueOnce(undefined as never)
|
||||
|
||||
const { config, context } = buildBaseConfig()
|
||||
const result = await installConfigDepsAndLoadHooks(config, context)
|
||||
|
||||
expect(result).toBeDefined()
|
||||
expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1)
|
||||
expect(loggerMock.debug).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('does not throw when install fails and tolerateConfigDependenciesErrors is true', async () => {
|
||||
prepare()
|
||||
|
||||
const simulatedError = new Error('401 Unauthorized: missing auth token')
|
||||
jest.mocked(resolveAndInstallConfigDeps).mockRejectedValueOnce(simulatedError)
|
||||
|
||||
const { config, context } = buildBaseConfig()
|
||||
|
||||
const result = await installConfigDepsAndLoadHooks(config, context, {
|
||||
tolerateConfigDependenciesErrors: true,
|
||||
})
|
||||
|
||||
expect(result).toBeDefined()
|
||||
expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1)
|
||||
expect(logger.debug).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
message: expect.stringContaining('Failed to install configDependencies'),
|
||||
err: simulatedError,
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
test('throws when install fails and tolerateConfigDependenciesErrors is not set (default behaviour)', async () => {
|
||||
prepare()
|
||||
|
||||
const simulatedError = new Error('401 Unauthorized: missing auth token')
|
||||
jest.mocked(resolveAndInstallConfigDeps).mockRejectedValueOnce(simulatedError)
|
||||
|
||||
const { config, context } = buildBaseConfig()
|
||||
|
||||
await expect(
|
||||
installConfigDepsAndLoadHooks(config, context)
|
||||
).rejects.toThrow('401 Unauthorized: missing auth token')
|
||||
|
||||
expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
test('does not swallow store creation errors even when tolerateConfigDependenciesErrors is true', async () => {
|
||||
prepare()
|
||||
|
||||
const storeError = new Error('EACCES: permission denied opening store dir')
|
||||
jest.mocked(createStoreController).mockRejectedValueOnce(storeError)
|
||||
|
||||
const { config, context } = buildBaseConfig()
|
||||
|
||||
await expect(
|
||||
installConfigDepsAndLoadHooks(config, context, { tolerateConfigDependenciesErrors: true })
|
||||
).rejects.toThrow('EACCES: permission denied opening store dir')
|
||||
|
||||
expect(resolveAndInstallConfigDeps).not.toHaveBeenCalled()
|
||||
expect(loggerMock.debug).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('does not swallow store close errors even when tolerateConfigDependenciesErrors is true', async () => {
|
||||
prepare()
|
||||
|
||||
jest.mocked(resolveAndInstallConfigDeps).mockResolvedValueOnce(undefined as never)
|
||||
storeCloseMock.mockReset().mockRejectedValueOnce(new Error('store close failed'))
|
||||
|
||||
const { config, context } = buildBaseConfig()
|
||||
|
||||
await expect(
|
||||
installConfigDepsAndLoadHooks(config, context, { tolerateConfigDependenciesErrors: true })
|
||||
).rejects.toThrow('store close failed')
|
||||
|
||||
expect(resolveAndInstallConfigDeps).toHaveBeenCalledTimes(1)
|
||||
expect(loggerMock.debug).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
function buildBaseConfig (): { config: Config, context: ConfigContext } {
|
||||
const dir = process.cwd()
|
||||
const config = {
|
||||
ignorePnpmfile: true,
|
||||
configDependencies: { 'some-helper-pkg': '1.0.0+sha512-abc' },
|
||||
dir,
|
||||
lockfileDir: dir,
|
||||
} as unknown as Config
|
||||
const context = {
|
||||
rootProjectManifestDir: dir,
|
||||
} as unknown as ConfigContext
|
||||
return { config, context }
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user