config option to silence noisy error when using "non enabled" commands

[gkspranger]

as a YB dev/ops person, i would like a config option to silence the noisy error that occurs for all commands that are "non enabled" and yet a YB user triggered, so that i don't have to have unpleasant conversations with my YB users about "why" said commands are "non enabled"

e.g.

now

!disabledcmd :boom: Error in template greg: template is disabled in this Yetibot :boom:

preferred

config option set to silence noisy errors !disabledcmd non-event

in relation to issue #134 -- if the commands are hidden from "help view", then it is highly unlikely a YB user would even see/issue this noisy error .. that said, let's not underestimate our YB users "curiosity" and allow for a "non event" -- even if they get lucky triggering a "non enabled" command ..

right around :: https://github.com/yetibot/yetibot.core/blob/master/src/yetibot/core/hooks.clj:100

thanks !!

[devth]

Cool! So it would only prevent errors on disabled commands, right? i.e. if an enabled command threw an error they'd still see it.

[gkspranger]

hrmmm .. ok, let me re-think this and zoom out a little ..

if a command exists AND is disabled AND a user invokes said command, i guess i want the same error message i get when i invoke an "unknown topic"

if a command exists AND is enabled AND a user invokes the command, do the thing

so if we really want to keep the "foo command is disabled in this YB" output -- i guess this request could be altered to ask that i have a config option to use the default "unknown topic" error message if the command is disabled AND is invoked -- INSTEAD OF the default "foo command is disabled in this YB" error message .. ORRRR -- we drop the "is disabled" all together and just default to the "unknown topic" error message -- since the argument could be made that a disabled command is the same as an unknown topic -- at least from the user's perspective ..

i hope this all makes sense ..

which has now spurred a new idea -- a configurable error message for "unknown topics" .. will think about it and open a new issue once it is clear in my head ..

thanks again !!

[devth]

I could see a case for disabled commands pretending to be unknown. Interesting idea. And yes we could make it work either way with config. Not clear which route is better. 🤔

[gkspranger]

i am having a hard time finding a justification to noisily "error" to the user that a command exists, but has been disabled .. the reasoning i am following is that users don't host the bot, operators do -- and they have disabled the command for a reason .. so why create headaches for said operators ??

anyhoo -- just thinking aloud ..

[devth]

Appreciate your perspective! I think I agree.

[devth]

Working on this next. After giving it some thought, I like the simplicity of silencing by default and not even making it a config option. It will still be subject to fallback command, which is configurable. By default, it's enabled and set to help:

https://github.com/yetibot/yetibot.core/blob/9d1403f7d8a3c17b1b8f45baead8fc32bc10d7cf/src/yetibot/core/models/default_command.clj#L10-L28

[gkspranger]

oh nice !! i didn't even know about the fallback command .. that allows me to craft a custom "error message" when a user issues a command that is not known ..

brilliant -- thanks !!

[devth]

Ok, I think I have this in a good place. I cleaned up all the possibilities for command handling. They were previously nested in multiple levels and it wasn't super clear. Now we have:

https://github.com/yetibot/yetibot.core/blob/7bcb67215f8c6eb3c9a8e19f5d94d7935e210822/src/yetibot/core/hooks.clj#L87-L116

which clearly shows the 6 possibilities.

As an example, with this config:

   :yetibot-command-fallback-enabled "true"
   :yetibot-default-command "custom"
   :yetibot-command-whitelist-0 "echo"
   :yetibot-command-whitelist-1 "list"
   :yetibot-command-whitelist-2 "custom"

Here's what a customized default handler could look like, defined with an alias named custom:

Note that the alias custom must be specifically whitelisted, as well as any other alias that should be enabled (this wouldn't be the case if using blacklists).

You'll notice I updated the help docs too to indicate whether fallback commands are enabled, and if so what the fallback/default command is.

If fallback is not enabled and the user enters a disabled command, Yetibot returns nothing.

~🤔 Hmm, I think I should rename :yetibot-default-command config to be :yetibot-command-fallback for consistency.~ Not going to rename for now.

[devth]

And here's an example with no default command configured, it falls back to help:

[gkspranger]

very nice -- thanks for the example and the work ..

you may not like me very much after this -- but if i may inject my opinion about aliases .. the big question is -- do we really want to subject them to the same whitelist/blacklist scrutiny a normal command gets ??

here is my logic ::

1. an alias is dynamic << meaning it is not something that goes in a config/code file -- it is something that has to be created with YB, while it is running
2. i can only create alias commands from "real" commands that exist and have been whitelisted (>> or at least not blacklisted)
3. therefore, any alias a user creates should be allowed and not checked with command_enabled?, simply because it is composed of other commands and is "dynamic"

also -- if !alias is enabled by default (<< foundation command) and i am using a whitelist -- every time someone wants to create a new, useful alias, they will need to create it and then ask the YB operator to whitelist it ..

or maybe i am missing a more efficient workflow ??

[devth]

lol 😂 😂

I actually had a similar thought on aliases, but I also wanted to get the latest thinking merged and let it settle.

Your logic makes total sense, and I agree. I think this should be a simpler fix too.

[gkspranger]

👍 again, thanks for the work and being open to outside opinions ..

i am coming from the world of Hubot (<< i am sure you know it, but link just in case not :: https://hubot.github.com/) .. and in that world -- the core is very small, and everything you want to "use" is a plugin -- commands, adapters, middleware, etc .. your impl is very "batteries included" -- which is great, because it means there is less work for me to do on the whole, but at the same time, i have to be very measured with what i "turn on", because then i am supporting it AND not all YB users are created the same ..

anyhoo -- this is great, it gets us a lot closer to where we want to be in the end ..

[devth]

This is great feedback. I am somewhat familiar with Hubot but honestly I've never run it. It's actually what originally inspired me to start building a chat bot while I was learning Clojure in 2011 😁

I've thought about pulling apart the core into a more minimal setup but haven't seen a strong case for it, and would also want the ability to dynamically compose modules via config (as opposed to leiningen dependency information). It's possible, but hasn't been built yet.

[devth]

Aliases are always enabled in https://github.com/yetibot/yetibot.core/pull/138!