-
Notifications
You must be signed in to change notification settings - Fork 30
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
feat: integrate asyncapi-validator to validate incomming messages #53
Conversation
@derberg I've taken a totally different approach here. what do you think? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great mate 🚀
I only have few improvements and fix request but from the concept point of view -> well done
@WaleedAshraf wanna have a look?
@reselbob you asked me about this one a week ago, so I guess you'll be happy it is almost there. Feel free to suggest if you would expect something more. Can be done here or in a separate Pr, depending on complexity
removed custom hook documentation from README.md
@derberg made the suggested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
@@ -1,7 +1,11 @@ | |||
const { red, gray } = require('colors/safe'); | |||
|
|||
module.exports = (err, message, next) => { | |||
console.error(red(`❗ ${err.message}`)); | |||
if (err.stack) console.error(gray(err.stack.substr(err.stack.indexOf('\n')+1))); | |||
if(err.name === 'AsyncAPIValidationError'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indent seems broken 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
corrected. (I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more thing, sorry but I think it would be super cool to have it in the readme
please change this
#publish your message.
mqtt pub -t 'smartylighting/streetlights/1/0/event/{streetlightId}/lighting/measured' -h 'test.mosquitto.org' -m '{"id": 1, "lumens": 3, "sentAt": "2017-06-07T12:34:32.000Z"}'
# You should see the sent message in the logs of the previously started server.
to something like
#publish an invalid message.
mqtt pub -t 'smartylighting/streetlights/1/0/event/123/lighting/measured' -h 'test.mosquitto.org' -m '{"id": 1, "lumens": "3", "sentAt": "2017-06-07T12:34:32.000Z"}'
#publish a valid message
mqtt pub -t 'smartylighting/streetlights/1/0/event/123/lighting/measured' -h 'test.mosquitto.org' -m '{"id": 1, "lumens": 3, "sentAt": "2017-06-07T12:34:32.000Z"}'
#You should see the sent message in the logs of the previously started server.
#Notice that the server automatically validates incoming messages and logs out validation errors
what do you think?
@KhudaDad414 I would make code suggestion so you can just commit but unfortunately GH doesn't allow this to lines that were not "touched" by the PR |
@derberg Sorry, I was out of town yesterday. pushed the suggested changes. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving with pleasure
🎉 This PR is included in version 0.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Related issue(s)
Resolves #8