Introduce failFast option

[P0lip]

This PR implements fastFail that is supposed to define how lenient the processing is.
Currently, json-schema-ref-parser bails upon the the first exception, regardless of it type - whether it's caused by resolver, parser, missing pointer, etc.
This is fine for the most of the time, yet there might cases where errors should be allowed.
fastFail=false meets that demand.

Key changes:

• fastFail - true by default for backwards compatibility
• exposed errors on each $RefParser instance
• @stoplight/yaml replaced js-yaml
• @stoplight/json replaced JSON.parse

None of these changes should be considered as breaking.

I tried to stay reasonable about errors and limit the scope to the user-related ones, therefore I didn't cover all spots.
If there is something really nasty happening in our own code, we should throw.

I'm not quite sure about the source of the error we set. At the moment, as stated in docs, it's equal to the uri of document where the faulty document was referenced.
The alternative option is simply set it to the uri of the document that caused the error.

[coveralls]

Pull Request Test Coverage Report for Build 383

• 175 of 185 (94.59%) changed or added relevant lines in 17 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.9%) to 94.788%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/bundle.js	2	3	66.67%
lib/parse.js	21	22	95.45%
lib/parsers/text.js	1	2	50.0%
lib/parsers/yaml.js	9	10	90.0%
lib/resolvers/file.js	3	4	75.0%
lib/util/url.js	4	5	80.0%
lib/pointer.js	10	12	83.33%
lib/util/errors.js	46	48	95.83%

Totals
Change from base Build 381:	0.9%
Covered Lines:	789
Relevant Lines:	821

---

💛 - Coveralls

[JamesMessinger]

Really good job on this PR. I haven't 100% finished reviewing it yet, so there may be a few more comments from me tomorrow. But so far I like everything I see 👍

[JamesMessinger]

@P0lip - Thanks for making all those changes. The PR looks good, but I'd like to discuss a design change and get you and @philsturgeon's opinions....

I'm thinking that whenever there's an error — whether it's one error or many, and whether failFast is true or false — it should result in a throw, not a return. The only time the library should return a normal result is when there were no errors. This leaves no ambiguity about whether the operation was successful or not.

To me, this seems like the most intuitive, idiomatic, and consistent behavior. But I'd like to hear what you guys think.

My proposal is that we remove the $RefParser.errors property and instead when failFast is true and one or more errors occur, we throw a JSONParserErrorGroup error, with its errors property set to the error(s) that occurred. The JSONParserErrorGroup.message property should be something like "3 errors occurred while reading my-schema.yaml". In addition, we'd add a JSONParserErrorGroup.files property, which is the $RefParser object that contains whatever was parsed successfully.

Thoughts?

[philsturgeon]

I like the sound of this, but in the interest of not marching along to a v9.0 release 3 days after a v8.0 release, perhaps we could sling this in as part of a v8.1 and get a TODO issue made up for getting this done later? Generally I try and balance great new changes with reducing major release rampage but YMMV.

[JamesMessinger]

@philsturgeon - I don't think this PR (as-is, or with my proposed changes) requires a major version bump. It's fully backward-compatible and opt-in behavior.

The failFast option is true by default, which means that as soon as an error occurs, it will be thrown (and will not be wrapped in a JSONParserErrorGroup). Only if the user sets failFast: false will the new behavior occur.

[philsturgeon]

OH Sorry I misread things, you are trying to change how the PR is working, it sounded like "oh I have an idea, let's change something bigger!" and I was trying to punt that. 😆

Yeah your thing sounds good. Exceptions for problems might be better than good and bad results coming back.

[P0lip]

@JamesMessinger
If I understand your proposal correctly, we'd like to throw in both cases, right?
The primary difference would be that with failFast set to true, we'd throw a sort of accumulated exception, while in the other scenario we'd simply bail upon the first exception.

Frankly speaking, I don't have any strong preference Both seems fairly similar and the potential change won't be difficult to implement, so I will the call up to you.
The biggest benefit of throwing in both scenarios is that you need to explicitly handle the exception, which, I believe, is something we'd like to enforce here.

[JamesMessinger]

Ok, cool. Sounds like we're all in agreement then. @P0lip - can you make the changes to the PR?

To recap:

1. Remove the errors property from the $RefParser class

   • This class will only be returned if everything is successful, so there's no need for an errors property

2. If failFast is true (which it is by default), then nothing changes.

   • The very first error gets thrown immediately
   • The error is not wrapped

3. If failFast is false, then all errors are collected into a JSONParserErrorGroup

   • Errors are always wrapped in a JSONParserErrorGroup, even if there is only 1 error
   • The JSONParserErrorGroup will be thrown (not returned)
   • The JSONParserErrorGroup.errors property is an array containing all the errors
   • The JSONParserErrorGroup.message property will be something like "3 errors occurred while reading my-schema.yaml"
   • The JSONParserErrorGroup.files property will be the $RefParser object, which contains whatever was successfully parsed

[P0lip]

@JamesMessinger my apologies for a tad delayed answer.
Yes, sounds good. Going to do it tomorrow!

[P0lip]

I believe I've addressed everything.
LMK what you think.

[JamesMessinger]

Hey guys (@P0lip and @philsturgeon). Just wanted to let y'all know that I haven't forgotten about this PR. I'm just crazy super insanely busy at work, and don't have any time to look at it yet. I'll review it this weekend, and hopefully merge and publish it. 🎉

[fmvilas]

This would be super useful for our https://playground.asyncapi.io. I came here to make this proposal but you guys figured out already 👏 Thanks a lot!

[philsturgeon]

@JamesMessinger could you try and get to it this weekend? Otherwise I'll have to hit that big green button and I'm on highly questionable home-made mead. 🥴

[JamesMessinger]

@philsturgeon - I'm on it. Sorry for the delay. I've been working 80-hour weeks for the past month and have had zero time for anything else

[philsturgeon]

@JamesMessinger oh of course! Thank you so much, you’re a champion. Hope everything settles down soon that sounds rough 😫

[JamesMessinger]

I've declared this weekend a vacation from work. Which means that instead of coding on work stuff all weekend, I'll be coding on APIDevTools stuff all weekend! 😁

Hooray for quarantine!

[JamesMessinger]

@P0lip and @philsturgeon - two things:

1. I renamed the failFast option to continueOnError. No functionality change. Just changed the name to be more clear and default to false rather than true.

2. The new dependencies that were added in this PR (@stoplight/json and @stoplight/yaml) don't work on IE or Edge. Please see Issue Stoplight dependencies don't support IE or Edge #165