fix(aws-lambda-nodejs): address PR review findings for spawn refactor

- Use getTsconfigCompilerOptionsArray in Docker path instead of naive
  string splitting to properly handle compiler option values with spaces
- Wrap callback step operations with try-catch for contextual error
  messages during local bundling file operations
- Add regression tests for Docker preCompilation tsconfig handling
- Add test for callback failure error context
- Add getTsconfigCompilerOptionsArray parity tests in util.test.ts
- Remove unused getTsconfigCompilerOptions import from bundling.ts

Author: web-flow

packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts

@@ -6,7 +6,7 @@ import { PackageInstallation } from './package-installation';
 import { LockFile, PackageManager } from './package-manager';
 import type { BundlingOptions } from './types';
 import { OutputFormat, SourceMapMode } from './types';
-import { exec, extractDependencies, findUp, getTsconfigCompilerOptions, getTsconfigCompilerOptionsArray, isSdkV2Runtime } from './util';
+import { exec, extractDependencies, findUp, getTsconfigCompilerOptionsArray, isSdkV2Runtime } from './util';
 import type { Architecture, AssetCode } from '../../aws-lambda';
 import { Code, Runtime } from '../../aws-lambda';
 import * as cdk from '../../core';
@@ -269,8 +269,8 @@ export class Bundling implements cdk.BundlingOptions {
       if (!tsconfig) {
         throw new ValidationError('CannotFindTsconfigJsonPre', 'Cannot find a `tsconfig.json` but `preCompilation` is set to `true`, please specify it via `tsconfig`', scope);
       }
-      const compilerOptions = getTsconfigCompilerOptions(tsconfig);
-      tscCommand = preparePosixShellCommand([options.tscRunner!, relativeEntryPath, ...compilerOptions.split(/\s+/).filter(Boolean)]);
+      const compilerOptionsArray = getTsconfigCompilerOptionsArray(tsconfig);
+      tscCommand = preparePosixShellCommand([options.tscRunner!, relativeEntryPath, ...compilerOptionsArray]);
       relativeEntryPath = relativeEntryPath.replace(/\.ts(x?)$/, '.js$1');
     }
 
@@ -371,7 +371,11 @@ export class Bundling implements cdk.BundlingOptions {
               });
               break;
             case 'callback':
-              step.operation();
+              try {
+                step.operation();
+              } catch (err) {
+                throw new Error(`Local bundling file operation failed: ${err instanceof Error ? err.message : String(err)}`);
+              }
               break;
           }
         }

packages/aws-cdk-lib/aws-lambda-nodejs/test/bundling.test.ts

@@ -253,22 +253,22 @@ test('esbuild bundling with esbuild options', () => {
   });
 
   // Correctly bundles with esbuild
