Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show exra information in validation error messages #108

Merged
merged 2 commits into from
Nov 3, 2015

Conversation

tarmolov
Copy link
Member

@tarmolov tarmolov commented Nov 2, 2015

@@ -39,7 +40,11 @@ module.exports = function (value, name, declaration) {
case 'number':
var parsedValue = parseFloat(value);
if (Number.isNaN(parsedValue)) {
throw new ApiError(ApiError.BAD_REQUEST, 'invalid value for ' + name + ' parameter');
var errorMessage = util.format(
'invalid value for "%s" parameter, expected type "%s" but got "%d"',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you using %d if value was NaN?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you suggest using %s?

%d -> NaN
%s -> nan

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You used value, not parsedValue,
%s for value variable may be string or Object or undefined etc
%s for parsedValue always be NaN

@dfilatov
Copy link
Contributor

dfilatov commented Nov 3, 2015

lgtm except copy-paste

@@ -39,7 +40,11 @@ module.exports = function (value, name, declaration) {
case 'number':
var parsedValue = parseFloat(value);
if (Number.isNaN(parsedValue)) {
throw new ApiError(ApiError.BAD_REQUEST, 'invalid value for ' + name + ' parameter');
var errorMessage = util.format(
'invalid value for "%s" parameter, expected type "%s" but got "NaN"',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's show got type: typeof value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but please keep value as well

@tarmolov tarmolov merged commit a063c06 into master Nov 3, 2015
@tarmolov tarmolov deleted the error-information branch November 3, 2015 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants