From dab722fadffb3e2216ab7a4557e3469cb14be179 Mon Sep 17 00:00:00 2001 From: Tom Elliott Date: Mon, 22 Jan 2024 04:17:39 +0000 Subject: [PATCH] Add errors to actions to allow for custom handling Actions now allow you to specify an error handling function (via the ErrorFunc function). The error handlers can post messages without interactive elements. Unit tests have been added for this path. Fixes #31 Fixes #10 --- action.go | 2 + api.go | 9 ++++ blocks.go | 8 ++++ error.go | 18 ++++++++ examples/error/main.go | 11 +++++ slack/action.go | 2 + slack/app_test.go | 2 +- slack/channel.go | 9 ++++ slack/ephemeral.go | 10 +++++ slack/error_test.go | 73 +++++++++++++++++++++++++++++++ slack/errors.go | 30 +++++++++++++ slack/event.go | 57 ++++++++++++++++++------ slack/examples_test.go | 2 +- slack/message.go | 10 +++++ slack/modal.go | 20 +++++++++ slack/receivemessage_test.go | 2 +- slack/receiveslashcommand_test.go | 2 +- slack/slackclient_test.go | 20 +++++++-- 18 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 error.go create mode 100644 slack/error_test.go diff --git a/action.go b/action.go index eda3a38..6403866 100644 --- a/action.go +++ b/action.go @@ -3,6 +3,8 @@ package spanner import "context" type Action interface { + HasError + Type() string Data() interface{} } diff --git a/api.go b/api.go index e40868b..5b1ee60 100644 --- a/api.go +++ b/api.go @@ -51,6 +51,7 @@ type SlashCommand interface { // It can be used to create blocks and handle submission or closing of the modal. type Modal interface { BlockUI + SubmitButton(title string) ModalSubmission CloseButton(title string) bool } @@ -75,6 +76,14 @@ type EphemeralSender interface { // Messages are constructed using BlockUI commands. type Message interface { BlockUI + HasError + + Channel(channelID string) +} + +type NonInteractiveMessage interface { + NonInteractiveBlockUI + HasError Channel(channelID string) } diff --git a/blocks.go b/blocks.go index 7f7bb38..b8c1638 100644 --- a/blocks.go +++ b/blocks.go @@ -2,9 +2,17 @@ package spanner // BlockUI allows the creation of Slack blocks in a message or modal. type BlockUI interface { + NonInteractiveBlockUI + InteractiveBlockUI +} + +type NonInteractiveBlockUI interface { Header(message string) PlainText(text string) Markdown(text string) +} + +type InteractiveBlockUI interface { TextInput(label string, hint string, placeholder string) string MultilineTextInput(label string, hint string, placeholder string) string Divider() diff --git a/error.go b/error.go new file mode 100644 index 0000000..2d23a59 --- /dev/null +++ b/error.go @@ -0,0 +1,18 @@ +package spanner + +import "context" + +type HasError interface { + ErrorFunc(ErrorFunc) +} + +type ErrorFunc func(ctx context.Context, ev ErrorEvent) + +type ErrorEvent interface { + SendMessage(channelID string) ErrorMessage + ReceiveError() error +} + +type ErrorMessage interface { + NonInteractiveMessage +} diff --git a/examples/error/main.go b/examples/error/main.go index cd55e55..49d27ae 100644 --- a/examples/error/main.go +++ b/examples/error/main.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "fmt" "log" "os" @@ -63,12 +64,22 @@ func main() { replyGood := ev.SendMessage(msg.Channel().ID()) replyGood.PlainText("This message should succeed") + replyGood.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + panic("did not expect this message to fail") + }) replyBad := ev.SendMessage("invalid_channel") replyBad.PlainText("This message will always fail to post") + replyBad.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + errorNotice := ev.SendMessage(msg.Channel().ID()) + errorNotice.PlainText(fmt.Sprintf("There was an error sending a message: %v", ev.ReceiveError())) + }) replySkipped := ev.SendMessage(msg.Channel().ID()) replySkipped.PlainText("This message should be skipped because of the previous error") + replySkipped.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + panic("did not expect this message to fail") + }) } return nil }) diff --git a/slack/action.go b/slack/action.go index eb80c39..c803b5a 100644 --- a/slack/action.go +++ b/slack/action.go @@ -11,6 +11,8 @@ type action interface { // exec performs and action and returns a payload to acknowledge the request as appropriate exec(ctx context.Context, req request) (interface{}, error) + + getErrorFunc() spanner.ErrorFunc } type actionQueue struct { diff --git a/slack/app_test.go b/slack/app_test.go index 33ef8b5..c04d320 100644 --- a/slack/app_test.go +++ b/slack/app_test.go @@ -10,7 +10,7 @@ import ( ) func TestHandlerIsCalledForEachEvent(t *testing.T) { - client := newTestClient() + client := newTestClient([]string{"ABC123"}) testApp := client.CreateApp() results := make(chan struct{}, 2) diff --git a/slack/channel.go b/slack/channel.go index 1912990..167132f 100644 --- a/slack/channel.go +++ b/slack/channel.go @@ -46,6 +46,15 @@ var _ action = &joinChannelAction{} type joinChannelAction struct { channelID string + errFunc spanner.ErrorFunc +} + +func (j *joinChannelAction) ErrorFunc(ef spanner.ErrorFunc) { + j.errFunc = ef +} + +func (j *joinChannelAction) getErrorFunc() spanner.ErrorFunc { + return j.errFunc } // Data implements action. diff --git a/slack/ephemeral.go b/slack/ephemeral.go index d5a1451..da5609a 100644 --- a/slack/ephemeral.go +++ b/slack/ephemeral.go @@ -30,6 +30,16 @@ var _ action = &sendEphemeralMessageAction{} type sendEphemeralMessageAction struct { text string + + errFunc spanner.ErrorFunc +} + +func (e *sendEphemeralMessageAction) ErrorFunc(ef spanner.ErrorFunc) { + e.errFunc = ef +} + +func (e *sendEphemeralMessageAction) getErrorFunc() spanner.ErrorFunc { + return e.errFunc } // Data implements action. diff --git a/slack/error_test.go b/slack/error_test.go new file mode 100644 index 0000000..8d90272 --- /dev/null +++ b/slack/error_test.go @@ -0,0 +1,73 @@ +package slack + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/slack-go/slack/slackevents" + "github.com/theothertomelliott/spanner" +) + +// TestErrorHandling verifies that error handlers are called appropriately +func TestErrorHandling(t *testing.T) { + client := newTestClient([]string{"ABC123"}) + testApp := client.CreateApp() + + go func() { + err := testApp.Run(handlerTestErrors) + if err != nil { + t.Errorf("error running app: %v", err) + } + }() + + // Send hello message + client.SendEventToApp(messageEvent( + slackevents.MessageEvent{ + Text: "hello", + Channel: "ABC123", + User: "DEF456", + }, + )) + + // Expect a single message and clear the message list + if len(client.messagesSent) != 2 { + t.Errorf("expected two messages to be sent, got %d", len(client.messagesSent)) + } + + firstBlocks, _ := json.MarshalIndent(client.messagesSent[0].blocks, "", " ") + if !strings.Contains(string(firstBlocks), `This message should succeed`) { + t.Errorf("first message content was not as expected, got: %v", string(firstBlocks)) + } + + secondBlocks, _ := json.MarshalIndent(client.messagesSent[1].blocks, "", " ") + if !strings.Contains(string(secondBlocks), `There was an error sending a message`) { + t.Errorf("first message content was not as expected, got: %v", string(secondBlocks)) + } +} + +func handlerTestErrors(ctx context.Context, ev spanner.Event) error { + if msg := ev.ReceiveMessage(); msg != nil && msg.Text() == "hello" { + replyGood := ev.SendMessage(msg.Channel().ID()) + replyGood.PlainText("This message should succeed") + replyGood.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + panic("did not expect this message to fail") + }) + + replyBad := ev.SendMessage("invalid_channel") + replyBad.PlainText("This message will always fail to post") + replyBad.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + errorNotice := ev.SendMessage(msg.Channel().ID()) + errorNotice.PlainText(fmt.Sprintf("There was an error sending a message: %v", ev.ReceiveError())) + }) + + replySkipped := ev.SendMessage(msg.Channel().ID()) + replySkipped.PlainText("This message should be skipped because of the previous error") + replySkipped.ErrorFunc(func(ctx context.Context, ev spanner.ErrorEvent) { + panic("did not expect this message to fail") + }) + } + return nil +} diff --git a/slack/errors.go b/slack/errors.go index f0eca0e..d3c0dca 100644 --- a/slack/errors.go +++ b/slack/errors.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/slack-go/slack" + "github.com/theothertomelliott/spanner" ) func renderSlackError(err error) error { @@ -16,3 +17,32 @@ func renderSlackError(err error) error { } return err } + +var _ spanner.ErrorEvent = &errorEvent{} + +func newErrorEvent(err error) *errorEvent { + q := &actionQueue{} + return &errorEvent{ + actionQueue: q, + sender: &MessageSender{ + actionQueue: q, + }, + err: err, + } +} + +type errorEvent struct { + actionQueue *actionQueue + sender *MessageSender + + err error +} + +func (e *errorEvent) SendMessage(channelID string) spanner.ErrorMessage { + return e.sender.SendMessage(channelID) +} + +// ReceiveError implements spanner.ErrorEvent. +func (e *errorEvent) ReceiveError() error { + return e.err +} diff --git a/slack/event.go b/slack/event.go index 83007f9..76739b4 100644 --- a/slack/event.go +++ b/slack/event.go @@ -18,6 +18,19 @@ type eventPopulator interface { var _ spanner.Event = &event{} +func newEvent() *event { + q := &actionQueue{} + return &event{ + eventType: "unknown", + state: eventState{ + actionQueue: q, + MessageSender: &MessageSender{ + actionQueue: q, + }, + }, + } +} + type event struct { hash string eventType string @@ -92,9 +105,19 @@ func (e *event) finishEvent( ctx context.Context, actionInterceptor spanner.ActionInterceptor, req request, +) error { + return finishEvent(ctx, actionInterceptor, req, e.state.actionQueue, true) +} + +func finishEvent( + ctx context.Context, + actionInterceptor spanner.ActionInterceptor, + req request, + actionQueue *actionQueue, + shouldAck bool, ) error { var payload interface{} - for _, a := range e.state.actionQueue.actions { + for _, a := range actionQueue.actions { var ( newPayload interface{} execFunc = func(ctx context.Context) error { @@ -105,9 +128,23 @@ func (e *event) finishEvent( ) err := actionInterceptor(ctx, a, execFunc) + if err != nil { + if ef := a.getErrorFunc(); ef != nil { + // Set up and run handler for error + errorEvent := newErrorEvent(err) + ef(ctx, errorEvent) + + // Process actions from error event + err := finishEvent(ctx, actionInterceptor, req, errorEvent.actionQueue, false) + if err != nil { + return fmt.Errorf("executing error event: %w", err) + } + } + return fmt.Errorf("executing action: %w", err) } + if newPayload != nil { if payload != nil { // TODO: Make this log configurable @@ -117,11 +154,14 @@ func (e *event) finishEvent( } } - // Acknowledge the event if payload == nil { payload = map[string]interface{}{} } - req.client.Ack(req.req, payload) + + if shouldAck { + // Acknowledge the event + req.client.Ack(req.req, payload) + } return nil } @@ -136,16 +176,7 @@ type eventPopulation struct { } func parseCombinedEvent(ctx context.Context, client socketClient, ce combinedEvent) *event { - q := &actionQueue{} - out := &event{ - eventType: "unknown", - state: eventState{ - actionQueue: q, - MessageSender: &MessageSender{ - actionQueue: q, - }, - }, - } + out := newEvent() defer func() { // Set clients in metadata diff --git a/slack/examples_test.go b/slack/examples_test.go index 3571627..4593f53 100644 --- a/slack/examples_test.go +++ b/slack/examples_test.go @@ -15,7 +15,7 @@ import ( // TestGettingStarted verifies that the code in examples/gettingstarted // interacts with Slack in the expected way func TestGettingStarted(t *testing.T) { - client := newTestClient() + client := newTestClient([]string{"ABC123"}) testApp := client.CreateApp() go func() { diff --git a/slack/message.go b/slack/message.go index ef39019..526d3b4 100644 --- a/slack/message.go +++ b/slack/message.go @@ -36,6 +36,16 @@ type message struct { currentEventDepth int actionMessageTS string unsent bool + + errFunc spanner.ErrorFunc +} + +func (m *message) ErrorFunc(ef spanner.ErrorFunc) { + m.errFunc = ef +} + +func (m *message) getErrorFunc() spanner.ErrorFunc { + return m.errFunc } func (m *message) Type() string { diff --git a/slack/modal.go b/slack/modal.go index b0b8e89..04b1624 100644 --- a/slack/modal.go +++ b/slack/modal.go @@ -31,6 +31,16 @@ type modal struct { submitText *string closeText *string + + errFunc spanner.ErrorFunc +} + +func (m *modal) ErrorFunc(ef spanner.ErrorFunc) { + m.errFunc = ef +} + +func (m *modal) getErrorFunc() spanner.ErrorFunc { + return m.errFunc } type updateType int @@ -175,6 +185,16 @@ type modalSubmission struct { NextModal *modal `json:"next_modal"` parent *modal + + errFunc spanner.ErrorFunc +} + +func (m *modalSubmission) ErrorFunc(ef spanner.ErrorFunc) { + m.errFunc = ef +} + +func (m *modalSubmission) getErrorFunc() spanner.ErrorFunc { + return m.errFunc } func (m *modalSubmission) PushModal(title string) spanner.Modal { diff --git a/slack/receivemessage_test.go b/slack/receivemessage_test.go index e0df4f2..1ced528 100644 --- a/slack/receivemessage_test.go +++ b/slack/receivemessage_test.go @@ -9,7 +9,7 @@ import ( ) func TestReceiveMessageContent(t *testing.T) { - client := newTestClient() + client := newTestClient([]string{"ABC123"}) testApp := client.CreateApp() message := slackevents.MessageEvent{ diff --git a/slack/receiveslashcommand_test.go b/slack/receiveslashcommand_test.go index 2289b8c..121acdd 100644 --- a/slack/receiveslashcommand_test.go +++ b/slack/receiveslashcommand_test.go @@ -9,7 +9,7 @@ import ( ) func TestReceiveSlashCommand(t *testing.T) { - client := newTestClient() + client := newTestClient([]string{"ABC123"}) testApp := client.CreateApp() slashCommand := slack.SlashCommand{ diff --git a/slack/slackclient_test.go b/slack/slackclient_test.go index 404ca12..78a2d13 100644 --- a/slack/slackclient_test.go +++ b/slack/slackclient_test.go @@ -2,6 +2,7 @@ package slack import ( "context" + "fmt" "sync" "github.com/slack-go/slack" @@ -10,11 +11,17 @@ import ( "github.com/theothertomelliott/spanner" ) -func newTestClient() *testClient { +func newTestClient(validChannels []string) *testClient { + validChannelMap := make(map[string]struct{}, len(validChannels)) + for _, c := range validChannels { + validChannelMap[c] = struct{}{} + } + return &testClient{ - Events: make(chan socketmode.Event), - stop: make(chan struct{}), - postEvent: make(chan interface{}, 10), + Events: make(chan socketmode.Event), + stop: make(chan struct{}), + postEvent: make(chan interface{}, 10), + validChannels: validChannelMap, } } @@ -24,6 +31,8 @@ type testClient struct { messagesSent []sentMessage messagesUpdated []updatedMessage + validChannels map[string]struct{} + Events chan socketmode.Event postEvent chan interface{} @@ -90,6 +99,9 @@ func (r *testClient) RunContext(context.Context) error { func (*testClient) Ack(req socketmode.Request, payload ...interface{}) {} func (c *testClient) SendMessageWithMetadata(ctx context.Context, channelID string, blocks []slack.Block, metadata slack.SlackMetadata) (string, string, string, error) { + if _, ok := c.validChannels[channelID]; !ok { + return "", "", "", fmt.Errorf("invalid channel: %s", channelID) + } c.messagesSent = append(c.messagesSent, sentMessage{ channelID: channelID, blocks: blocks,