fix(core): handle schema validation errors running commands directly (n…

…rwl#22864)
AgentEnder · Apr 23, 2024 · 69aa47f · 69aa47f
1 parent 62b7808
commit 69aa47f
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 64 deletions.
diff --git a/packages/nx/src/tasks-runner/task-orchestrator.ts b/packages/nx/src/tasks-runner/task-orchestrator.ts
@@ -393,53 +393,69 @@ export class TaskOrchestrator {
         targetConfiguration.executor === 'nx:run-commands' &&
         !shouldPrefix
       ) {
-        const { schema } = getExecutorForTask(task, this.projectGraph);
-        const isRunOne = this.initiatingProject != null;
-        const combinedOptions = combineOptionsForExecutor(
-          task.overrides,
-          task.target.configuration ?? targetConfiguration.defaultConfiguration,
-          targetConfiguration,
-          schema,
-          task.target.project,
-          relative(task.projectRoot ?? workspaceRoot, process.cwd()),
-          process.env.NX_VERBOSE_LOGGING === 'true'
-        );
-        if (combinedOptions.env) {
-          env = {
-            ...env,
-            ...combinedOptions.env,
-          };
-        }
-        if (streamOutput) {
-          const args = getPrintableCommandArgsForTask(task);
-          output.logCommand(args.join(' '));
-        }
-        const { success, terminalOutput } = await runCommandsImpl(
-          {
-            ...combinedOptions,
-            env,
-            usePty: isRunOne && !this.tasksSchedule.hasTasks(),
-            streamOutput,
-          },
-          {
-            root: workspaceRoot, // only root is needed in runCommandsImpl
-          } as any
-        );
+        try {
+          const { schema } = getExecutorForTask(task, this.projectGraph);
+          const isRunOne = this.initiatingProject != null;
+          const combinedOptions = combineOptionsForExecutor(
+            task.overrides,
+            task.target.configuration ??
+              targetConfiguration.defaultConfiguration,
+            targetConfiguration,
+            schema,
+            task.target.project,
+            relative(task.projectRoot ?? workspaceRoot, process.cwd()),
+            process.env.NX_VERBOSE_LOGGING === 'true'
+          );
+          if (combinedOptions.env) {
+            env = {
+              ...env,
+              ...combinedOptions.env,
+            };
+          }
+          if (streamOutput) {
+            const args = getPrintableCommandArgsForTask(task);
+            output.logCommand(args.join(' '));
+          }
+          const { success, terminalOutput } = await runCommandsImpl(
+            {
+              ...combinedOptions,
+              env,
+              usePty: isRunOne && !this.tasksSchedule.hasTasks(),
+              streamOutput,
+            },
+            {
+              root: workspaceRoot, // only root is needed in runCommandsImpl
+            } as any
+          );
 
-        const status = success ? 'success' : 'failure';
-        if (!streamOutput) {
-          this.options.lifeCycle.printTaskTerminalOutput(
+          const status = success ? 'success' : 'failure';
+          if (!streamOutput) {
+            this.options.lifeCycle.printTaskTerminalOutput(
+              task,
+              status,
+              terminalOutput
+            );
+          }
+          writeFileSync(temporaryOutputPath, terminalOutput);
+          results.push({
             task,
             status,
-            terminalOutput
-          );
+            terminalOutput,
+          });
+        } catch (e) {
+          if (process.env.NX_VERBOSE_LOGGING === 'true') {
+            console.error(e);
+          } else {
+            console.error(e.message);
+          }
+          const terminalOutput = e.stack ?? e.message ?? '';
+          writeFileSync(temporaryOutputPath, terminalOutput);
+          results.push({
+            task,
+            status: 'failure',
+            terminalOutput,
+          });
         }
-        writeFileSync(temporaryOutputPath, terminalOutput);
-        results.push({
-          task,
-          status,
-          terminalOutput,
-        });
       } else {
         // cache prep
         const { code, terminalOutput } = await this.runTaskInForkedProcess(

diff --git a/packages/nx/src/utils/params.spec.ts b/packages/nx/src/utils/params.spec.ts
@@ -826,7 +826,8 @@ describe('params', () => {
           }
         )
       ).toThrowErrorMatchingInlineSnapshot(`
-        "Options did not match schema. Please fix 1 of the following errors:
+        "Options did not match schema: {}.
+        Please fix 1 of the following errors:
          - Required property 'a' is missing
          - Required property 'b' is missing"
       `);
@@ -862,7 +863,15 @@ describe('params', () => {
             ],
           }
         )
-      ).toThrowErrorMatchingInlineSnapshot(`"Options did not match schema."`);
+      ).toThrowErrorMatchingInlineSnapshot(`
+        "Options did not match schema: {
+          "a": true,
+          "b": false
+        }.
+        Should only match one of 
+         - {"required":["a"]}
+         - {"required":["b"]}"
+      `);
     });
 
     it('should throw if none of the anyOf conditions are met', () => {

diff --git a/packages/nx/src/utils/params.ts b/packages/nx/src/utils/params.ts
@@ -271,27 +271,40 @@ export function validateObject(
     }
   }
   if (schema.oneOf) {
-    for (const s of schema.oneOf) {
-      const errors: Error[] = [];
-      for (const s of schema.oneOf) {
-        try {
-          validateObject(opts, s, definitions);
-        } catch (e) {
-          errors.push(e);
-        }
-      }
-      if (errors.length === schema.oneOf.length) {
-        throw new Error(
-          `Options did not match schema. Please fix 1 of the following errors:\n${errors
-            .map((e) => ' - ' + e.message)
-            .join('\n')}`
-        );
-      }
-      if (errors.length < schema.oneOf.length - 1) {
-        // TODO: This error could be better.
-        throw new Error(`Options did not match schema.`);
+    const matches: Array<PropertyDescription> = [];
+    const errors: Array<Error> = [];
+    for (const propertyDescription of schema.oneOf) {
+      try {
+        validateObject(opts, propertyDescription, definitions);
+        matches.push(propertyDescription);
+      } catch (error) {
+        errors.push(error);
       }
     }
+    // If the options matched none of the oneOf property descriptions
+    if (matches.length === 0) {
+      throw new Error(
+        `Options did not match schema: ${JSON.stringify(
+          opts,
+          null,
+          2
+        )}.\nPlease fix 1 of the following errors:\n${errors
+          .map((e) => ' - ' + e.message)
+          .join('\n')}`
+      );
+    }
+    // If the options matched none of the oneOf property descriptions
+    if (matches.length > 1) {
+      throw new Error(
+        `Options did not match schema: ${JSON.stringify(
+          opts,
+          null,
+          2
+        )}.\nShould only match one of \n${matches
+          .map((m) => ' - ' + JSON.stringify(m))
+          .join('\n')}`
+      );
+    }
   }
 
   (schema.required ?? []).forEach((p) => {