[spec-model] Improve error if input-files do not match schema (#37827)

- Fixes #37825

Author: mikeharder

.github/shared/cmd/spec-model.js

@@ -4,7 +4,7 @@ import { debugLogger } from "../src/logger.js";
 import { SpecModel } from "../src/spec-model.js";
 
 const USAGE =
-  "Usage: npx spec-model path/to/spec [--debug] [--include-refs] [--relative-paths]\n" +
+  "Usage: npx spec-model path/to/spec [--debug] [--include-refs] [--relative-paths] [--no-embed-errors]\n" +
   "Example: npx spec-model specfication/contosowidgetmanager";
 
 // Exclude first two args (node, script file)
@@ -19,6 +19,9 @@ args = args.filter((a) => a != "--include-refs");
 const relativePaths = args.includes("--relative-paths");
 args = args.filter((a) => a != "--relative-paths");
 
+const noEmbedErrors = args.includes("--no-embed-errors");
+args = args.filter((a) => a != "--no-embed-errors");
+
 if (args.length < 1) {
   console.error(USAGE);
   process.exit(1);
@@ -40,7 +43,7 @@ const specModel = new SpecModel(specPath, { logger });
 console.log(
   JSON.stringify(
     // Always embed errors, since we always want to return a valid JSON object instead of throwing
-    await specModel.toJSONAsync({ embedErrors: true, includeRefs, relativePaths }),
+    await specModel.toJSONAsync({ embedErrors: !noEmbedErrors, includeRefs, relativePaths }),
     null,
     2,
   ),

.github/shared/package-lock.json

@@ -38,7 +38,8 @@
     "debug": "^4.4.3",
     "js-yaml": "^4.1.0",
     "marked": "^16.3.0",
-    "simple-git": "^3.27.0"
+    "simple-git": "^3.27.0",
+    "zod": "^4.1.11"
   },
   "devDependencies": {
     "@eslint/js": "^9.22.0",

.github/shared/package.json

@@ -4,7 +4,9 @@ import { readFile } from "fs/promises";
 import yaml from "js-yaml";
 import { marked } from "marked";
 import { dirname, normalize, relative, resolve } from "path";
+import * as z from "zod";
 import { mapAsync } from "./array.js";
+import { SpecModelError } from "./spec-model-error.js";
 import { embedError } from "./spec-model.js";
 import { Tag } from "./tag.js";
 
@@ -18,6 +20,21 @@ import { Tag } from "./tag.js";
  */
 export const TagMatchRegex = /yaml.*\$\(tag\) ?== ?(["'])(.*?)\1/;
 
+// Example: "foo.json"
+const jsonFileSchema = z.string().regex(/\.json$/i);
+
+// Examples:
+// {}
+// {"input-file": "foo.json"}
+// {"input-file": ["foo.json", "bar.json"]}
+const inputFileSchema = z.object({
+  "input-file": z
+    // May be undefined, a single json filename, or an array of json filenames
+    .optional(z.union([jsonFileSchema, z.array(jsonFileSchema)]))
+    // Normalize single string to array of string.  Don't change 'undefined'.
+    .transform((value) => (typeof value === "string" ? [value] : value)),
+});
+
 export class Readme {
   /**
    * Content of `readme.md`, either loaded from `#path` or passed in via `options`.
@@ -53,12 +70,14 @@ export class Readme {
    * @param {import('./logger.js').ILogger} [options.logger]
    * @param {SpecModel} [options.specModel]
    */
-  constructor(path, options) {
-    this.#path = resolve(options?.specModel?.folder ?? "", path);
+  constructor(path, options = {}) {
+    const { content, logger, specModel } = options;
+
+    this.#path = resolve(specModel?.folder ?? "", path);
 
-    this.#content = options?.content;
-    this.#logger = options?.logger;
-    this.#specModel = options?.specModel;
+    this.#content = content;
+    this.#logger = logger;
+    this.#specModel = specModel;
   }
 
   /**
@@ -127,11 +146,31 @@ export class Readme {
         const obj = /** @type {any} */ (yaml.load(block.text, { schema: yaml.FAILSAFE_SCHEMA }));
 
         if (!obj) {
-          this.#logger?.debug(`No yaml object found for tag ${tagName} in ${this.#path}`);
+          this.#logger?.debug(`No YAML object found for tag ${tagName} in ${this.#path}`);
           continue;
         }
 
-        if (!obj["input-file"]) {
+        let parsedObj;
+        try {
+          parsedObj = inputFileSchema.parse(obj);
+        } catch (error) {
+          if (error instanceof z.ZodError) {
+            throw new SpecModelError(
+              `Unable to parse input-file YAML for tag ${tagName} in ${this.#path}`,
+              {
+                source: this.#path,
+                readme: this.#path,
+                tag: tagName,
+                cause: error,
+              },
+            );
+          } /* v8 ignore start: defensive rethrow */ else {
+            throw error;
+          }
+          /* v8 ignore end */
+        }
+
+        if (!parsedObj["input-file"]) {
           // The yaml block does not contain an input-file key
           continue;
         }
@@ -149,10 +188,7 @@ export class Readme {
           throw new Error(message);
         }
 
-        // It's possible for input-file to be a string or an array
-        const inputFilePaths = Array.isArray(obj["input-file"])
-          ? obj["input-file"]
-          : [obj["input-file"]];
+        const inputFilePaths = parsedObj["input-file"];
 
         const swaggerPathsResolved = inputFilePaths
           .map((p) => Readme.#normalizeSwaggerPath(p))
@@ -207,7 +243,9 @@ export class Readme {
    * @param {ToJSONOptions} [options]
    * @returns {Promise<Object>}
    */
-  async toJSONAsync(options) {
+  async toJSONAsync(options = {}) {
+    const { relativePaths } = options;
+
     return await embedError(async () => {
       const tags = await mapAsync(
         [...(await this.getTags()).values()].sort((a, b) => a.name.localeCompare(b.name)),
@@ -216,7 +254,7 @@ export class Readme {
 
       return {
         path:
-          options?.relativePaths && this.#specModel
+          relativePaths && this.#specModel
             ? relative(this.#specModel.folder, this.#path)
             : this.#path,
         globalConfig: await this.getGlobalConfig(),

.github/shared/src/readme.js

@@ -6,19 +6,19 @@ export class SpecModelError extends Error {
    *
    * @type {string|undefined}
    */
-  source;
+  #source;
 
   /**
    * Path to readme that caused the error (if known)
    * @type {string|undefined}
    */
-  readme;
+  #readme;
 
   /**
    * Name of tag that caused the error (if known)
    * @type {string|undefined}
    */
-  tag;
+  #tag;
 
   /**
    * @param {string} message
@@ -28,22 +28,34 @@ export class SpecModelError extends Error {
    * @param {string} [options.readme] Path to readme that caused the error (if known)
    * @param {string} [options.tag] Name of tag that caused the error (if known)
    */
-  constructor(message, options) {
-    super(message, { cause: options?.cause });
+  constructor(message, options = {}) {
+    const { cause, source, readme, tag } = options;
+
+    const fullMessage =
+      message +
+      (source ? `\n  Problem File: ${source}` : "") +
+      (readme ? `\n  Readme: ${readme}` : "") +
+      (tag ? `\n  Tag: ${tag}` : "") +
+      (cause ? `\n  Cause: ${cause}` : "");
+
+    super(fullMessage, { cause });
 
     this.name = this.constructor.name;
 
-    this.source = options?.source;
-    this.readme = options?.readme;
-    this.tag = options?.tag;
+    this.#source = source;
+    this.#readme = readme;
+    this.#tag = tag;
+  }
+
+  get source() {
+    return this.#source;
+  }
+
+  get readme() {
+    return this.#readme;
   }
 
-  toString() {
-    return (
-      `SpecModelError: ${this.message}` +
-      `${this.source ? `\n\tProblem File: ${this.source}` : ""}` +
-      `${this.readme ? `\n\tReadme: ${this.readme}` : ""}` +
-      `${this.tag ? `\n\tTag: ${this.tag}` : ""}`
-    );
+  get tag() {
+    return this.#tag;
   }
 }

.github/shared/src/spec-model-error.js

@@ -36,7 +36,9 @@ export class SpecModel {
    * @param {Object} [options]
    * @param {import('./logger.js').ILogger} [options.logger]
    */
-  constructor(folder, options) {
+  constructor(folder, options = {}) {
+    const { logger } = options;
+
     const resolvedFolder = resolve(folder);
 
     const cachedSpecModel = specModelCache.get(resolvedFolder);
@@ -45,7 +47,7 @@ export class SpecModel {
     }
 
     this.#folder = resolvedFolder;
-    this.#logger = options?.logger;
+    this.#logger = logger;
 
     specModelCache.set(resolvedFolder, this);
   }
@@ -216,7 +218,7 @@ export class SpecModel {
    * @param {ToJSONOptions} [options]
    * @returns {Promise<Object>}
    */
-  async toJSONAsync(options) {
+  async toJSONAsync(options = {}) {
     return await embedError(async () => {
       const readmes = await mapAsync(
         [...(await this.getReadmes()).values()].sort((a, b) => a.path.localeCompare(b.path)),
@@ -240,14 +242,17 @@ export class SpecModel {
 /**
  * @template T
  * @param {() => Promise<T>} fn
- * @param {{embedErrors?: boolean}} [options]
+ * @param {Object} [options]
+ * @param {boolean} [options.embedErrors]
  * @returns {Promise<T | {error: string}>}
  */
-export async function embedError(fn, options) {
+export async function embedError(fn, options = {}) {
+  const { embedErrors } = options;
+
   try {
     return await fn();
   } catch (error) {
-    if (options?.embedErrors && error instanceof Error) {
+    if (embedErrors && error instanceof Error) {
       return { error: error.message };
     } else {
       throw error;

.github/shared/src/spec-model.js

@@ -60,11 +60,13 @@ export class Swagger {
    * @param {import('./logger.js').ILogger} [options.logger]
    * @param {Tag} [options.tag]
    */
-  constructor(path, options) {
-    const rootDir = dirname(options?.tag?.readme?.path ?? "");
+  constructor(path, options = {}) {
+    const { logger, tag } = options;
+
+    const rootDir = dirname(tag?.readme?.path ?? "");
     this.#path = resolve(rootDir, path);
-    this.#logger = options?.logger;
-    this.#tag = options?.tag;
+    this.#logger = logger;
+    this.#tag = tag;
   }
 
   /**
@@ -202,14 +204,16 @@ export class Swagger {
    * @param {ToJSONOptions} [options]
    * @returns {Promise<Object>}
    */
-  async toJSONAsync(options) {
+  async toJSONAsync(options = {}) {
+    const { includeRefs, relativePaths } = options;
+
     return await embedError(
       async () => ({
         path:
-          options?.relativePaths && this.#tag?.readme?.specModel
+          relativePaths && this.#tag?.readme?.specModel
             ? relative(this.#tag?.readme?.specModel.folder, this.#path)
             : this.#path,
-        refs: options?.includeRefs
+        refs: includeRefs
           ? await mapAsync(
               [...(await this.getRefs()).values()].sort((a, b) => a.path.localeCompare(b.path)),
               async (s) =>

.github/shared/src/swagger.js

@@ -32,10 +32,12 @@ export class Tag {
    * @param {import('./logger.js').ILogger} [options.logger]
    * @param {Readme} [options.readme]
    */
-  constructor(name, inputFilePaths, options) {
+  constructor(name, inputFilePaths, options = {}) {
+    const { logger, readme } = options;
+
     this.#name = name;
-    this.#logger = options?.logger;
-    this.#readme = options?.readme;
+    this.#logger = logger;
+    this.#readme = readme;
 
     this.#inputFiles = new Map(
       inputFilePaths.map((p) => {
@@ -70,7 +72,7 @@ export class Tag {
    * @param {ToJSONOptions} [options]
    * @returns {Promise<Object>}
    */
-  async toJSONAsync(options) {
+  async toJSONAsync(options = {}) {
     return await embedError(
       async () => ({
         name: this.#name,