fix: capture stdout on the publish executor to check the error message (

#252)

packages/nx-python/src/executors/publish/executor.spec.ts

@@ -12,6 +12,12 @@ const nxDevkitMocks = vi.hoisted(() => {
   };
 });
 
+const childProcessMocks = vi.hoisted(() => {
+  return {
+    spawn: vi.fn(),
+  };
+});
+
 vi.mock('@nx/devkit', async (importOriginal) => {
   const actual = await importOriginal<typeof import('@nx/devkit')>();
   return {
@@ -28,11 +34,18 @@ vi.mock('fs-extra', async (importOriginal) => {
   };
 });
 
+vi.mock('child_process', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('child_process')>();
+  return {
+    ...actual,
+    ...childProcessMocks,
+  };
+});
+
 import chalk from 'chalk';
-import '../../utils/mocks/cross-spawn.mock';
 import * as poetryUtils from '../utils/poetry';
 import executor from './executor';
-import spawn from 'cross-spawn';
+import { EventEmitter } from 'events';
 
 describe('Publish Executor', () => {
   let checkPoetryExecutableMock: MockInstance;
@@ -64,15 +77,6 @@ describe('Publish Executor', () => {
       .spyOn(poetryUtils, 'activateVenv')
       .mockReturnValue(undefined);
 
-    vi.mocked(spawn.sync).mockReturnValue({
-      status: 0,
-      output: [''],
-      pid: 0,
-      signal: null,
-      stderr: null,
-      stdout: null,
-    });
-
     vi.spyOn(process, 'chdir').mockReturnValue(undefined);
   });
 
@@ -96,7 +100,7 @@ describe('Publish Executor', () => {
     const output = await executor(options, context);
     expect(checkPoetryExecutableMock).toHaveBeenCalled();
     expect(activateVenvMock).toHaveBeenCalledWith('.');
-    expect(spawn.sync).not.toHaveBeenCalled();
+    expect(childProcessMocks.spawn).not.toHaveBeenCalled();
     expect(output.success).toBe(false);
   });
 
@@ -112,7 +116,7 @@ describe('Publish Executor', () => {
     const output = await executor(options, context);
     expect(checkPoetryExecutableMock).toHaveBeenCalled();
     expect(activateVenvMock).toHaveBeenCalledWith('.');
-    expect(spawn.sync).not.toHaveBeenCalled();
+    expect(childProcessMocks.spawn).not.toHaveBeenCalled();
     expect(output.success).toBe(false);
   });
 
@@ -129,7 +133,7 @@ describe('Publish Executor', () => {
     const output = await executor(options, context);
     expect(checkPoetryExecutableMock).toHaveBeenCalled();
     expect(activateVenvMock).toHaveBeenCalledWith('.');
-    expect(spawn.sync).not.toHaveBeenCalled();
+    expect(childProcessMocks.spawn).not.toHaveBeenCalled();
     expect(output.success).toBe(false);
   });
 
@@ -145,13 +149,24 @@ describe('Publish Executor', () => {
       dryRun: false,
     };
 
+    const spawnEvent = new EventEmitter();
+    childProcessMocks.spawn.mockReturnValue({
+      stdout: new EventEmitter(),
+      stderr: new EventEmitter(),
+      on: vi.fn().mockImplementation((event, callback) => {
+        spawnEvent.on(event, callback);
+        spawnEvent.emit('close', 0);
+      }),
+    });
+
     const output = await executor(options, context);
     expect(checkPoetryExecutableMock).toHaveBeenCalled();
     expect(activateVenvMock).toHaveBeenCalledWith('.');
-    expect(spawn.sync).toHaveBeenCalledWith('poetry', ['publish'], {
+    expect(childProcessMocks.spawn).toHaveBeenCalledWith('poetry publish', {
       cwd: 'tmp',
-      shell: false,
-      stdio: 'inherit',
+      env: { ...process.env, FORCE_COLOR: 'true' },
+      shell: true,
+      stdio: ['inherit', 'pipe', 'pipe'],
     });
     expect(output.success).toBe(true);
     expect(nxDevkitMocks.runExecutor).toHaveBeenCalledWith(
@@ -181,16 +196,26 @@ describe('Publish Executor', () => {
       __unparsed__: ['-vvv', '--dry-run'],
     };
 
+    const spawnEvent = new EventEmitter();
+    childProcessMocks.spawn.mockReturnValue({
+      stdout: new EventEmitter(),
+      stderr: new EventEmitter(),
+      on: vi.fn().mockImplementation((event, callback) => {
+        spawnEvent.on(event, callback);
+        spawnEvent.emit('close', 0);
+      }),
+    });
+
     const output = await executor(options, context);
     expect(checkPoetryExecutableMock).toHaveBeenCalled();
     expect(activateVenvMock).toHaveBeenCalledWith('.');
-    expect(spawn.sync).toHaveBeenCalledWith(
-      'poetry',
-      ['publish', '-vvv', '--dry-run'],
+    expect(childProcessMocks.spawn).toHaveBeenCalledWith(
+      'poetry publish -vvv --dry-run',
       {
         cwd: 'tmp',
-        shell: false,
-        stdio: 'inherit',
+        env: { ...process.env, FORCE_COLOR: 'true' },
+        shell: true,
+        stdio: ['inherit', 'pipe', 'pipe'],
       },
     );
     expect(output.success).toBe(true);
@@ -207,4 +232,107 @@ describe('Publish Executor', () => {
     );
     expect(fsExtraMocks.removeSync).toHaveBeenCalledWith('tmp');
   });
+
+  it('should run poetry publish and not throw an exception when the message contains "File already exists"', async () => {
+    nxDevkitMocks.runExecutor.mockResolvedValueOnce([
+      { success: true, buildFolderPath: 'tmp' },
+    ]);
+    fsExtraMocks.removeSync.mockReturnValue(undefined);
+
+    const options = {
+      buildTarget: 'build',
+      dryRun: false,
+      silent: false,
+    };
+
+    const spawnEvent = new EventEmitter();
+    const stdoutEvent = new EventEmitter();
+    childProcessMocks.spawn.mockReturnValue({
+      stdout: {
+        on: vi.fn().mockImplementation((event, callback) => {
+          stdoutEvent.on(event, callback);
+          stdoutEvent.emit(event, 'HTTP Error 400: File already exists');
+        }),
+      },
+      stderr: new EventEmitter(),
+      on: vi.fn().mockImplementation((event, callback) => {
+        spawnEvent.on(event, callback);
+        spawnEvent.emit('close', 1);
+      }),
+    });
+
+    const output = await executor(options, context);
+    expect(checkPoetryExecutableMock).toHaveBeenCalled();
+    expect(activateVenvMock).toHaveBeenCalledWith('.');
+    expect(childProcessMocks.spawn).toHaveBeenCalledWith('poetry publish', {
+      cwd: 'tmp',
+      env: { ...process.env, FORCE_COLOR: 'true' },
+      shell: true,
+      stdio: ['inherit', 'pipe', 'pipe'],
+    });
+    expect(output.success).toBe(true);
+    expect(nxDevkitMocks.runExecutor).toHaveBeenCalledWith(
+      {
+        configuration: undefined,
+        project: 'app',
+        target: 'build',
+      },
+      {
+        keepBuildFolder: true,
+      },
+      context,
+    );
+    expect(fsExtraMocks.removeSync).toHaveBeenCalledWith('tmp');
+  });
+
+  it('should throw an exception when status code is not 0 and the message does not contains "File already exists"', async () => {
+    nxDevkitMocks.runExecutor.mockResolvedValueOnce([
+      { success: true, buildFolderPath: 'tmp' },
+    ]);
+    fsExtraMocks.removeSync.mockReturnValue(undefined);
+
+    const options = {
+      buildTarget: 'build',
+      dryRun: false,
+      silent: false,
+    };
+
+    const spawnEvent = new EventEmitter();
+    const stdoutEvent = new EventEmitter();
+    childProcessMocks.spawn.mockReturnValue({
+      stdout: {
+        on: vi.fn().mockImplementation((event, callback) => {
+          stdoutEvent.on(event, callback);
+          stdoutEvent.emit('data', 'Some other error message');
+        }),
+      },
+      stderr: new EventEmitter(),
+      on: vi.fn().mockImplementation((event, callback) => {
+        spawnEvent.on(event, callback);
+        spawnEvent.emit('close', 1);
+      }),
+    });
+
+    const output = await executor(options, context);
+    expect(checkPoetryExecutableMock).toHaveBeenCalled();
+    expect(activateVenvMock).toHaveBeenCalledWith('.');
+    expect(childProcessMocks.spawn).toHaveBeenCalledWith('poetry publish', {
+      cwd: 'tmp',
+      env: { ...process.env, FORCE_COLOR: 'true' },
+      shell: true,
+      stdio: ['inherit', 'pipe', 'pipe'],
+    });
+    expect(output.success).toBe(false);
+    expect(nxDevkitMocks.runExecutor).toHaveBeenCalledWith(
+      {
+        configuration: undefined,
+        project: 'app',
+        target: 'build',
+      },
+      {
+        keepBuildFolder: true,
+      },
+      context,
+    );
+  });
 });

packages/nx-python/src/executors/publish/executor.ts

@@ -5,10 +5,11 @@ import { Logger } from '../utils/logger';
 import {
   activateVenv,
   checkPoetryExecutable,
-  runPoetry,
+  POETRY_EXECUTABLE,
 } from '../utils/poetry';
 import { BuildExecutorOutput } from '../build/schema';
 import { removeSync } from 'fs-extra';
+import { spawnPromise } from '../utils/cmd';
 
 const logger = new Logger();
 
@@ -19,12 +20,12 @@ export default async function executor(
   logger.setOptions(options);
   const workspaceRoot = context.root;
   process.chdir(workspaceRoot);
+  let buildFolderPath = '';
+
   try {
     activateVenv(workspaceRoot);
     await checkPoetryExecutable();
 
-    let buildFolderPath = '';
-
     for await (const output of await runExecutor<BuildExecutorOutput>(
       {
         project: context.projectName,
@@ -51,33 +52,48 @@ export default async function executor(
       chalk`\n  {bold Publishing project {bgBlue  ${context.projectName} }...}\n`,
     );
 
-    await runPoetry(
-      [
-        'publish',
-        ...(options.dryRun ? ['--dry-run'] : []),
-        ...(options.__unparsed__ ?? []),
-      ],
-      {
-        cwd: buildFolderPath,
-      },
+    const commandArgs = [
+      'publish',
+      ...(options.dryRun ? ['--dry-run'] : []),
+      ...(options.__unparsed__ ?? []),
+    ];
+    const commandStr = `${POETRY_EXECUTABLE} ${commandArgs.join(' ')}`;
+
+    console.log(
+      chalk`{bold Running command}: ${commandStr} ${
+        buildFolderPath && buildFolderPath !== '.'
+          ? chalk`at {bold ${buildFolderPath}} folder`
+          : ''
+      }\n`,
     );
 
+    await spawnPromise(commandStr, buildFolderPath);
     removeSync(buildFolderPath);
 
     return {
       success: true,
     };
   } catch (error) {
-    if (error.message && error.message.includes('File already exists')) {
-      logger.info(
-        chalk`\n  {bgYellow.bold  WARNING } {bold The package is already published}\n`,
-      );
-      return {
-        success: true,
-      };
+    if (buildFolderPath) {
+      removeSync(buildFolderPath);
+    }
+
+    if (typeof error === 'object' && 'code' in error && 'output' in error) {
+      if (error.code !== 0 && error.output.includes('File already exists')) {
+        logger.info(
+          chalk`\n  {bgYellow.bold  WARNING } {bold The package is already published}\n`,
+        );
+
+        return {
+          success: true,
+        };
+      } else if (error.code !== 0) {
+        logger.info(
+          chalk`\n  {bgRed.bold  ERROR } {bold The publish command failed}\n`,
+        );
+      }
     }
 
-    logger.info(chalk`\n  {bgRed.bold  ERROR } ${error.message}\n`);
     return {
       success: false,
     };

packages/nx-python/src/executors/utils/cmd.ts

@@ -0,0 +1,56 @@
+import type { SpawnSyncOptions } from 'child_process';
+import { spawn } from 'child_process';
+
+export type SpawnPromiseResult = {
+  success: boolean;
+  code: number | null;
+  output: string;
+};
+
+export const spawnPromise = function (
+  command: string,
+  cwd: string,
+  envVars?: Record<string, string | undefined>,
+  output = true,
+): Promise<SpawnPromiseResult> {
+  return new Promise((resolve, reject) => {
+    console.log(`Running command: ${command}`);
+    const env: Record<string, string> = {
+      ...process.env,
+      ...(envVars ?? {}),
+      ...(output ? { FORCE_COLOR: 'true' } : {}),
+    };
+
+    const args: SpawnSyncOptions = {
+      cwd,
+      shell: true,
+      stdio: output ? ['inherit', 'pipe', 'pipe'] : 'inherit',
+      env,
+    };
+
+    const child = spawn(command, args);
+    let outputStr = '';
+
+    if (output) {
+      child.stdout?.on('data', (data) => {
+        process.stdout.write(data);
+        outputStr += data;
+      });
+
+      child.stderr?.on('data', (data) => {
+        process.stderr.write(data);
+        outputStr += data;
+      });
+    }
+
+    child.on('close', (code) => {
+      const result = {
+        success: code === 0,
+        code,
+        output: outputStr,
+      };
+
+      code === 0 ? resolve(result) : reject(result);
+    });
+  });
+};