Skip to content

Commit

Permalink
Do not log full Slack event details in case of unmarshalling error #583
Browse files Browse the repository at this point in the history
##### ISSUE TYPE
 - Bug fix Pull Request

##### SUMMARY

- Do not log full Slack event details in case of unmarshalling error

This is an issue of the upstream library. While I prepared a workaround, I also prepared a simple change in `slack-go/slack` repo: slack-go/slack#1067

**NOTE:** This pull request depends on #582. Please merge #582 first, and I will rebase this one.

Actual changes on this PR: 7813821

Fixes #579

##### TESTING

See the original issue for testing instruction: #579
You can use `pkosiec/botkube:slack-unmarshall-err` Docker image for testing the workaround.
Once you run my image and try to reproduce the issue, you'll see in the logs:

```
ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "user_huddle_changed"
ERRO[2022-05-19T16:00:57Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_join"
ERRO[2022-05-19T16:01:00Z] Slack unmarshalling error: RTM Error: Received unmapped event "sh_room_leave"
```
  • Loading branch information
pkosiec committed May 26, 2022
1 parent 40ade68 commit 0a2bf80
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pkg/bot/export_test.go
@@ -0,0 +1,5 @@
package bot

func (b *SlackBot) StripUnmarshallingErrEventDetails(errMessage string) string {
return b.stripUnmarshallingErrEventDetails(errMessage)
}
23 changes: 22 additions & 1 deletion pkg/bot/slack.go
Expand Up @@ -112,7 +112,7 @@ func (b *SlackBot) Start() {
log.Errorf("Slack outgoing event error: %+v", ev.Error())

case *slack.UnmarshallingErrorEvent:
log.Errorf("Slack unmarshalling error: %+v", ev.Error())
log.Errorf("Slack unmarshalling error: %+v", b.stripUnmarshallingErrEventDetails(ev.Error()))

case *slack.RateLimitedError:
log.Errorf("Slack rate limiting error: %+v", ev.Error())
Expand All @@ -126,6 +126,27 @@ func (b *SlackBot) Start() {
}
}

// Temporary workaround until the PR is merged: https://github.com/slack-go/slack/pull/1067
// There are just two cases when the full event details are concatenated with the error message: https://github.com/slack-go/slack/blob/v0.10.3/websocket_managed_conn.go#L474-L492
// Also, we need keep the JSON unmarshal error intact: https://github.com/slack-go/slack/blob/v0.10.3/websocket_managed_conn.go#L405
func (b *SlackBot) stripUnmarshallingErrEventDetails(errMessage string) string {
const errMsgSeparator = ": "
const prefix = "RTM Error"

if !strings.HasPrefix(errMessage, prefix) {
return errMessage
}

// has prefix, so let's split the message into parts
msgParts := strings.Split(errMessage, errMsgSeparator)
if len(msgParts) < 3 {
// impossible with current version of the dependency
return errMessage
}

return strings.Join(msgParts[:2], errMsgSeparator)
}

func (sm *slackMessage) HandleMessage(b *SlackBot) {
// Check if message posted in authenticated channel
info, err := sm.SlackClient.GetConversationInfo(sm.Event.Channel, true)
Expand Down
53 changes: 53 additions & 0 deletions pkg/bot/slack_test.go
@@ -0,0 +1,53 @@
package bot_test

import (
"fmt"
"github.com/infracloudio/botkube/pkg/bot"
"github.com/stretchr/testify/assert"
"testing"
)

func TestSlackBot_StripUnmarshallingErrEventDetails(t *testing.T) {
//given
sampleEvent := `{"type":"user_huddle_changed","user":{"id":"id","team_id":"team_id"}, "event_ts":"1652949120.004700"}`

testCases := []struct {
Name string
Input string
Expected string
}{
{
Name: "Unmapped event",
Input: fmt.Sprintf(`RTM Error: Received unmapped event "user_huddle_changed": %s`, sampleEvent),
Expected: `RTM Error: Received unmapped event "user_huddle_changed"`,
},
{
Name: "Unmarshalling error message",
Input: fmt.Sprintf(`RTM Error: Could not unmarshall event "user_huddle_changed": %s`, sampleEvent),
Expected: `RTM Error: Could not unmarshall event "user_huddle_changed"`,
},
{
Name: "JSON unmarshal error",
Input: "cannot unmarshal bool into Go value of type string",
Expected: "cannot unmarshal bool into Go value of type string",
},
{
Name: "JSON unmarshal error with colons",
// this is a real error when doing json.Unmarshal([]byte(`":::"`), &time)
Input: `parsing time "":::"" as ""2006-01-02T15:04:05Z07:00"": cannot parse ":::"" as "2006"`,
Expected: `parsing time "":::"" as ""2006-01-02T15:04:05Z07:00"": cannot parse ":::"" as "2006"`,
},
}

for _, testCase := range testCases {
t.Run(testCase.Name, func(t *testing.T) {
slackBot := &bot.SlackBot{}

// when
actual := slackBot.StripUnmarshallingErrEventDetails(testCase.Input)

// then
assert.Equal(t, testCase.Expected, actual)
})
}
}

0 comments on commit 0a2bf80

Please sign in to comment.