Fixed error message when empty string passed as expiresIn or notBefore option#531
Fixed error message when empty string passed as expiresIn or notBefore option#531ziluvatar merged 2 commits intoauth0:masterfrom
Conversation
|
@MitMaro @ziluvatar may I ask you to review this change? Thanks! :) |
|
I wonder if the error message should be normalized or if this validation should be moved into the |
|
Good one @MitMaro , yes, probably it is better to update the schema, so if it is an string it has to have some value. |
f05328f to
8db0267
Compare
|
@MitMaro @ziluvatar thanks for comments! I moved validation to |
MitMaro
left a comment
There was a problem hiding this comment.
Awesome work!
Left two minor comment on cleaning up the tests a bit. :)
test/claim-exp.test.js
Outdated
| expect(err).to.be.instanceOf(Error); | ||
| expect(err).to.have.property('message', 'val is not a non-empty string or a valid number. val=""'); | ||
| expect(err).to.have.property('message') | ||
| .match(/"expiresIn" should be a number of seconds or string representing a timespan/); |
There was a problem hiding this comment.
You should be able to move this test above into the array in jwt.sign "expiresIn" option validation block.
test/claim-nbf.test.js
Outdated
| expect(err).to.be.instanceOf(Error); | ||
| expect(err).to.have.property('message', 'val is not a non-empty string or a valid number. val=""'); | ||
| expect(err).to.have.property('message') | ||
| .match(/"notBefore" should be a number of seconds or string representing a timespan/); |
There was a problem hiding this comment.
Ditto to above, you should be able to move this test above into the array in jwt.sign "expiresIn" option validation block.
|
@MitMaro indeed, good point, thanks for the comments! Updated in latest commit. |
|
@ziluvatar sorry for bothering again, any other comments on this PR? :) |
|
@andrewnester nothing to sorry about, thanks for the interest! I will merge this one since we were returning an error anyway, however, in this case it will return a better known descriptive error. |
This PR fixes TODO items to change error message when empty string passed as
expiresInornotBeforeoptions