From 4e9b8e17963e00b1ecfcbf47e870d1510bfee877 Mon Sep 17 00:00:00 2001 From: ryannjohnson-range <69135312+ryannjohnson-range@users.noreply.github.com> Date: Fri, 26 Feb 2021 03:12:02 +0000 Subject: [PATCH 1/2] Add OptionOnWarning for the web api client (#1) The Slack web api returns a "warning" property on response objects. The requests succeed, but this field will return a comma-separated list of warning codes that range from "superfluous_charset" to "method_deprecated", as well as extra messages in the .response_metadata.warnings property. This option allows applications to get notified when these warnings exist. --- misc.go | 22 ++++++++++++++++++++++ slack.go | 24 ++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/misc.go b/misc.go index 821bda869..7e60b8f4d 100644 --- a/misc.go +++ b/misc.go @@ -26,6 +26,7 @@ import ( type SlackResponse struct { Ok bool `json:"ok"` Error string `json:"error"` + Warning string `json:"warning"` ResponseMetadata ResponseMetadata `json:"response_metadata"` } @@ -44,6 +45,27 @@ func (t SlackResponse) Err() error { return errors.New(t.Error) } +type warner interface { + Warn() *Warning +} + +// Warning provides warning from the web api. +// https://api.slack.com/web#slack-web-api__evaluating-responses +type Warning struct { + Codes []string + Warnings []string +} + +func (t SlackResponse) Warn() *Warning { + if t.Warning == "" && len(t.ResponseMetadata.Warnings) == 0 { + return nil + } + return &Warning{ + Codes: strings.Split(t.Warning, ","), + Warnings: t.ResponseMetadata.Warnings, + } +} + // RateLimitedError represents the rate limit respond from slack type RateLimitedError struct { RetryAfter time.Duration diff --git a/slack.go b/slack.go index 143673656..0538d2432 100644 --- a/slack.go +++ b/slack.go @@ -63,6 +63,7 @@ type Client struct { debug bool log ilogger httpclient httpClient + onWarning func(*Warning) } // Option defines an option for a Client @@ -89,6 +90,13 @@ func OptionLog(l logger) func(*Client) { } } +// OptionOnWarning set callback for response warnings. +func OptionOnWarning(fn func(*Warning)) func(*Client) { + return func(c *Client) { + c.onWarning = fn + } +} + // OptionAPIURL set the url for the client. only useful for testing. func OptionAPIURL(u string) func(*Client) { return func(c *Client) { c.endpoint = u } @@ -153,10 +161,22 @@ func (api *Client) Debug() bool { // post to a slack web method. func (api *Client) postMethod(ctx context.Context, path string, values url.Values, intf interface{}) error { - return postForm(ctx, api.httpclient, api.endpoint+path, values, intf, api) + err := postForm(ctx, api.httpclient, api.endpoint+path, values, intf, api) + if w, ok := intf.(warner); ok { + if warning := w.Warn(); warning != nil && api.onWarning != nil { + api.onWarning(warning) + } + } + return err } // get a slack web method. func (api *Client) getMethod(ctx context.Context, path string, values url.Values, intf interface{}) error { - return getResource(ctx, api.httpclient, api.endpoint+path, values, intf, api) + err := getResource(ctx, api.httpclient, api.endpoint+path, values, intf, api) + if w, ok := intf.(warner); ok { + if warning := w.Warn(); warning != nil && api.onWarning != nil { + api.onWarning(warning) + } + } + return err } From 0b8d7ec4af2af1f95503122544ff4448e0772fd4 Mon Sep 17 00:00:00 2001 From: ryannjohnson-range <69135312+ryannjohnson-range@users.noreply.github.com> Date: Fri, 5 Mar 2021 21:26:34 +0000 Subject: [PATCH 2/2] Add path and values to onWarning callback (#2) --- slack.go | 8 ++++---- views_test.go | 33 +++++++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/slack.go b/slack.go index 0538d2432..319d84d68 100644 --- a/slack.go +++ b/slack.go @@ -63,7 +63,7 @@ type Client struct { debug bool log ilogger httpclient httpClient - onWarning func(*Warning) + onWarning func(w *Warning, path string, values url.Values) } // Option defines an option for a Client @@ -91,7 +91,7 @@ func OptionLog(l logger) func(*Client) { } // OptionOnWarning set callback for response warnings. -func OptionOnWarning(fn func(*Warning)) func(*Client) { +func OptionOnWarning(fn func(w *Warning, path string, values url.Values)) func(*Client) { return func(c *Client) { c.onWarning = fn } @@ -164,7 +164,7 @@ func (api *Client) postMethod(ctx context.Context, path string, values url.Value err := postForm(ctx, api.httpclient, api.endpoint+path, values, intf, api) if w, ok := intf.(warner); ok { if warning := w.Warn(); warning != nil && api.onWarning != nil { - api.onWarning(warning) + api.onWarning(warning, path, values) } } return err @@ -175,7 +175,7 @@ func (api *Client) getMethod(ctx context.Context, path string, values url.Values err := getResource(ctx, api.httpclient, api.endpoint+path, values, intf, api) if w, ok := intf.(warner); ok { if warning := w.Warn(); warning != nil && api.onWarning != nil { - api.onWarning(warning) + api.onWarning(warning, path, values) } } return err diff --git a/views_test.go b/views_test.go index 1dc1438ad..59312c567 100644 --- a/views_test.go +++ b/views_test.go @@ -67,6 +67,7 @@ func TestSlack_OpenView(t *testing.T) { rawResp: `{ "ok": false, "error": "dummy_error_from_slack", + "warning": "missing_charset", "response_metadata": { "warnings": [ "missing_charset" @@ -78,8 +79,9 @@ func TestSlack_OpenView(t *testing.T) { }`, expectedResp: &ViewResponse{ SlackResponse{ - Ok: false, - Error: dummySlackErr.Error(), + Ok: false, + Error: dummySlackErr.Error(), + Warning: "missing_charset", ResponseMetadata: ResponseMetadata{ Messages: []string{"[WARN] A Content-Type HTTP header was presented but did not declare a charset, such as a 'utf-8'"}, Warnings: []string{"missing_charset"}, @@ -206,6 +208,9 @@ func TestSlack_OpenView(t *testing.T) { if resp == nil || c.expectedResp == nil { return } + if resp.Warning != c.expectedResp.Warning { + t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.Warning, resp.Warning) + } if !reflect.DeepEqual(resp.ResponseMetadata.Messages, c.expectedResp.ResponseMetadata.Messages) { t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.ResponseMetadata.Messages, resp.ResponseMetadata.Messages) } @@ -240,6 +245,7 @@ func TestSlack_View_PublishView(t *testing.T) { rawResp: `{ "ok": false, "error": "dummy_error_from_slack", + "warning": "missing_charset", "response_metadata": { "warnings": [ "missing_charset" @@ -251,8 +257,9 @@ func TestSlack_View_PublishView(t *testing.T) { }`, expectedResp: &ViewResponse{ SlackResponse{ - Ok: false, - Error: dummySlackErr.Error(), + Ok: false, + Error: dummySlackErr.Error(), + Warning: "missing_charset", ResponseMetadata: ResponseMetadata{ Messages: []string{"[WARN] A Content-Type HTTP header was presented but did not declare a charset, such as a 'utf-8'"}, Warnings: []string{"missing_charset"}, @@ -365,6 +372,9 @@ func TestSlack_View_PublishView(t *testing.T) { if resp == nil || c.expectedResp == nil { return } + if resp.Warning != c.expectedResp.Warning { + t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.Warning, resp.Warning) + } if !reflect.DeepEqual(resp.ResponseMetadata.Messages, c.expectedResp.ResponseMetadata.Messages) { t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.ResponseMetadata.Messages, resp.ResponseMetadata.Messages) } @@ -399,6 +409,7 @@ func TestSlack_PushView(t *testing.T) { rawResp: `{ "ok": false, "error": "dummy_error_from_slack", + "warning": "missing_charset", "response_metadata": { "warnings": [ "missing_charset" @@ -410,8 +421,9 @@ func TestSlack_PushView(t *testing.T) { }`, expectedResp: &ViewResponse{ SlackResponse{ - Ok: false, - Error: dummySlackErr.Error(), + Ok: false, + Error: dummySlackErr.Error(), + Warning: "missing_charset", ResponseMetadata: ResponseMetadata{ Messages: []string{"[WARN] A Content-Type HTTP header was presented but did not declare a charset, such as a 'utf-8'"}, Warnings: []string{"missing_charset"}, @@ -574,6 +586,7 @@ func TestSlack_UpdateView(t *testing.T) { rawResp: `{ "ok": false, "error": "dummy_error_from_slack", + "warning": "missing_charset", "response_metadata": { "warnings": [ "missing_charset" @@ -585,8 +598,9 @@ func TestSlack_UpdateView(t *testing.T) { }`, expectedResp: &ViewResponse{ SlackResponse{ - Ok: false, - Error: dummySlackErr.Error(), + Ok: false, + Error: dummySlackErr.Error(), + Warning: "missing_charset", ResponseMetadata: ResponseMetadata{ Messages: []string{"[WARN] A Content-Type HTTP header was presented but did not declare a charset, such as a 'utf-8'"}, Warnings: []string{"missing_charset"}, @@ -713,6 +727,9 @@ func TestSlack_UpdateView(t *testing.T) { if resp == nil || c.expectedResp == nil { return } + if resp.Warning != c.expectedResp.Warning { + t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.Warning, resp.Warning) + } if !reflect.DeepEqual(resp.ResponseMetadata.Messages, c.expectedResp.ResponseMetadata.Messages) { t.Fatalf("expected:\n\t%v\n but got:\n\t%v\n", c.expectedResp.ResponseMetadata.Messages, resp.ResponseMetadata.Messages) }