Skip to content

Commit

Permalink
fix(js): node executor should always log error (#17622)
Browse files Browse the repository at this point in the history
  • Loading branch information
mandarini authored Jun 16, 2023
1 parent bfe8e47 commit 36838d6
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 23 deletions.
50 changes: 50 additions & 0 deletions e2e/js/src/js-node.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import {
cleanupProject,
newProject,
runCLI,
uniq,
updateFile,
updateProjectConfig,
} from '@nx/e2e/utils';

describe('js:node error handling', () => {
let scope: string;

beforeEach(() => {
scope = newProject();
});

afterEach(() => cleanupProject());

it('should log out the error', () => {
const esbuildLib = uniq('esbuildlib');

runCLI(
`generate @nx/js:lib ${esbuildLib} --bundler=esbuild --no-interactive`
);

updateFile(`libs/${esbuildLib}/src/index.ts`, () => {
return `
console.log('Hello from my library!');
throw new Error('This is an error');
`;
});

updateProjectConfig(esbuildLib, (config) => {
config.targets['run-node'] = {
executor: '@nx/js:node',
options: {
buildTarget: `${esbuildLib}:build`,
watch: false,
},
};
return config;
});

const output = runCLI(`run ${esbuildLib}:run-node`, {
redirectStderr: true,
});
expect(output).toContain('Hello from my library!');
expect(output).toContain('This is an error');
}, 240_000);
});
32 changes: 16 additions & 16 deletions e2e/utils/command-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface RunCmdOpts {
cwd?: string;
silent?: boolean;
verbose?: boolean;
redirectStderr?: boolean;
}

/**
Expand Down Expand Up @@ -325,26 +326,25 @@ export function runCLI(
silenceError: false,
env: undefined,
verbose: undefined,
redirectStderr: undefined,
}
): string {
try {
const pm = getPackageManagerCommand();
const logs = execSync(
`${pm.runNxSilent} ${command} ${
opts.verbose ?? isVerboseE2ERun() ? ' --verbose' : ''
}`,
{
cwd: opts.cwd || tmpProjPath(),
env: {
CI: 'true',
...getStrippedEnvironmentVariables(),
...opts.env,
},
encoding: 'utf-8',
stdio: 'pipe',
maxBuffer: 50 * 1024 * 1024,
}
);
const commandToRun = `${pm.runNxSilent} ${command} ${
opts.verbose ?? isVerboseE2ERun() ? ' --verbose' : ''
}${opts.redirectStderr ? ' 2>&1' : ''}`;
const logs = execSync(commandToRun, {
cwd: opts.cwd || tmpProjPath(),
env: {
CI: 'true',
...getStrippedEnvironmentVariables(),
...opts.env,
},
encoding: 'utf-8',
stdio: 'pipe',
maxBuffer: 50 * 1024 * 1024,
});

if (opts.verbose ?? isVerboseE2ERun()) {
output.log({
Expand Down
35 changes: 28 additions & 7 deletions packages/js/src/executors/node/node.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import {
import { daemonClient } from 'nx/src/daemon/client/client';
import { randomUUID } from 'crypto';
import { join } from 'path';
import * as path from 'path';

import { InspectType, NodeExecutorOptions } from './schema';
import { createAsyncIterable } from '@nx/devkit/src/utils/async-iterable';
import { calculateProjectDependencies } from '../../utils/buildable-libs-utils';
import { killTree } from './lib/kill-tree';
import { fileExists } from 'nx/src/utils/fileutils';

interface ActiveTask {
id: string;
Expand Down Expand Up @@ -71,12 +73,11 @@ export async function* nodeExecutor(

// Re-map buildable workspace projects to their output directory.
const mappings = calculateResolveMappings(context, options);
const fileToRun = join(
context.root,
buildOptions.outputPath,
buildOptions.outputFileName ?? 'main.js'
);

const outputFileName =
buildOptions.outputFileName ?? `${path.parse(buildOptions.main).name}.js`;

const fileToRun = join(context.root, buildOptions.outputPath, outputFileName);
const tasks: ActiveTask[] = [];
let currentTask: ActiveTask = null;

Expand Down Expand Up @@ -159,7 +160,7 @@ export async function* nodeExecutor(
stdio: [0, 1, 'pipe', 'ipc'],
env: {
...process.env,
NX_FILE_TO_RUN: fileToRun,
NX_FILE_TO_RUN: fileToRunCorrectPath(fileToRun),
NX_MAPPINGS: JSON.stringify(mappings),
},
}
Expand All @@ -168,7 +169,8 @@ export async function* nodeExecutor(
task.childProcess.stderr.on('data', (data) => {
// Don't log out error if task is killed and new one has started.
// This could happen if a new build is triggered while new process is starting, since the operation is not atomic.
if (options.watch && !task.killed) {
// Log the error in normal mode
if (!options.watch || !task.killed) {
logger.error(data.toString());
}
});
Expand Down Expand Up @@ -304,4 +306,23 @@ function runWaitUntilTargets(
);
}

function fileToRunCorrectPath(fileToRun: string): string {
if (!fileExists(fileToRun)) {
const cjsFile = fileToRun.replace(/\.js$/, '.cjs');
if (fileExists(cjsFile)) {
fileToRun = cjsFile;
} else {
const mjsFile = fileToRun.replace(/\.js$/, '.mjs');
if (fileExists(mjsFile)) {
fileToRun = mjsFile;
} else {
throw new Error(
`Could not find ${fileToRun}. Make sure your build succeeded.`
);
}
}
}
return fileToRun;
}

export default nodeExecutor;

1 comment on commit 36838d6

@vercel
Copy link

@vercel vercel bot commented on 36838d6 Jun 16, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

nx-dev – ./

nx-five.vercel.app
nx-dev-nrwl.vercel.app
nx-dev-git-master-nrwl.vercel.app
nx.dev

Please sign in to comment.