Skip to content

Commit 914c164

Browse files
philsturgeonphilsturgeon
authored
Merge pull request APIDevTools#167 from stoplightio/fix/path-on-error
Throw errors with more meaningful paths
2 parents cf399e0 + 04b31a3 commit 914c164

File tree

7 files changed

+160
-7
lines changed

7 files changed

+160
-7
lines changed

‎lib/dereference.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ function dereference$Ref ($ref, path, pathFromRoot, parents, $refs, options) {
9696
// console.log('Dereferencing $ref pointer "%s" at %s', $ref.$ref, path);
9797

9898
let $refPath = url.resolve(path, $ref.$ref);
99-
let pointer = $refs._resolve($refPath, pathFromRoot, options);
99+
let pointer = $refs._resolve($refPath, path, options);
100100

101101
if (pointer === null) {
102102
return {

‎lib/ref.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
module.exports = $Ref;
44

55
const Pointer = require("./pointer");
6-
const { JSONParserError, JSONParserErrorGroup, ParserError, MissingPointerError, ResolverError, isHandledError } = require("./util/errors");
7-
const { safePointerToPath } = require("./util/url");
6+
const { JSONParserError, JSONParserErrorGroup, ParserError, MissingPointerError, ResolverError, InvalidPointerError, isHandledError, normalizeError } = require("./util/errors");
7+
const { safePointerToPath, stripHash, getHash } = require("./util/url");
88

99
/**
1010
* This class represents a single JSON reference and its resolved value.
@@ -61,11 +61,13 @@ $Ref.prototype.addError = function (err) {
6161
this.errors = [];
6262
}
6363

64+
// the path has been almost certainly set at this point,
65+
// but just in case something went wrong, let's inject path if necessary
6466
if (Array.isArray(err.errors)) {
65-
this.errors.push(...err.errors);
67+
this.errors.push(...err.errors.map(normalizeError));
6668
}
6769
else {
68-
this.errors.push(err);
70+
this.errors.push(normalizeError(err));
6971
}
7072
};
7173

@@ -117,7 +119,16 @@ $Ref.prototype.resolve = function (path, options, friendlyPath, pathFromRoot) {
117119
throw err;
118120
}
119121

120-
err.path = safePointerToPath(pathFromRoot);
122+
if (err.path === null) {
123+
err.path = safePointerToPath(getHash(pathFromRoot));
124+
}
125+
126+
if (err instanceof InvalidPointerError) {
127+
// this is a special case - InvalidPointerError is thrown when dereferencing external file,
128+
// but the issue is caused by the source file that referenced the file that undergoes dereferencing
129+
err.source = stripHash(pathFromRoot);
130+
}
131+
121132
this.addError(err);
122133
return null;
123134
}

‎lib/util/errors.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const JSONParserError = exports.JSONParserError = class JSONParserError extends
1111
this.code = "EUNKNOWN";
1212
this.message = message;
1313
this.source = source;
14-
this.path = [];
14+
this.path = null;
1515

1616
Ono.extend(this);
1717
}
@@ -122,3 +122,11 @@ function setErrorName (err) {
122122
exports.isHandledError = function (err) {
123123
return err instanceof JSONParserError || err instanceof JSONParserErrorGroup;
124124
};
125+
126+
exports.normalizeError = function (err) {
127+
if (err.path === null) {
128+
err.path = [];
129+
}
130+
131+
return err;
132+
};

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"components": {
3+
"messages": {
4+
"testMessage": {
5+
"payload": {
6+
"$ref": "#/components/schemas/testSchema"
7+
}
8+
}
9+
},
10+
"schemas": {
11+
"testSchema": {
12+
"type": "object",
13+
"properties": {
14+
"name": {
15+
"type": "string"
16+
},
17+
"test": {
18+
"$ref": "non-existing.json"
19+
}
20+
}
21+
}
22+
}
23+
}
24+
}

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
"use strict";
2+
3+
const chai = require("chai");
4+
const chaiSubset = require("chai-subset");
5+
chai.use(chaiSubset);
6+
const { expect } = chai;
7+
const $RefParser = require("../../..");
8+
const helper = require("../../utils/helper");
9+
const path = require("../../utils/path");
10+
const { InvalidPointerError, ResolverError, MissingPointerError } = require("../../../lib/util/errors");
11+
12+
describe("Report correct error source and path for", () => {
13+
it("schema with broken reference", async () => {
14+
const parser = new $RefParser();
15+
try {
16+
await parser.dereference({ foo: { bar: { $ref: "I do not exist" }}}, { continueOnError: true });
17+
helper.shouldNotGetCalled();
18+
}
19+
catch (err) {
20+
expect(err.errors).to.containSubset([
21+
{
22+
name: ResolverError.name,
23+
source: source => typeof source === "string",
24+
path: ["foo", "bar"],
25+
message: message => typeof message === "string",
26+
},
27+
]);
28+
}
29+
});
30+
31+
it("schema with a local reference pointing at property with broken external reference", async () => {
32+
const parser = new $RefParser();
33+
try {
34+
await parser.dereference(path.abs("specs/error-source/broken-external.json"), { continueOnError: true });
35+
helper.shouldNotGetCalled();
36+
}
37+
catch (err) {
38+
expect(err.errors).to.containSubset([
39+
{
40+
name: ResolverError.name,
41+
source: path.abs("specs/error-source/broken-external.json"),
42+
path: ["components", "schemas", "testSchema", "properties", "test"],
43+
message: message => typeof message === "string",
44+
},
45+
]);
46+
}
47+
});
48+
49+
it("schema with a missing local pointer and reference pointing at external file with broken external", async () => {
50+
const parser = new $RefParser();
51+
try {
52+
await parser.dereference(path.abs("specs/error-source/invalid-external.json"), { continueOnError: true });
53+
helper.shouldNotGetCalled();
54+
}
55+
catch (err) {
56+
expect(err.errors).to.containSubset([
57+
{
58+
name: MissingPointerError.name,
59+
source: path.abs("specs/error-source/invalid-external.json"),
60+
path: ["foo", "bar"],
61+
message: message => typeof message === "string",
62+
},
63+
{
64+
name: ResolverError.name,
65+
source: path.abs("specs/error-source/broken-external.json"),
66+
path: ["components", "schemas", "testSchema", "properties", "test"],
67+
message: message => typeof message === "string",
68+
},
69+
]);
70+
}
71+
});
72+
73+
it("schema with an invalid pointer", async () => {
74+
const parser = new $RefParser();
75+
try {
76+
await parser.dereference(path.abs("specs/error-source/invalid-pointer.json"), { continueOnError: true });
77+
helper.shouldNotGetCalled();
78+
}
79+
catch (err) {
80+
expect(err.errors).to.containSubset([
81+
{
82+
name: InvalidPointerError.name,
83+
source: path.abs("specs/error-source/invalid-pointer.json"),
84+
path: ["foo", "baz"],
85+
message: message => typeof message === "string",
86+
},
87+
]);
88+
}
89+
});
90+
});

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
"foo": {
3+
"baz": {
4+
"$ref": "./broken-external.json#/components/messages/testMessage/payload"
5+
},
6+
"bar": {
7+
"$ref": "#/foo/baz-2"
8+
},
9+
"bazinga": {
10+
"$ref": "/#foo/baz-2"
11+
}
12+
}
13+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"foo": {
3+
"baz": {
4+
"$ref": "/#foo/baz-2"
5+
}
6+
}
7+
}

0 commit comments

Comments
 (0)