Json validator hardening

[hamnis]

If the json validator encounters a schema which exists, but has the wrong meta-schema, the current implementation will crash hard.

This pull request fixes that issue by wrapping all calls to the underlying library in Either.catchNonFatal

[istreeter]

Hi @hamnis, I'll explain why I'm undecided about merging this change.

First, I'll give a bit of background for the original issue you describe...

If the json validator encounters a schema which exists, but has the wrong meta-schema, the current implementation will crash hard.

This problem you're referring to got addressed by merging #238. The problem with the wrong meta-schema was causing the json-validator to do a http lookup at a point in the code where we had not anticipated any I/O. So we addressed it by preventing the json-validator from following references to external URLs.

Also, the Enrich application never had a hard crash. The old error was caught on this line of Enrich, and it would generate a failed event.

The lag (slow processing) we saw with Enrich was entirely because of json-validator doing a slow http lookup for each event. Even if we wrapped this lookup in a Either.catchNonFatal, then it would not have prevented this slow lookup, and slow processing of events.

---

Anyway, back to the current PR.....

The call we make to schema.validate(...) is supposed to be a pure function (no I/O) that always returns a result. If it ever throws an exception then something very unexpected has happened!

Now of course, exceptions can always happen. So we have a choice to either catch the exception immediately (like your in PR) or we let the exception bubble up and we catch it here, which is what we've been doing so far.

The main reason to catch the exception early (like your PR) is that we create a SchemaViolation type of failed event, described here. It's a slightly more informative type of failed event, and it can be recovered by our standard event recovery processes. If we catch the exception later then we create a GenericError type of failed event.

The main reason to catch the exception later is that we also send the exception to Sentry which means Snowplow engineers get a notification and we know something bad has happened. And to re-iterate, any exception thrown from schema.validate is unexpected, so I think it's better to treat it as a genuine error in the code instead of treating as a problem with the data.

[hamnis]

Catching this in the library would allow other uses which does not trust java code to not throw exceptions. Now users may have to guard against that in the same way as enrich does.

[hamnis]

Closing due to lack of interest