From cced2fc50690d5e8c41602599ac43a5dfbdb7404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Mon, 6 Jun 2022 16:02:28 +0200 Subject: [PATCH] Update Slack API dependency and remove workaround for stripping event details from logs #610 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ##### 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: https://github.com/infracloudio/botkube/commit/a6d91f21f25f00d679572e62b45cd2396d571075~ As the upstream fix has been merged (https://github.com/slack-go/slack/pull/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 --- go.mod | 2 +- go.sum | 4 +-- pkg/bot/export_test.go | 5 ---- pkg/bot/slack.go | 23 +----------------- pkg/bot/slack_test.go | 55 ------------------------------------------ 5 files changed, 4 insertions(+), 85 deletions(-) delete mode 100644 pkg/bot/export_test.go delete mode 100644 pkg/bot/slack_test.go diff --git a/go.mod b/go.mod index 102d520d4..ff2d711da 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 785e4d6f0..c63e003e9 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pkg/bot/export_test.go b/pkg/bot/export_test.go deleted file mode 100644 index 9b84f8848..000000000 --- a/pkg/bot/export_test.go +++ /dev/null @@ -1,5 +0,0 @@ -package bot - -func (b *SlackBot) StripUnmarshallingErrEventDetails(errMessage string) string { - return b.stripUnmarshallingErrEventDetails(errMessage) -} diff --git a/pkg/bot/slack.go b/pkg/bot/slack.go index 3f55fbd83..5687cf5ab 100644 --- a/pkg/bot/slack.go +++ b/pkg/bot/slack.go @@ -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()) @@ -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 { diff --git a/pkg/bot/slack_test.go b/pkg/bot/slack_test.go deleted file mode 100644 index 4a11f87fe..000000000 --- a/pkg/bot/slack_test.go +++ /dev/null @@ -1,55 +0,0 @@ -package bot_test - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/infracloudio/botkube/pkg/bot" -) - -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) - }) - } -}