scrapinghub / slackbot

A chat bot for Slack (https://slack.com).
MIT License
1.26k stars 394 forks source link

No more stack traces to end-users #91

Closed owocki closed 7 years ago

owocki commented 8 years ago

What

Adds the ability to override error messages so that production end-users do not see stack traces.

Why

It's embarrassing to show stack traces to end users.

Img

exploits_of_a_mom

lins05 commented 8 years ago

Thanks for the patch (and the xkcd picture!). It makes sense to hide the tracebacks from end users sometimes, but I think ERRORS_TO can already did that.

You can just specify ERRORS_TO = "some-channel" in slackbot_settings.py, and invite the bot to that channel, then the bot would only send tracebacks to that channel instead of printing it in the end users' channel.

Still that can be improved a little bit: we can add a DEFAULT_ERROR_REPLY settings, which, if defined in slackbot_settings.py, could get a chance to overwrite the default "I had a problem handling xxx command" error message.

owocki commented 8 years ago

You can just specify ERRORS_TO = "some-channel" in slackbot_settings.py, and invite the bot to that channel, then the bot would only send tracebacks to that channel instead of printing it in the end users' channel.

Got it. The reason I haven't taken this route is that I have folks (mostly people who've forked my repo https://github.com/owocki/emojibot ) who are installing my code in slack teams that I am not in. Since I'm not in the slack teams, I can't be the target for the ERRORS_TO param.

Still that can be improved a little bit: we can add a DEFAULT_ERROR_REPLY settings, which, if defined in slackbot_settings.py, could get a chance to overwrite the default "I had a problem handling xxx command" error message.

If I understand your suggestion correctly, DEFAULT_ERROR_REPLY would reply in channel, and ERRORS_TO would be the target channel for stacktraces, right?

lins05 commented 8 years ago

DEFAULT_ERROR_REPLY would reply in channel, and ERRORS_TO would be the target channel for stacktraces, right?

I meant DEFAULT_ERROR_REPLY can be a string that is replied to the end user when errors happen during processing the users' message, just like DEFAULT_REPLY we already support, which is a string that's replied to the user when no plugin matches the message.

owocki commented 8 years ago

I meant DEFAULT_ERROR_REPLY can be a string that is replied to the end user when errors happen during processing the users' message, just like DEFAULT_REPLY we already support, which is a string that's replied to the user when no plugin matches the message.

Okie dokie, let me code that up.

jtatum commented 7 years ago

Thanks for the PR! Please reopen with DEFAULT_ERROR_REPLY 😄