Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

HeadersJsonExample doesn't use the tv4 headers error message coercion #360

Open
artem-zakharchenko opened this issue Dec 2, 2019 · 2 comments

Comments

@artem-zakharchenko
Copy link
Contributor

Intended behavior

Currently a HeadersJsonExample class takes the raw output of the TV4 validator and coerces them using a custom coercion function to a header-friendly custom error message format:

module.exports = (message, expectedHeaders) => {
let newMessage;
if (message.includes('Missing required property:')) {
const headerName = message.split('Missing required property: ')[1];
newMessage = `Header '${headerName}' is missing`;
} else if (message.includes('No enum match for: ')) {
const splitted = message.split('\' No enum match for: "');
const headerName = splitted[0].replace(/^At '\//, '');
const headerValue = splitted[1].replace(/"$/, '');
const expected = caseless(expectedHeaders).get(headerName);
newMessage = `Header '${headerName}' has value '${headerValue}' instead of '${expected}'`;
} else {
throw new Error(
"Unknown tv4 error message can't convert to headers message."
);
}
return newMessage;
};

for (let i = 0; i < result.length; i++) {
resultCopy[i].message = tv4ToHeadersMessage(
resultCopy[i].message,
this.expected
);
}

However, that coerced message is not being set on the end result object. You can verify that by looking at the headers integration test suite, which asserts the raw tv4 validation error message, instead of the coerced one (and tests pass):

it('has explanatory message', () => {
expect(result.fields.headers)
.to.have.errorAtIndex(0)
.withMessage(
`At '/content-type' Missing required property: content-type`
);
});

Expected behavior

The original intention in the source code implies that the custom coercion should be used for the headers error messages. I'd say we need to treat the original intention as the specification.

@artem-zakharchenko
Copy link
Contributor Author

artem-zakharchenko commented Dec 2, 2019

Caution: This change implies an entire Gavel -> Dredd update cycle, because there are structures that contain the "wrong" "At '...'" error format. Changing this error format is considered a breaking change.

@artem-zakharchenko
Copy link
Contributor Author

I think after migrating to AJV for JSD4 validation I've added a coercion for AJV to mimic the previous error messages. This issue should be fixed in that coercion logic.

getBackwardCompatibleErrorMessage(ajv, ajvError, pointer, property) {
const { keyword, data, params } = ajvError;
switch (keyword) {
case 'type':
return `At '${pointer}' Invalid type: ${typeof data} (expected ${
params.type
})`;
case 'required':
return `At '${pointer}' Missing required property: ${last(property)}`;
case 'enum':
return `At '${pointer}' No enum match for: "${data}"`;
default:
return ajv.errorsText([ajvError]);
}
}
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant