Throw errors with more meaningful paths

Author: P0lip

lib/dereference.js

@@ -96,7 +96,7 @@ function dereference$Ref ($ref, path, pathFromRoot, parents, $refs, options) {
   // console.log('Dereferencing $ref pointer "%s" at %s', $ref.$ref, path);
 
   let $refPath = url.resolve(path, $ref.$ref);
-  let pointer = $refs._resolve($refPath, pathFromRoot, options);
+  let pointer = $refs._resolve($refPath, path, options);
 
   if (pointer === null) {
     return {

lib/ref.js

@@ -3,8 +3,8 @@
 module.exports = $Ref;
 
 const Pointer = require("./pointer");
-const { JSONParserError, JSONParserErrorGroup, ParserError, MissingPointerError, ResolverError, isHandledError } = require("./util/errors");
-const { safePointerToPath } = require("./util/url");
+const { JSONParserError, JSONParserErrorGroup, ParserError, MissingPointerError, ResolverError, InvalidPointerError, isHandledError, normalizeError } = require("./util/errors");
+const { safePointerToPath, stripHash, getHash } = require("./util/url");
 
 /**
  * This class represents a single JSON reference and its resolved value.
@@ -61,11 +61,13 @@ $Ref.prototype.addError = function (err) {
     this.errors = [];
   }
 
+  // the path has been almost certainly set at this point,
+  // but just in case something went wrong, let's inject path if necessary
   if (Array.isArray(err.errors)) {
-    this.errors.push(...err.errors);
+    this.errors.push(...err.errors.map(normalizeError));
   }
   else {
-    this.errors.push(err);
+    this.errors.push(normalizeError(err));
   }
 };
 
@@ -117,7 +119,16 @@ $Ref.prototype.resolve = function (path, options, friendlyPath, pathFromRoot) {
       throw err;
     }
 
-    err.path = safePointerToPath(pathFromRoot);
+    if (err.path === null) {
+      err.path = safePointerToPath(getHash(pathFromRoot));
+    }
+
+    if (err instanceof InvalidPointerError) {
+      // this is a special case - InvalidPointerError is thrown when dereferencing external file,
+      // but the issue is caused by the source file that referenced the file that undergoes dereferencing
+      err.source = stripHash(pathFromRoot);
+    }
+
     this.addError(err);
     return null;
   }

lib/util/errors.js

@@ -11,7 +11,7 @@ const JSONParserError = exports.JSONParserError = class JSONParserError extends
     this.code = "EUNKNOWN";
     this.message = message;
     this.source = source;
-    this.path = [];
+    this.path = null;
 
     Ono.extend(this);
   }
@@ -122,3 +122,11 @@ function setErrorName (err) {
 exports.isHandledError = function (err) {
   return err instanceof JSONParserError || err instanceof JSONParserErrorGroup;
 };
+
+exports.normalizeError = function (err) {
+  if (err.path === null) {
+    err.path = [];
+  }
+
+  return err;
+};

test/specs/error-source/broken-external.json

@@ -0,0 +1,24 @@
+{
+  "components": {
+    "messages": {
+      "testMessage": {
+        "payload": {
+          "$ref": "#/components/schemas/testSchema"
+        }
+      }
+    },
+    "schemas": {
+      "testSchema": {
+        "type": "object",
+        "properties": {
+          "name": {
+            "type": "string"
+          },
+          "test": {
+            "$ref": "non-existing.json"
+          }
+        }
+      }
+    }
+  }
+}

test/specs/error-source/error-source.spec.js

@@ -0,0 +1,90 @@
+"use strict";
+
+const chai = require("chai");
+const chaiSubset = require("chai-subset");
+chai.use(chaiSubset);
+const { expect } = chai;
+const $RefParser = require("../../..");
+const helper = require("../../utils/helper");
+const path = require("../../utils/path");
+const { InvalidPointerError, ResolverError, MissingPointerError } = require("../../../lib/util/errors");
+
+describe("Report correct error source and path for", () => {
+  it("schema with broken reference", async () => {
+    const parser = new $RefParser();
+    try {
+      await parser.dereference({ foo: { bar: { $ref: "I do not exist" }}}, { continueOnError: true });
+      helper.shouldNotGetCalled();
+    }
+    catch (err) {
+      expect(err.errors).to.containSubset([
+        {
+          name: ResolverError.name,
+          source: source => typeof source === "string",
+          path: ["foo", "bar"],
+          message: message => typeof message === "string",
+        },
+      ]);
+    }
+  });
+
+  it("schema with a local reference pointing at property with broken external reference", async () => {
+    const parser = new $RefParser();
+    try {
+      await parser.dereference(path.abs("specs/error-source/broken-external.json"), { continueOnError: true });
+      helper.shouldNotGetCalled();
+    }
+    catch (err) {
+      expect(err.errors).to.containSubset([
+        {
+          name: ResolverError.name,
+          source: path.abs("specs/error-source/broken-external.json"),
+          path: ["components", "schemas", "testSchema", "properties", "test"],
+          message: message => typeof message === "string",
+        },
+      ]);
+    }
+  });
+
+  it("schema with a missing local pointer and reference pointing at external file with broken external", async () => {
+    const parser = new $RefParser();
+    try {
+      await parser.dereference(path.abs("specs/error-source/invalid-external.json"), { continueOnError: true });
+      helper.shouldNotGetCalled();
+    }
+    catch (err) {
+      expect(err.errors).to.containSubset([
+        {
+          name: MissingPointerError.name,
+          source: path.abs("specs/error-source/invalid-external.json"),
+          path: ["foo", "bar"],
+          message: message => typeof message === "string",
+        },
+        {
+          name: ResolverError.name,
+          source: path.abs("specs/error-source/broken-external.json"),
+          path: ["components", "schemas", "testSchema", "properties", "test"],
+          message: message => typeof message === "string",
+        },
+      ]);
+    }
+  });
+
+  it("schema with an invalid pointer", async () => {
+    const parser = new $RefParser();
+    try {
+      await parser.dereference(path.abs("specs/error-source/invalid-pointer.json"), { continueOnError: true });
+      helper.shouldNotGetCalled();
+    }
+    catch (err) {
+      expect(err.errors).to.containSubset([
+        {
+          name: InvalidPointerError.name,
+          source: path.abs("specs/error-source/invalid-pointer.json"),
+          path: ["foo", "baz"],
+          message: message => typeof message === "string",
+        },
+      ]);
+    }
+  });
+});

test/specs/error-source/invalid-external.json

@@ -0,0 +1,13 @@
+{
+  "foo": {
+    "baz": {
+      "$ref": "./broken-external.json#/components/messages/testMessage/payload"
+    },
+    "bar": {
+      "$ref": "#/foo/baz-2"
+    },
+    "bazinga": {
+      "$ref": "/#foo/baz-2"
+    }
+  }
+}

test/specs/error-source/invalid-pointer.json

@@ -0,0 +1,7 @@
+{
+  "foo": {
+    "baz": {
+      "$ref": "/#foo/baz-2"
+    }
+  }
+}