-  const defineInstructions = "'--define:process.env.KEY=\"VALUE\"' '--define:process.env.BOOL=true' '--define:process.env.NUMBER=7777' '--define:process.env.STRING=\"this is a \\\"test\\\"\"'";
+  const defineInstructions = '\'--define:process.env.KEY="VALUE"\' \'--define:process.env.BOOL=true\' \'--define:process.env.NUMBER=7777\' \'--define:process.env.STRING="this is a \\"test\\""\'';
   expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(depsLockFilePath), {
     assetHashType: AssetHashType.OUTPUT,
     bundling: expect.objectContaining({
       command: [
         'bash',
         '-c',
         [
-          `'esbuild' '--bundle' '/asset-input/lib/handler.ts'`,
-          `'--target=es2020' '--platform=node' '--format=esm' '--outfile=/asset-output/index.mjs'`,
+          '\'esbuild\' \'--bundle\' \'/asset-input/lib/handler.ts\'',
+          '\'--target=es2020\' \'--platform=node\' \'--format=esm\' \'--outfile=/asset-output/index.mjs\'',
           `'--minify' '--sourcemap' '--sources-content=false' '--external:${STANDARD_EXTERNAL}' '--loader:.png=dataurl'`,
           defineInstructions,
-          `'--log-level=silent' '--keep-names' '--tsconfig=/asset-input/lib/custom-tsconfig.ts'`,
-          `'--metafile=/asset-output/index.meta.json' '--banner:js=/* comments */' '--footer:js=/* comments */'`,
-          `'--main-fields=module,main' '--inject:./my-shim.js' '--inject:./path with space/second-shim.js'`,
-          `'--log-limit=0' '--resolve-extensions=.ts,.js' '--splitting' '--keep-names' '--out-extension:.js=.mjs'`,
+          '\'--log-level=silent\' \'--keep-names\' \'--tsconfig=/asset-input/lib/custom-tsconfig.ts\'',
+          '\'--metafile=/asset-output/index.meta.json\' \'--banner:js=/* comments */\' \'--footer:js=/* comments */\'',
+          '\'--main-fields=module,main\' \'--inject:./my-shim.js\' \'--inject:./path with space/second-shim.js\'',
+          '\'--log-limit=0\' \'--resolve-extensions=.ts,.js\' \'--splitting\' \'--keep-names\' \'--out-extension:.js=.mjs\'',
         ].join(' '),
       ],
     }),
@@ -394,7 +394,7 @@ test('esbuild bundling with feature flag enabled using Node Latest', () => {
     bundling: expect.objectContaining({
       command: [
         'bash', '-c',
-        `'esbuild' '--bundle' '/asset-input/lib/handler.ts' '--target=node22' '--platform=node' '--outfile=/asset-output/index.js'`,
+        '\'esbuild\' \'--bundle\' \'/asset-input/lib/handler.ts\' \'--target=node22\' \'--platform=node\' \'--outfile=/asset-output/index.js\'',
       ],
     }),
   });
@@ -421,7 +421,7 @@ test('esbuild bundling with feature flag enabled using Node 16', () => {
     bundling: expect.objectContaining({
       command: [
         'bash', '-c',
-        `'esbuild' '--bundle' '/asset-input/lib/handler.ts' '--target=node16' '--platform=node' '--outfile=/asset-output/index.js' '--external:aws-sdk'`,
+        '\'esbuild\' \'--bundle\' \'/asset-input/lib/handler.ts\' \'--target=node16\' \'--platform=node\' \'--outfile=/asset-output/index.js\' \'--external:aws-sdk\'',
       ],
     }),
   });
@@ -869,8 +869,8 @@ test('esbuild bundling with pre compilations', () => {
     architecture: Architecture.X86_64,
   });
 
-  const compilerOptions = util.getTsconfigCompilerOptions(findParentTsConfigPath(__dirname));
-  const quotedCompilerOptions = compilerOptions.split(/\s+/).filter(Boolean).map((a: string) => `'${a}'`).join(' ');
+  const compilerOptionsArray = util.getTsconfigCompilerOptionsArray(findParentTsConfigPath(__dirname));
+  const quotedCompilerOptions = compilerOptionsArray.map((a: string) => `'${a}'`).join(' ');
 
   // Correctly bundles with esbuild
   expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(packageLock), {
@@ -1087,7 +1087,7 @@ test('bundling using NODEJS_LATEST doesn\'t externalize anything by default', ()
     bundling: expect.objectContaining({
       command: [
         'bash', '-c',
-        `'esbuild' '--bundle' '/asset-input/lib/handler.ts' '--target=node22' '--platform=node' '--outfile=/asset-output/index.js'`,
+        '\'esbuild\' \'--bundle\' \'/asset-input/lib/handler.ts\' \'--target=node22\' \'--platform=node\' \'--outfile=/asset-output/index.js\'',
       ],
     }),
   });
@@ -1188,6 +1188,76 @@ test('Node 16 runtimes warn about sdk v2 upgrades', () => {
   );
 });
 
