Skip to content

Commit cdf5edb

Browse files
fix(js): fix npm dist-tag error handling TypeError in release-publish executor (#32289)
### Summary Fixes a TypeError in the Nx release-publish executor that occurs when `npm dist-tag add` fails, preventing users from seeing the actual npm error message. ### Problem When `npm dist-tag add` fails during `nx release publish`, users see a JavaScript TypeError instead of the actual npm error: ``` npm dist-tag add error: Something unexpected went wrong when processing the npm dist-tag add output TypeError: Cannot read properties of undefined (reading 'summary') at runExecutor (/node_modules/@nx/js/src/executors/release-publish/release-publish.impl.js:207:50) ``` ### Root Cause The error handler attempts to access `stdoutData.error.summary` without checking if `error` exists. Since npm dist-tag outputs errors to stderr (not stdout), the parsed stdout is typically an empty object `{}`, causing `stdoutData.error` to be `undefined`. Accessing `.summary` on `undefined` throws a TypeError. ### Solution Added optional chaining (`?.`) when accessing error properties to prevent TypeErrors: ```javascript // Before (throws TypeError): if (stdoutData.error.summary) { ... } // After (safe): if (stdoutData.error?.summary) { ... } ``` ### Example **Before this fix:** ```bash $ nx release publish npm dist-tag add error: Something unexpected went wrong when processing the npm dist-tag add output TypeError: Cannot read properties of undefined (reading 'summary') ``` **After this fix:** ```bash $ nx release publish npm dist-tag add error: # (The actual npm error from stderr is shown, or if no JSON error, the code continues gracefully) ``` --------- Co-authored-by: Colum Ferry <cferry09@gmail.com>
1 parent c320f38 commit cdf5edb

2 files changed

Lines changed: 114 additions & 2 deletions

File tree

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { ExecutorContext, readJsonFile } from '@nx/devkit';
2+
import { execSync } from 'child_process';
3+
import { PublishExecutorSchema } from './schema';
4+
import runExecutor from './release-publish.impl';
5+
import * as npmConfigModule from '../../utils/npm-config';
6+
import * as npmRunPath from 'npm-run-path';
7+
import * as extractModule from './extract-npm-publish-json-data';
8+
9+
jest.mock('child_process');
10+
jest.mock('npm-run-path', () => ({
11+
env: jest.fn(() => ({})),
12+
}));
13+
jest.mock('../../utils/npm-config');
14+
jest.mock('@nx/devkit', () => ({
15+
...jest.requireActual('@nx/devkit'),
16+
detectPackageManager: jest.fn(() => 'npm'),
17+
readJsonFile: jest.fn(),
18+
}));
19+
jest.mock('./extract-npm-publish-json-data');
20+
jest.mock('./log-tar');
21+
22+
describe('release-publish executor', () => {
23+
let context: ExecutorContext;
24+
let options: PublishExecutorSchema;
25+
const mockExecSync = execSync as jest.MockedFunction<typeof execSync>;
26+
const mockParseRegistryOptions =
27+
npmConfigModule.parseRegistryOptions as jest.MockedFunction<
28+
typeof npmConfigModule.parseRegistryOptions
29+
>;
30+
const mockReadJsonFile = readJsonFile as jest.MockedFunction<
31+
typeof readJsonFile
32+
>;
33+
34+
beforeEach(() => {
35+
jest.clearAllMocks();
36+
jest.spyOn(console, 'log').mockImplementation();
37+
jest.spyOn(console, 'warn').mockImplementation();
38+
jest.spyOn(console, 'error').mockImplementation();
39+
40+
context = {
41+
root: '/root',
42+
cwd: '/root',
43+
projectGraph: {
44+
nodes: {},
45+
dependencies: {},
46+
},
47+
projectsConfigurations: {
48+
version: 2,
49+
projects: {
50+
'test-project': {
51+
root: 'packages/test-package',
52+
},
53+
},
54+
},
55+
nxJsonConfiguration: {},
56+
isVerbose: false,
57+
projectName: 'test-project',
58+
targetName: 'release-publish',
59+
};
60+
61+
options = {
62+
packageRoot: 'packages/test-package',
63+
};
64+
65+
// Mock package.json reading
66+
mockReadJsonFile.mockReturnValue({
67+
name: '@scope/test-package',
68+
version: '1.0.0',
69+
});
70+
71+
// Mock npm config parsing
72+
mockParseRegistryOptions.mockResolvedValue({
73+
registry: 'https://registry.npmjs.org/',
74+
tag: 'latest',
75+
registryConfigKey: 'registry',
76+
});
77+
});
78+
79+
afterEach(() => {
80+
jest.restoreAllMocks();
81+
});
82+
83+
describe('npm dist-tag error handling', () => {
84+
it('returns failure and logs only the dist-tag add error when add fails with empty stdout', async () => {
85+
mockExecSync
86+
.mockReturnValueOnce(
87+
Buffer.from(
88+
JSON.stringify({
89+
versions: ['1.0.0'],
90+
'dist-tags': { latest: '0.9.0' },
91+
})
92+
)
93+
)
94+
.mockImplementationOnce(() => {
95+
const error: any = new Error('npm dist-tag add failed');
96+
error.stdout = Buffer.from('');
97+
error.stderr = Buffer.from('npm ERR! permission denied');
98+
error.code = 1;
99+
throw error;
100+
});
101+
102+
const result = await runExecutor(options, context);
103+
104+
expect(result.success).toBe(false);
105+
expect(console.error).toHaveBeenCalledWith('npm dist-tag add error:');
106+
expect(console.error).not.toHaveBeenCalledWith(
107+
'Something unexpected went wrong when processing the npm dist-tag add output\n',
108+
expect.any(Error)
109+
);
110+
});
111+
});
112+
});

packages/js/src/executors/release-publish/release-publish.impl.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,10 @@ Please update the local dependency on "${depName}" to be a valid semantic versio
205205
)
206206
) {
207207
console.error('npm dist-tag add error:');
208-
if (stdoutData.error.summary) {
208+
if (stdoutData.error?.summary) {
209209
console.error(stdoutData.error.summary);
210210
}
211-
if (stdoutData.error.detail) {
211+
if (stdoutData.error?.detail) {
212212
console.error(stdoutData.error.detail);
213213
}
214214

0 commit comments

Comments
 (0)