Skip to content

Commit d1d0633

Browse files
authored
fix(jsii-diff): don't fail on new packages (#502)
When a new package is created, jsii-diff will try to download the previous package from NPM and then fail. Remember that the package doesn't exist upstream and fail silently in that case.
1 parent 7ba1aab commit d1d0633

File tree

2 files changed

+80
-18
lines changed

2 files changed

+80
-18
lines changed

packages/jsii-diff/bin/jsii-diff.ts

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import spec = require('jsii-spec');
44
import log4js = require('log4js');
55
import yargs = require('yargs');
66
import { compareAssemblies } from '../lib';
7-
import { downloadNpmPackage } from '../lib/util';
7+
import { DownloadFailure, downloadNpmPackage, showDownloadFailure } from '../lib/util';
88
import { VERSION } from '../lib/version';
99

1010
const LOG = log4js.getLogger('jsii-diff');
@@ -33,13 +33,24 @@ async function main(): Promise<number> {
3333
configureLog4js(argv.verbose);
3434

3535
LOG.debug(`Loading original assembly from ${(argv as any).original}`);
36-
const original = await loadAssembly((argv as any).original);
36+
const loadOriginal = await loadAssembly((argv as any).original);
37+
if (!loadOriginal.success) {
38+
process.stderr.write(`Could not load '${loadOriginal.resolved}': ${showDownloadFailure(loadOriginal.reason)}. Skipping analysis\n`);
39+
return 0;
40+
}
3741

3842
LOG.debug(`Loading updated assembly from ${(argv as any).updated}`);
39-
const updated = await loadAssembly((argv as any).updated);
43+
const loadUpdated = await loadAssembly((argv as any).updated);
44+
if (!loadUpdated.success) {
45+
process.stderr.write(`Could not load '${loadUpdated.resolved}': ${showDownloadFailure(loadUpdated.reason)}. Skipping analysis\n`);
46+
return 0;
47+
}
48+
49+
const original = loadOriginal.assembly;
50+
const updated = loadUpdated.assembly;
4051

4152
if (original.name !== updated.name) {
42-
process.stderr.write(`Look like different assemblies: '${original.name}' vs '${updated.name}'. Comparing is probably pointless...`);
53+
process.stderr.write(`Look like different assemblies: '${original.name}' vs '${updated.name}'. Comparing is probably pointless...\n`);
4354
}
4455

4556
LOG.info(`Starting analysis`);
@@ -66,27 +77,37 @@ async function main(): Promise<number> {
6677
// Allow both npm:<package> (legacy) and npm://<package> (looks better)
6778
const NPM_REGEX = /^npm:(\/\/)?/;
6879

69-
async function loadAssembly(name: string) {
80+
/**
81+
* Load the indicated assembly from the given name
82+
*
83+
* Supports downloading from NPM as well as from file or directory.
84+
*/
85+
async function loadAssembly(requested: string): Promise<LoadAssemblyResult> {
86+
let resolved = requested;
7087
try {
71-
if (name.match(NPM_REGEX)) {
72-
let pkg = name.replace(NPM_REGEX, '');
88+
if (requested.match(NPM_REGEX)) {
89+
let pkg = requested.replace(NPM_REGEX, '');
7390
if (!pkg) { pkg = await loadPackageNameFromAssembly(); }
7491

75-
// Put 'pkg' back into 'name' so any errors loading the assembly get a good source description
76-
name = `npm://${pkg}`;
77-
if (pkg.indexOf('@', 1) === -1) { name += '@latest'; }
92+
resolved = `npm://${pkg}`;
93+
if (pkg.indexOf('@', 1) === -1) { resolved += '@latest'; }
7894

79-
return await downloadNpmPackage(pkg, loadFromFilesystem);
95+
const download = await downloadNpmPackage(pkg, loadFromFilesystem);
96+
if (download.success) {
97+
return { requested, resolved, success: true, assembly: download.result };
98+
}
99+
return { requested, resolved, success: false, reason: download.reason };
80100
} else {
81-
return await loadFromFilesystem(name);
101+
// We don't accept failure loading from the filesystem
102+
return { requested, resolved, success: true, assembly: await loadFromFilesystem(requested) };
82103
}
83104
} catch (e) {
84105
// Prepend information about which assembly we've failed to load
85106
//
86107
// Look at the type of error. If it has a lot of lines (like validation errors
87108
// tend to do) log everything to the debug log and only show a couple
88109
const maxLines = 3;
89-
const messageWithContext = `Error loading assembly '${name}': ${e.message}`;
110+
const messageWithContext = `Error loading assembly '${resolved}': ${e.message}`;
90111
const errorLines = messageWithContext.split('\n');
91112
if (errorLines.length < maxLines) { throw new Error(messageWithContext); }
92113
for (const line of errorLines) {
@@ -96,6 +117,9 @@ async function loadAssembly(name: string) {
96117
}
97118
}
98119

120+
type LoadAssemblyResult = { requested: string; resolved: string }
121+
& ({ success: true; assembly: reflect.Assembly } | { success: false; reason: DownloadFailure });
122+
99123
async function loadPackageNameFromAssembly(): Promise<string> {
100124
const JSII_ASSEMBLY_FILE = '.jsii';
101125
if (!await fs.pathExists(JSII_ASSEMBLY_FILE)) {

packages/jsii-diff/lib/util.ts

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,57 @@ export async function inTempDir<T>(block: () => T | Promise<T>): Promise<T> {
2121
}
2222
}
2323

24-
export async function downloadNpmPackage<T>(pkg: string, block: (dir: string) => Promise<T>): Promise<T> {
24+
export type DownloadFailure = 'no_such_package';
25+
26+
export type NpmDownloadResult<T> = { success: true; result: T } | { success: false; reason: DownloadFailure };
27+
28+
export function showDownloadFailure(f: DownloadFailure) {
29+
switch (f) {
30+
case 'no_such_package': return 'NPM package does not exist';
31+
}
32+
}
33+
34+
export async function downloadNpmPackage<T>(pkg: string, block: (dir: string) => Promise<T>): Promise<NpmDownloadResult<T>> {
2535
return await inTempDir(async () => {
2636
LOG.info(`Fetching NPM package ${pkg}`);
2737

28-
// Need to install package and dependencies in order for jsii-reflect
29-
// to not bork when it can find the dependencies.
30-
await exec(`npm install --silent --prefix . ${pkg}`);
38+
try {
39+
// Need to install package and dependencies in order for jsii-reflect
40+
// to not bork when it can find the dependencies.
41+
await exec(`npm install --silent --prefix . ${pkg}`);
42+
} catch (e) {
43+
// If this fails, might be because the package doesn't exist
44+
if (!isSubprocesFailedError(e)) { throw e; }
45+
if (await npmPackageExists(pkg)) {
46+
throw new Error(`NPM fetch failed: ${e}. Please try again.`);
47+
}
48+
LOG.warn(`NPM package ${pkg} does not exist.`);
49+
return { success: false, reason: 'no_such_package' } as NpmDownloadResult<T>;
50+
}
3151

3252
const pkgDir = trimVersionString(pkg);
33-
return await block(path.join(process.cwd(), 'node_modules', pkgDir));
53+
return {
54+
success: true,
55+
result: await block(path.join(process.cwd(), 'node_modules', pkgDir))
56+
} as NpmDownloadResult<T>;
3457
});
3558
}
3659

60+
function isSubprocesFailedError(e: any) {
61+
return e.code !== undefined && e.cmd !== undefined;
62+
}
63+
64+
async function npmPackageExists(pkg: string): Promise<boolean> {
65+
try {
66+
LOG.info(`Checking existence of ${pkg}`);
67+
await exec(`npm show --silent ${pkg}`);
68+
return true;
69+
} catch (e) {
70+
if (!isSubprocesFailedError(e)) { throw e; }
71+
return false;
72+
}
73+
}
74+
3775
/**
3876
* Trim an optional version string from an NPM package name
3977
*/

0 commit comments

Comments
 (0)