-
Notifications
You must be signed in to change notification settings - Fork 124
fix: update blacklist handling to return instead of throwing an error #362
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
base: master
Are you sure you want to change the base?
Conversation
|
Could someone take a look at this please? |
| if (alias.action && route.hasBlacklist) { | ||
| if (this.checkBlacklist(route, alias.action)) { | ||
| this.logger.debug(` The '${alias.action}' action is in the blacklist!`); | ||
| throw new ServiceNotFoundError({ action: alias.action }); |
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.
Here you should keep the error throwing because we do it in whitelist as well.
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.
done
src/index.js
Outdated
| // Blacklist check | ||
| if (route.hasBlacklist) { | ||
| if (this.checkBlacklist(route, action.name)) { | ||
| this.logger.debug( |
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.
Please remove the log, no log message in whitelist as well.
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.
done
|
sory @icebob... took me agfain forever to continue here... too much stuff going on at work. |
|
ping. @icebob |
I recently introduced the Blacklist feature (different username @jappyjan , same person :D but this time using my work account ^^)
Tuns out that i accidentally introduced a bug which causes the moleculer broker/service to kill the process instead of just not executing an action.
This PR fixes it by replacing the throwing of an error when a service is blacklisted with a simple return like we do it for the whitelist too.