Repository: slack-go/slack
Pull request #1086 (2022-07-04 15:58:19)

Add option to detect unmapped error
**Description**

This pull request adds:
- dedicated type error for `UnmappedError` 
- unit tests coverage

**Reason**

The library uses the `UnmarshallingErrorEvent` error in two cases:
1. when there was a real `json.Unmarshal`
2. or when there was no corresponding Go struct for unmarshal process

We want to be able to check what kind of error we got. As a result, we will be able to do the assertion whether we care about this unmarshal error or not. Example snippet:

```go
// handlingEvents holds event names for which we have corresponding business logic.
var handlingEvents = sets.NewString("message", "connected")

// ...trimmed...

switch ev := msg.Data.(type) {
case *slack.UnmarshallingErrorEvent:
  err, ok := ev.ErrorObj.(slack.UnmappedError)
  if ok && !handlingEvents.Has(err.EventType()) { // it's not an err for us, as we don't care about this event type.
    b.log.Debugf("Slack unmarshalling error: %+v", ev.Error())
    continue
  }

  b.log.Errorf("Slack unmarshalling error: %+v", ev.Error())
}
```


Currently, we have in logs:
```
ERRO[2022-06-15T11:30:02Z] Slack unmarshalling error: RTM Error: Received unmapped event "user_status_changed"
```

We could do the error string assertion but it's not a good practice, so we want to avoid that.

**Implementation**
I decided to leave the `UnmarshallingErrorEvent` as it is, but instead change 
```go
err := fmt.Errorf("RTM Error: Received unmapped event %q", typeStr)
```
to typed error. As a result, is **not a breaking change.**


Patch:
diff --git a/websocket_managed_conn.go b/websocket_managed_conn.go
index 14dea21..f107b2a 100644
--- a/websocket_managed_conn.go
+++ b/websocket_managed_conn.go
@@ -16,6 +16,31 @@ import (
 	"github.com/slack-go/slack/internal/timex"
 )
 
+// UnmappedError represents error occurred when there is no mapping between given event name
+// and corresponding Go struct.
+type UnmappedError struct {
+	// EventType returns event type name.
+	EventType string
+	// RawEvent returns raw event body.
+	RawEvent json.RawMessage
+
+	ctxMsg string
+}
+
+// NewUnmappedError returns new UnmappedError instance.
+func NewUnmappedError(ctxMsg, eventType string, raw json.RawMessage) *UnmappedError {
+	return &UnmappedError{
+		ctxMsg:    ctxMsg,
+		EventType: eventType,
+		RawEvent:  raw,
+	}
+}
+
+// Error returns human-readable error message.
+func (u UnmappedError) Error() string {
+	return fmt.Sprintf("%s: Received unmapped event %q", u.ctxMsg, u.EventType)
+}
+
 // ManageConnection can be called on a Slack RTM instance returned by the
 // NewRTM method. It will connect to the slack RTM API and handle all incoming
 // and outgoing events. If a connection fails then it will attempt to reconnect
@@ -474,7 +499,7 @@ func (rtm *RTM) handleEvent(typeStr string, event json.RawMessage) {
 	v, exists := EventMapping[typeStr]
 	if !exists {
 		rtm.Debugf("RTM Error - received unmapped event %q: %s\n", typeStr, string(event))
-		err := fmt.Errorf("RTM Error: Received unmapped event %q", typeStr)
+		err := NewUnmappedError("RTM Error", typeStr, event)
 		rtm.IncomingEvents <- RTMEvent{"unmarshalling_error", &UnmarshallingErrorEvent{err}}
 		return
 	}


Test patch:
diff --git a/websocket_managed_conn_test.go b/websocket_managed_conn_test.go
index 429ce8d..c7a8b70 100644
--- a/websocket_managed_conn_test.go
+++ b/websocket_managed_conn_test.go
@@ -1,6 +1,7 @@
 package slack_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"log"
 	"net/http"
@@ -9,6 +10,7 @@ import (
 
 	websocket "github.com/gorilla/websocket"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/slack-go/slack"
 	"github.com/slack-go/slack/slacktest"
@@ -313,3 +315,61 @@ func TestRTMSingleConnect(t *testing.T) {
 	assert.True(t, connectedReceived, "Should have received a connected event from the RTM instance.")
 	assert.True(t, testMessageReceived, "Should have received a test message from the server.")
 }
+
+func TestRTMUnmappedError(t *testing.T) {
+	const unmappedEventName = "user_status_changed"
+	// Set up the test server.
+	testServer := slacktest.NewTestServer()
+	go testServer.Start()
+
+	// Setup and start the RTM.
+	api := slack.New(testToken, slack.OptionAPIURL(testServer.GetAPIURL()))
+	rtm := api.NewRTM()
+	go rtm.ManageConnection()
+
+	// Observe incoming messages.
+	done := make(chan struct{})
+	var gotUnmarshallingError *slack.UnmarshallingErrorEvent
+	go func() {
+		for msg := range rtm.IncomingEvents {
+			switch ev := msg.Data.(type) {
+			case *slack.UnmarshallingErrorEvent:
+				gotUnmarshallingError = ev
+				rtm.Disconnect()
+			case *slack.DisconnectedEvent:
+				if ev.Intentional {
+					done <- struct{}{}
+					return
+				}
+			default:
+				t.Logf("Discarded event of type '%s' with content '%#v'", msg.Type, ev)
+			}
+		}
+	}()
+
+	// Send a message and sleep for some time to make sure the message can be processed client-side.
+	testServer.SendToWebsocket(fixSlackMessage(t, unmappedEventName))
+	<-done
+	testServer.Stop()
+
+	// Verify that we got the expected error with details
+	unmappedErr, ok := gotUnmarshallingError.ErrorObj.(*slack.UnmappedError)
+	require.True(t, ok)
+	assert.Equal(t, unmappedEventName, unmappedErr.EventType)
+}
+
+func fixSlackMessage(t *testing.T, eType string) string {
+	t.Helper()
+
+	m := slack.Message{
+		Msg: slack.Msg{
+			Type:      eType,
+			Text:      "Fixture Slack message",
+			Timestamp: fmt.Sprintf("%d", time.Now().Unix()),
+		},
+	}
+	msg, err := json.Marshal(m)
+	require.NoError(t, err)
+
+	return string(msg)
+}