Skip to content

Commit

Permalink
Update Slack API dependency and remove workaround for stripping event…
Browse files Browse the repository at this point in the history
… details from logs #610

##### ISSUE TYPE
 - Feature Pull Request
 - Bug fix Pull Request
 - Docs Pull Request

##### SUMMARY

~⚠️ **NOTE:** For convienence, this PR depends on #603 and needs to be rebased when #603 is merged (it should happen very soon). Actual changes: a6d91f21f25f00d679572e62b45cd2396d571075~

As the upstream fix has been merged (slack-go/slack#1067), this PR updates `slack-go/slack` dependency and removes no longer needed workaround.

See the original issue: #579
See the PR that introduced the workaround: #583

##### TESTING

> **NOTE:** You can also use `pkosiec/botkube:rm-slack-workaround` Docker image for testing. The image was repushed **after** rebase.

1. Checkout this PR
2. Edit `comm_config.yaml` and enable Slack
3. Export needed envs:

```bash
export KUBECONFIG=/Users/$USER/.kube/config
export LOG_LEVEL=info
```

4. Run BotKube locally:

```bash
go run ./cmd/botkube/main.go
```

5. Start Slack huddle and say something, then leave
6. Observe logs still without event details
  • Loading branch information
pkosiec committed Jun 6, 2022
1 parent 8314d73 commit cced2fc
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 85 deletions.
2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -13,7 +13,7 @@ require (
github.com/prometheus/client_golang v1.12.2
github.com/sha1sum/aws_signing_client v0.0.0-20200229211254-f7815c59d5c1
github.com/sirupsen/logrus v1.8.1
github.com/slack-go/slack v0.10.3
github.com/slack-go/slack v0.10.4-0.20220606002947-9fd6da5aee56
github.com/stretchr/testify v1.7.1
github.com/vrischmann/envconfig v1.3.0
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -1004,8 +1004,8 @@ github.com/sirupsen/logrus v1.6.0/go.mod h1:7uNnSEd1DgxDLC74fIahvMZmmYsHGZGEOFrf
github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE=
github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0=
github.com/slack-go/slack v0.10.3 h1:kKYwlKY73AfSrtAk9UHWCXXfitudkDztNI9GYBviLxw=
github.com/slack-go/slack v0.10.3/go.mod h1:hlGi5oXA+Gt+yWTPP0plCdRKmjsDxecdHxYQdlMQKOw=
github.com/slack-go/slack v0.10.4-0.20220606002947-9fd6da5aee56 h1:MH0qxpIb/gmsc/MAbsgMNAK3giE5Zd/6gH8yX/4wsrM=
github.com/slack-go/slack v0.10.4-0.20220606002947-9fd6da5aee56/go.mod h1:hlGi5oXA+Gt+yWTPP0plCdRKmjsDxecdHxYQdlMQKOw=
github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1:OnSkiWE9lh6wB0YB77sQom3nweQdgAjqCqsofrRNTgc=
github.com/smartystreets/assertions v1.0.0 h1:UVQPSSmc3qtTi+zPPkCXvZX9VvW/xT/NsRvKfwY81a8=
github.com/smartystreets/assertions v1.0.0/go.mod h1:kHHU4qYBaI3q23Pp3VPrmWhuIUrLW/7eUrw0BU5VaoM=
Expand Down
5 changes: 0 additions & 5 deletions pkg/bot/export_test.go

This file was deleted.

23 changes: 1 addition & 22 deletions pkg/bot/slack.go
Expand Up @@ -139,7 +139,7 @@ func (b *SlackBot) Start(ctx context.Context) error {
b.log.Errorf("Slack outgoing event error: %+v", ev.Error())

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

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

// 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)
}

// TODO: refactor - handle and send methods should be defined on Bot level

func (sm *slackMessage) HandleMessage(b *SlackBot) error {
Expand Down
55 changes: 0 additions & 55 deletions pkg/bot/slack_test.go

This file was deleted.

0 comments on commit cced2fc

Please sign in to comment.