+// --- Regression tests for PR review findings ---
+
+test('Docker bundling with preCompilation uses getTsconfigCompilerOptionsArray for proper escaping', () => {
+  const packageLock = path.join(__dirname, '..', 'package-lock.json');
+
+  Bundling.bundle(stack, {
+    entry: __filename.replace('.js', '.ts'),
+    projectRoot: path.dirname(packageLock),
+    depsLockFilePath: packageLock,
+    runtime: STANDARD_RUNTIME,
+    preCompilation: true,
+    forceDockerBundling: true,
+    architecture: Architecture.X86_64,
+  });
+
+  // The Docker tsc command should use getTsconfigCompilerOptionsArray (not naive string splitting)
+  // to properly handle compiler option values that may contain spaces.
+  // Verify the Docker command matches what getTsconfigCompilerOptionsArray produces.
+  const compilerOptionsArray = util.getTsconfigCompilerOptionsArray(findParentTsConfigPath(__dirname));
+  const quotedCompilerOptions = compilerOptionsArray.map((a: string) => `'${a}'`).join(' ');
+
+  expect(Code.fromAsset).toHaveBeenCalledWith(path.dirname(packageLock), {
+    assetHashType: AssetHashType.OUTPUT,
+    bundling: expect.objectContaining({
+      command: [
+        'bash', '-c',
+        [
+          `'tsc' '/asset-input/test/bundling.test.ts' ${quotedCompilerOptions} &&`,
+          `'esbuild' '--bundle' '/asset-input/test/bundling.test.js' '--target=${STANDARD_TARGET}' '--platform=node' '--outfile=/asset-output/index.js' '--external:${STANDARD_EXTERNAL}'`,
+        ].join(' '),
+      ],
+    }),
+  });
+});
+
+test('Local bundling callback failure includes contextual error message', () => {
+  const spawnSyncMock = jest.spyOn(child_process, 'spawnSync').mockReturnValue({
+    status: 0,
+    stderr: Buffer.from('stderr'),
+    stdout: Buffer.from('stdout'),
+    pid: 123,
+    output: ['stdout', 'stderr'],
+    signal: null,
+  });
+  const writeFileSyncMock = jest.spyOn(fs, 'writeFileSync').mockImplementation(() => {
+    throw new Error('EACCES: permission denied');
+  });
+  const copyFileSyncMock = jest.spyOn(fs, 'copyFileSync').mockReturnValue();
+
+  const packageLock = path.join(__dirname, '..', 'package-lock.json');
+  const bundler = new Bundling(stack, {
+    entry: __filename,
+    projectRoot: path.dirname(packageLock),
+    depsLockFilePath: packageLock,
+    runtime: STANDARD_RUNTIME,
+    architecture: Architecture.X86_64,
+    externalModules: ['abc'],
+    nodeModules: ['delay'],
+  });
+
+  // The callback step should wrap fs errors with context
+  expect(() => {
+    bundler.local?.tryBundle('/outdir', { image: STANDARD_RUNTIME.bundlingDockerImage });
+  }).toThrow(/Local bundling file operation failed.*EACCES/);
+
+  spawnSyncMock.mockRestore();
+  writeFileSyncMock.mockRestore();
+  copyFileSyncMock.mockRestore();
+});
+
 // --- Local bundling spawn tests ---
 
 const spawnSyncMockReturnValue = {

packages/aws-cdk-lib/aws-lambda-nodejs/test/util.test.ts

@@ -2,7 +2,7 @@ import * as child_process from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
 import { bockfs } from '@aws-cdk/cdk-build-tools';
-import { callsites, exec, extractDependencies, findUp, findUpMultiple, getTsconfigCompilerOptions } from '../lib/util';
+import { callsites, exec, extractDependencies, findUp, findUpMultiple, getTsconfigCompilerOptions, getTsconfigCompilerOptionsArray } from '../lib/util';
 
 beforeEach(() => {
   jest.clearAllMocks();
@@ -262,3 +262,46 @@ describe('getTsconfigCompilerOptions', () => {
     ].join(' '));
   });
 });
+
+describe('getTsconfigCompilerOptionsArray', () => {
+  test('should produce semantically equivalent output to getTsconfigCompilerOptions', () => {
+    const tsconfig = path.join(__dirname, 'testtsconfig.json');
+    const stringResult = getTsconfigCompilerOptions(tsconfig);
+    const arrayResult = getTsconfigCompilerOptionsArray(tsconfig);
+
+    // The array joined with spaces should equal the string result
+    expect(arrayResult.join(' ')).toEqual(stringResult);
+  });
+
+  test('should produce semantically equivalent output with extended config', () => {
+    const tsconfig = path.join(__dirname, 'testtsconfig-extended.json');
+    const stringResult = getTsconfigCompilerOptions(tsconfig);
+    const arrayResult = getTsconfigCompilerOptionsArray(tsconfig);
+
+    expect(arrayResult.join(' ')).toEqual(stringResult);
+  });
+
+  test('should return array elements for each flag and value', () => {
+    const tsconfig = path.join(__dirname, 'testtsconfig.json');
+    const arrayResult = getTsconfigCompilerOptionsArray(tsconfig);
+
+    // Boolean true flags are single elements
+    expect(arrayResult).toContain('--alwaysStrict');
+    expect(arrayResult).toContain('--declaration');
+
+    // Boolean false flags have the flag and 'false' as separate elements
+    const declMapIdx = arrayResult.indexOf('--declarationMap');
+    expect(declMapIdx).toBeGreaterThanOrEqual(0);
+    expect(arrayResult[declMapIdx + 1]).toBe('false');
+
+    // String values are separate elements
+    const targetIdx = arrayResult.indexOf('--target');
+    expect(targetIdx).toBeGreaterThanOrEqual(0);
+    expect(arrayResult[targetIdx + 1]).toBe('ES2022');
+
+    // Array values are joined with commas as a single element
+    const libIdx = arrayResult.indexOf('--lib');
+    expect(libIdx).toBeGreaterThanOrEqual(0);
+    expect(arrayResult[libIdx + 1]).toBe('es2022,dom');
+  });
+});