Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add client callback for API warning messages #913

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions misc.go
Expand Up @@ -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"`
}

Expand All @@ -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
Expand Down
24 changes: 22 additions & 2 deletions slack.go
Expand Up @@ -63,6 +63,7 @@ type Client struct {
debug bool
log ilogger
httpclient httpClient
onWarning func(w *Warning, path string, values url.Values)
}

// Option defines an option for a Client
Expand All @@ -89,6 +90,13 @@ func OptionLog(l logger) func(*Client) {
}
}

// OptionOnWarning set callback for response warnings.
func OptionOnWarning(fn func(w *Warning, path string, values url.Values)) 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 }
Expand Down Expand Up @@ -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, path, values)
}
}
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, path, values)
}
}
return err
}
33 changes: 25 additions & 8 deletions views_test.go
Expand Up @@ -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"
Expand All @@ -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"},
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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"
Expand All @@ -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"},
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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"
Expand All @@ -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"},
Expand Down Expand Up @@ -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"
Expand All @@ -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"},
Expand Down Expand Up @@ -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)
}
Expand Down