Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions packages/aws-cdk/lib/cli/util/npm.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,30 @@
import { exec as _exec } from 'child_process';
import { promisify } from 'util';
import * as semver from 'semver';
import { debug } from '../../logging';
import { ToolkitError } from '../../toolkit/error';

const exec = promisify(_exec);

/* istanbul ignore next: not called during unit tests */
export async function getLatestVersionFromNpm(): Promise<string> {
const { stdout, stderr } = await exec('npm view aws-cdk version', { timeout: 3000 });
if (stderr && stderr.trim().length > 0) {
debug(`The 'npm view' command generated an error stream with content [${stderr.trim()}]`);
export async function execNpmView(currentVersion: string) {
const [latestResult, currentResult] = await Promise.all([
exec('npm view aws-cdk@latest version', { timeout: 3000 })
.catch(err => {
throw new ToolkitError(`Failed to fetch latest version info: ${err.message}`);
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super up on Promises syntax. Is catch with a throw inside the right way to do this?

Where are these errors handled?

Copy link
Contributor Author

@tttol tttol Mar 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is catch with a throw inside the right way to do this?

It’s not the wrong way in TypeScript, but this implementation is a little confusing. I used a single catch to handle all thrown exceptions.

exec(`npm view aws-cdk@${currentVersion} name version deprecated --json`, { timeout: 3000 })
.catch(err => {
throw new ToolkitError(`Failed to fetch current version(${currentVersion}) info: ${err.message}`);
})
]);

if (latestResult.stderr && latestResult.stderr.trim().length > 0) {
throw new ToolkitError(`npm view command failed: ${latestResult.stderr.trim()}`);
}
const latestVersion = stdout.trim();
if (!semver.valid(latestVersion)) {
throw new ToolkitError(`npm returned an invalid semver ${latestVersion}`);
}

return latestVersion;

const latestVersion = latestResult.stdout;
const currentInfo = JSON.parse(currentResult.stdout);

return {
latestVersion: latestVersion,
deprecated: currentInfo.deprecated
};
}
42 changes: 19 additions & 23 deletions packages/aws-cdk/lib/cli/version.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/* istanbul ignore file */
import * as path from 'path';
import * as chalk from 'chalk';
import * as fs from 'fs-extra';
import * as path from 'path';
import * as semver from 'semver';
import { debug, info } from '../logging';
import { ToolkitError } from '../toolkit/error';
import { cdkCacheDir } from '../util';
import { cliRootDir } from './root-dir';
import { formatAsBanner } from './util/console-formatters';
import { getLatestVersionFromNpm } from './util/npm';
import { execNpmView } from './util/npm';

const ONE_DAY_IN_SECONDS = 1 * 24 * 60 * 60;

Expand Down Expand Up @@ -84,20 +84,27 @@ export class VersionCheckTTL {

// Export for unit testing only.
// Don't use directly, use displayVersionMessage() instead.
export async function latestVersionIfHigher(currentVersion: string, cacheFile: VersionCheckTTL): Promise<string|null> {
export async function getVersionMessages(currentVersion: string, cacheFile: VersionCheckTTL): Promise<string[]> {
if (!(await cacheFile.hasExpired())) {
return null;
return [];
}

const latestVersion = await getLatestVersionFromNpm();
const isNewer = semver.gt(latestVersion, currentVersion);
const packageInfo = await execNpmView(currentVersion);
const latestVersion = packageInfo.latestVersion;
await cacheFile.update(latestVersion);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably stick both fields into the cachefile? Both latestVersion and deprecated?

The entire packageInfo object therefore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the logic to cache the entire packageInfo.


if (isNewer) {
return latestVersion;
} else {
return null;
if (semver.eq(latestVersion, currentVersion)) {
return [];
}
Comment on lines +97 to 99
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only correct if you assume the latest pointer never points to a deprecated version. If that's something you assume, you should put it in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the comment 👍


const versionMessage = [
`${chalk.red(packageInfo.deprecated as string)}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this string ever be empty so that the filter below removes it? I have a feeling it would contain ANSI escape characters without characters in between? Have you tested?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly. I changed it to check if packageInfo.deprecated is undefined before calling chalk.red.

`Newer version of CDK is available [${chalk.green(latestVersion as string)}]`,
getMajorVersionUpgradeMessage(currentVersion),
'Upgrade recommended (npm install -g aws-cdk)',
].filter(Boolean) as string[];

return versionMessage;
}

function getMajorVersionUpgradeMessage(currentVersion: string): string | void {
Expand All @@ -107,25 +114,14 @@ function getMajorVersionUpgradeMessage(currentVersion: string): string | void {
}
}

function getVersionMessage(currentVersion: string, laterVersion: string): string[] {
return [
`Newer version of CDK is available [${chalk.green(laterVersion as string)}]`,
getMajorVersionUpgradeMessage(currentVersion),
'Upgrade recommended (npm install -g aws-cdk)',
].filter(Boolean) as string[];
}

export async function displayVersionMessage(currentVersion = versionNumber(), versionCheckCache?: VersionCheckTTL): Promise<void> {
if (!process.stdout.isTTY || process.env.CDK_DISABLE_VERSION_CHECK) {
return;
}

try {
const laterVersion = await latestVersionIfHigher(currentVersion, versionCheckCache ?? new VersionCheckTTL());
if (laterVersion) {
const bannerMsg = formatAsBanner(getVersionMessage(currentVersion, laterVersion));
bannerMsg.forEach((e) => info(e));
}
const versionMessages = await getVersionMessages(currentVersion, versionCheckCache ?? new VersionCheckTTL());
formatAsBanner(versionMessages).forEach(e => info(e));
} catch (err: any) {
debug(`Could not run version check - ${err.message}`);
}
Expand Down
37 changes: 20 additions & 17 deletions packages/aws-cdk/lib/legacy-exports-source.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make sure no other changes than the removal are in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed unnecessary changes.

Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,30 @@
// See https://github.com/aws/aws-cdk/pull/33021 for more information.

// Note: All type exports are in `legacy-exports.ts`
export * from './legacy-logging-source';
export { deepClone, flatten, ifDefined, isArray, isEmpty, numberFromBool, partition, padLeft as leftPad, contentHash, deepMerge } from './util';
export { deployStack } from './api/deployments/deploy-stack';
export { cli, exec } from './cli/cli';
export { SdkProvider } from './api/aws-auth';
export { PluginHost } from './api/plugin';
export { Command, Configuration, PROJECT_CONTEXT } from './cli/user-configuration';
export { Settings } from './api/settings';
export { AwsCliCompatible } from './api/aws-auth/awscli-compatible';
export { cached } from './api/aws-auth/cached';
export { CredentialPlugins } from './api/aws-auth/credential-plugins';
export { setSdkTracing as enableTracing } from './api/aws-auth/tracing';
export { Bootstrapper } from './api/bootstrap';
export { CloudExecutable } from './api/cxapp/cloud-executable';
export { execProgram } from './api/cxapp/exec';
export { RequireApproval } from './diff';
export { Deployments } from './api/deployments';
export { deployStack } from './api/deployments/deploy-stack';
export { CfnEvaluationException } from './api/evaluate-cloudformation-template';
export { lowerCaseFirstCharacter } from './api/hotswap/common';
export { PluginHost } from './api/plugin';
export { Settings } from './api/settings';
export { cli, exec } from './cli/cli';
export { Command, Configuration, PROJECT_CONTEXT } from './cli/user-configuration';
export { formatAsBanner } from './cli/util/console-formatters';
export { setSdkTracing as enableTracing } from './api/aws-auth/tracing';
export { getVersionMessages as latestVersionIfHigher, versionNumber } from './cli/version';
export { aliases, command, describe } from './commands/docs';
export { lowerCaseFirstCharacter } from './api/hotswap/common';
export { Deployments } from './api/deployments';
export { cliRootDir as rootDir } from './cli/root-dir';
export { latestVersionIfHigher, versionNumber } from './cli/version';
export { RequireApproval } from './diff';
export { availableInitTemplates } from './init';
export { cached } from './api/aws-auth/cached';
export { CfnEvaluationException } from './api/evaluate-cloudformation-template';
export { CredentialPlugins } from './api/aws-auth/credential-plugins';
export { AwsCliCompatible } from './api/aws-auth/awscli-compatible';
export * from './legacy-logging-source';
export { deepClone, flatten, ifDefined, isArray, isEmpty, numberFromBool, partition } from './util';
export { contentHash } from './util/content-hash';
export { cliRootDir as rootDir } from './cli/root-dir';
export { deepMerge } from './util/objects';
export { leftPad } from './util/string-manipulation';
101 changes: 101 additions & 0 deletions packages/aws-cdk/test/cli/util/npm.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { execNpmView } from '../../../lib/cli/util/npm';

jest.mock('util', () => {
const mockExec = jest.fn();
const format = jest.fn((fmt, ...args) => {
return [fmt, ...args].join(' ');
});
return {
promisify: jest.fn(() => mockExec),
__mockExec: mockExec,
format,
};
});

const { __mockExec: mockedExec } = jest.requireMock('util');

describe('npm.ts', () => {
beforeEach(() => {
jest.clearAllMocks();
});

describe('execNpmView', () => {
test('returns latest version and current version info with deprecated message', async () => {
// Set up result for the first call (latest version)
mockedExec.mockImplementationOnce((cmd, options) => {
expect(cmd).toBe('npm view aws-cdk@latest version');
expect(options).toEqual({ timeout: 3000 });
return Promise.resolve({
stdout: '2.0.0\n',
stderr: '',
});
});

// Set up result for the second call (current version)
mockedExec.mockImplementationOnce((cmd, options) => {
expect(cmd).toBe('npm view aws-cdk@1.0.0 name version deprecated --json');
expect(options).toEqual({ timeout: 3000 });
return Promise.resolve({
stdout: '{"version": "1.0.0","deprecated": "This version has been deprecated.", "name": "aws-cdk"}',
stderr: '',
});
});

const result = await execNpmView('1.0.0');

expect(result).toEqual({
latestVersion: '2.0.0',
currentVersion: '1.0.0',
deprecated: 'This version has been deprecated.',
});

expect(mockedExec).toHaveBeenCalledTimes(2);
});

test('returns latest version and current version info without deprecated field', async () => {
// Set up result for the first call (latest version)
mockedExec.mockImplementationOnce(() => Promise.resolve({
stdout: '2.1000.0\n',
stderr: '',
}));

// Set up result for the second call (current version)
mockedExec.mockImplementationOnce(() => Promise.resolve({
stdout: '{"version": "2.179.0", "name": "aws-cdk"}',
stderr: '',
}));

const result = await execNpmView('2.179.0');

expect(result).toEqual({
latestVersion: '2.1000.0',
currentVersion: '2.179.0',
deprecated: undefined,
});
});

test('throws error when latest version npm command fails', async () => {
// Trigger error for the first call (latest version)
mockedExec.mockImplementationOnce(() =>
Promise.reject(new Error('npm ERR! code E404\nnpm ERR! 404 Not Found'))
);

await expect(execNpmView('1.0.0')).rejects.toThrow('Failed to fetch latest version info');
});

test('throws error when current version npm command fails', async () => {
// Set up result for the first call (latest version)
mockedExec.mockImplementationOnce(() => Promise.resolve({
stdout: '2.0.0\n',
stderr: '',
}));

// Trigger error for the second call (current version)
mockedExec.mockImplementationOnce(() =>
Promise.reject(new Error('npm ERR! code E404\nnpm ERR! 404 Not Found'))
);

await expect(execNpmView('1.0.0')).rejects.toThrow('Failed to fetch current version');
});
});
});
Loading
Loading