validateBody() throws in case it is provided with no schema

[honzajavorek]

When my JSON Schema happened to be invalid JSON due to a typo, I got an error it is unparseable JSON. But Gavel didn't halt at that moment and continued to evaluate the situation further, which lead to an error being thrown:

error: TypeError: argument obj is required
 at Object.format (/Users/honza/Projects/apiary/node_modules/dredd/node_modules/media-typer/index.js:46:11)
 at validateBody (/Users/honza/Projects/apiary/node_modules/dredd/node_modules/gavel/lib/next/units/validateBody.js:225:30)
 at validateMessage (/Users/honza/Projects/apiary/node_modules/dredd/node_modules/gavel/lib/next/validateMessage.js:32:20)
...

That comes from

https://github.com/apiaryio/gavel.js/blob/master/lib/units/validateBody.js#L235

Which is caused by expected.bodySchema being evaluated to undefined on this line:

https://github.com/apiaryio/gavel.js/blob/master/lib/units/validateBody.js#L173

[artem-zakharchenko]

I vote for Gavel not handling any schema validation of any kind. Since we are using third-party libraries to perform JSON bodies validation, I suggest to attempt to execute the validator and catch the error, if any. What Gavel could do is to present the error in a more developer-friendly way, so it doesn't look like some internal exception. It must, however, remain an exception, and must not be silenced by putting it into errors or any part of the validation result.

error: TypeError: argument obj is required

Is indeed a confusing error message. Since the end validation library is responsible for more verbose error messages depending on exception scenarios (i.e. malformed or missing JSON schema), we can't presume the origin of their exception on the Gavel's side. Thus, we can tolerate all validator's exceptions with a generic error message:

Validating "${fieldName}" failed: ${originalErrorMessage}

---

On the relevant note, as a part of apiaryio/gavel-spec#48 I would like to introduce an interface for input validation of Gavel. We could gradually start to implement input validation, which may include validation of missing properties.

I don't see, however, how input validation would help in this way, since Gavel decides to validate "body" only if expected.body || expected.bodySchema.

Reading the issue once more, I see that we could perform a basic assertion that expected.bodySchema is a valid JSON: JSON.parse(expected.bodySchema). Would this eliminate this unexpected behavior?

I assume that specifying both is an invalid behavior, especially since bodySchema takes priority internally during validateBody:

gavel.js/lib/units/validateBody.js

Lines 173 to 179 in 223150f

	const [expectedTypeError, expectedType] = expected.bodySchema
	? getBodySchemaType(expected.bodySchema)
	: getBodyType(
	expected.body,
	expected.headers && expected.headers['content-type'],
	'expected'
	);

So, I'm not sure what is the true origin of this error... Is this a malformed or missing schema? If it's missing, why does it even get to validateBody, as body validation must be omitted here:

gavel.js/lib/validate.js

Lines 45 to 47 in 223150f

	if (isset(expected.body) || isset(expected.bodySchema)) {
	result.fields.body = validateBody(expected, real);
	}

Note to me in the past: the issue originates from expected.bodySchema being a malformed JSON. It's passed to the validator as-is, but, apparently, things go dramatically wrong there.

[artem-zakharchenko]

@honzajavorek do you know about the scenarios when Gavel received bodySchema as a string?

The tests I've discovered assert bodySchema being an Object. I believe some intermediate step may feed Gavel with another input (dredd-transactions maybe?). I'd like to understand that use case better.

My main concern is when it would come to input validation in Gavel I would love to get rid of values type ambiguity. So for a bodySchema to always be an Object. This also saves us in case of this issue, since malformed object cannot be constructed, so it would cause an error before the malformed data passes into Gavel (which is what we want). Like in this case malformed data originates from drafter, goes into dredd (I assume), and ends up in Gavel. And the last one now throws an error notifying about that. That doesn't seem right.

[honzajavorek]

API Elements provide the schema in textual way, but I don't think Gavel needs to respect that. We can parse it into an object in Dredd prior to validation. Depends on how the JSON Schema validators expect the schema, because if we were to parse it from string to object and then the validator expects a string again, that would be inefficient.

[honzajavorek]

When I think of it, it might be the case that in Dredd it is always the case Gavel gets schema as a text, but in Gavel tests it is an object. And I'm not sure about how it is in Apiary.

[artem-zakharchenko]

Considering current behavior (mixture of both object and string), I'd say that current validators we use can digest both types of JSON Schema. I wonder what AJV accepts. Perhaps, we should lean toward it if we plan to adopt it in the future.

[honzajavorek]

Yes, I think we should see what tv4/ajv expects and Gavel should lean to that. I'm totally for accepting the schema in only one type, not both.

[honzajavorek]

Note: we discussed this and realized the schema should be a string on input, because otherwise there would be no way for Gavel to support any other kind of schemas except for JSON Schema or generally a schema which can be represented as a JavaScript object.

Imagine Gavel gets XML support and would like to accept DTD, XML Schema, or RELAX NG. Another example - consider Gavel gets support for the GraphQL data modelling language and would like to be able to accept it as a schema.

[ApiaryBot]

🎉 This issue has been resolved in version 6.1.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