From 3812d357fded7bdb4d048ffbdbdce1bc285caf2d Mon Sep 17 00:00:00 2001 From: Yassir Barchi Date: Fri, 6 Sep 2019 09:48:42 +0200 Subject: [PATCH] feat: add async feedback hook option (#414) * feat: add async feedback hook option * remove unnecessary return values * review: check feedback's resp state * fix embedmd error * fix config test * add feedback tests * fix errcheck issues --- README.md | 19 +++++++++- config/config.go | 3 ++ config/config_test.go | 2 + config/testdata/config.yml | 1 + gorush/feedback.go | 38 +++++++++++++++++++ gorush/feedback_test.go | 75 +++++++++++++++++++++++++++++++++++++ gorush/log.go | 2 + gorush/notification.go | 1 + gorush/notification_apns.go | 28 +++++++------- gorush/notification_fcm.go | 10 ++++- 10 files changed, 163 insertions(+), 16 deletions(-) create mode 100644 gorush/feedback.go create mode 100644 gorush/feedback_test.go diff --git a/README.md b/README.md index b5a0bc1..a0ac3b1 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ core: queue_num: 0 # default queue number is 8192 max_notification: 100 sync: false # set true if you need get error message from fail push notification in API response. + feedback_hook_url: "" # set a hook url if you need get error message asynchronously from fail push notification in API response. mode: "release" ssl: false cert_path: "cert.pem" @@ -516,6 +517,7 @@ Request body must has a notifications array. The following is a parameter table | name | type | description | required | note | |-------------------------|--------------|---------------------------------------------------------------------------------------------------|----------|---------------------------------------------------------------| +| notif_id | string | A unique string that identifies the notification for async feedback | - | | | tokens | string array | device tokens | o | | | platform | int | platform(iOS,Android) | o | 1=iOS, 2=Android (Firebase) | | message | string | message for notification | - | | @@ -526,7 +528,7 @@ Request body must has a notifications array. The following is a parameter table | data | string array | extensible partition | - | | | retry | int | retry send notification if fail response from server. Value must be small than `max_retry` field. | - | | | topic | string | send messages to topics | | | -| api_key | string | api key for firebase cloud message | - | only Android | +| api_key | string | api key for firebase cloud message | - | only Android | | to | string | The value must be a registration token, notification key, or topic. | - | only Android | | collapse_key | string | a key for collapsing notifications | - | only Android | | delay_while_idle | bool | a flag for device idling | - | only Android | @@ -779,7 +781,20 @@ Success response: } ``` -If you need error logs from sending fail notifications, please set `sync` as `true` on yaml config. +If you need error logs from sending fail notifications, please set a `feedback_hook_url`. The server with send the failing logs asynchronously to your API as `POST` requests. + +```diff +core: + port: "8088" # ignore this port number if auto_tls is enabled (listen 443). + worker_num: 0 # default worker number is runtime.NumCPU() + queue_num: 0 # default queue number is 8192 + max_notification: 100 + sync: false +- feedback_hook_url: "" ++ feedback_hook_url: "https://exemple.com/api/hook" +``` + +You can also switch to **sync** mode by setting the `sync` as `true` on yaml config. ```diff core: diff --git a/config/config.go b/config/config.go index 4c67eac..cb61d6b 100644 --- a/config/config.go +++ b/config/config.go @@ -19,6 +19,7 @@ core: queue_num: 0 # default queue number is 8192 max_notification: 100 sync: false # set true if you need get error message from fail push notification in API response. + feedback_hook_url: "" # set webhook url if you need get error message asynchronously from fail push notification in API response. mode: "release" ssl: false cert_path: "cert.pem" @@ -114,6 +115,7 @@ type SectionCore struct { CertBase64 string `yaml:"cert_base64"` KeyBase64 string `yaml:"key_base64"` HTTPProxy string `yaml:"http_proxy"` + FeedbackURL string `yaml:"feedback_hook_url"` PID SectionPID `yaml:"pid"` AutoTLS SectionAutoTLS `yaml:"auto_tls"` } @@ -256,6 +258,7 @@ func LoadConf(confPath string) (ConfYaml, error) { conf.Core.QueueNum = int64(viper.GetInt("core.queue_num")) conf.Core.Mode = viper.GetString("core.mode") conf.Core.Sync = viper.GetBool("core.sync") + conf.Core.FeedbackURL = viper.GetString("core.feedback_hook_url") conf.Core.SSL = viper.GetBool("core.ssl") conf.Core.CertPath = viper.GetString("core.cert_path") conf.Core.KeyPath = viper.GetString("core.key_path") diff --git a/config/config_test.go b/config/config_test.go index 56042fc..79bbede 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -44,6 +44,7 @@ func (suite *ConfigTestSuite) TestValidateConfDefault() { assert.Equal(suite.T(), int64(8192), suite.ConfGorushDefault.Core.QueueNum) assert.Equal(suite.T(), "release", suite.ConfGorushDefault.Core.Mode) assert.Equal(suite.T(), false, suite.ConfGorushDefault.Core.Sync) + assert.Equal(suite.T(), "", suite.ConfGorushDefault.Core.FeedbackURL) assert.Equal(suite.T(), false, suite.ConfGorushDefault.Core.SSL) assert.Equal(suite.T(), "cert.pem", suite.ConfGorushDefault.Core.CertPath) assert.Equal(suite.T(), "key.pem", suite.ConfGorushDefault.Core.KeyPath) @@ -116,6 +117,7 @@ func (suite *ConfigTestSuite) TestValidateConf() { assert.Equal(suite.T(), int64(8192), suite.ConfGorush.Core.QueueNum) assert.Equal(suite.T(), "release", suite.ConfGorush.Core.Mode) assert.Equal(suite.T(), false, suite.ConfGorush.Core.Sync) + assert.Equal(suite.T(), "", suite.ConfGorush.Core.FeedbackURL) assert.Equal(suite.T(), false, suite.ConfGorush.Core.SSL) assert.Equal(suite.T(), "cert.pem", suite.ConfGorush.Core.CertPath) assert.Equal(suite.T(), "key.pem", suite.ConfGorush.Core.KeyPath) diff --git a/config/testdata/config.yml b/config/testdata/config.yml index c8da2db..921b56e 100644 --- a/config/testdata/config.yml +++ b/config/testdata/config.yml @@ -6,6 +6,7 @@ core: queue_num: 0 # default queue number is 8192 max_notification: 100 sync: false # set true if you need get error message from fail push notification in API response. + feedback_hook_url: "" # set a hook url if you need get error message asynchronously from fail push notification in API response. mode: "release" ssl: false cert_path: "cert.pem" diff --git a/gorush/feedback.go b/gorush/feedback.go new file mode 100644 index 0000000..b1c9d15 --- /dev/null +++ b/gorush/feedback.go @@ -0,0 +1,38 @@ +package gorush + +import ( + "bytes" + "encoding/json" + "errors" + "net/http" +) + +// DispatchFeedback sends a feedback to the configured gateway. +func DispatchFeedback(log LogPushEntry, url string) error { + + if url == "" { + return errors.New("The url can't be empty") + } + + payload, err := json.Marshal(log) + + if err != nil { + return err + } + + req, _ := http.NewRequest("POST", url, bytes.NewBuffer(payload)) + req.Header.Set("Content-Type", "application/json; charset=utf-8") + + HTTPClient := &http.Client{} + resp, err := HTTPClient.Do(req) + + if resp != nil { + defer resp.Body.Close() + } + + if err != nil { + return err + } + + return nil +} diff --git a/gorush/feedback_test.go b/gorush/feedback_test.go new file mode 100644 index 0000000..2484589 --- /dev/null +++ b/gorush/feedback_test.go @@ -0,0 +1,75 @@ +package gorush + +import ( + "log" + "net/http" + "net/http/httptest" + "testing" + + "github.com/appleboy/gorush/config" + "github.com/stretchr/testify/assert" +) + +func TestEmptyFeedbackURL(t *testing.T) { + // PushConf, _ = config.LoadConf("") + logEntry := LogPushEntry{ + ID: "", + Type: "", + Platform: "", + Token: "", + Message: "", + Error: "", + } + + err := DispatchFeedback(logEntry, PushConf.Core.FeedbackURL) + assert.NotNil(t, err) +} + +func TestHTTPErrorInFeedbackCall(t *testing.T) { + config, _ := config.LoadConf("") + config.Core.FeedbackURL = "http://test.example.com/api/" + logEntry := LogPushEntry{ + ID: "", + Type: "", + Platform: "", + Token: "", + Message: "", + Error: "", + } + + err := DispatchFeedback(logEntry, config.Core.FeedbackURL) + assert.NotNil(t, err) +} + +func TestSuccessfulFeedbackCall(t *testing.T) { + + // Mock http server + httpMock := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/dispatch" { + w.Header().Add("Content-Type", "application/json") + _, err := w.Write([]byte(`{}`)) + + if err != nil { + log.Println(err) + panic(err) + } + } + }), + ) + defer httpMock.Close() + + config, _ := config.LoadConf("") + config.Core.FeedbackURL = httpMock.URL + logEntry := LogPushEntry{ + ID: "", + Type: "", + Platform: "", + Token: "", + Message: "", + Error: "", + } + + err := DispatchFeedback(logEntry, config.Core.FeedbackURL) + assert.Nil(t, err) +} diff --git a/gorush/log.go b/gorush/log.go index 34b02b9..c0ed42c 100644 --- a/gorush/log.go +++ b/gorush/log.go @@ -34,6 +34,7 @@ type LogReq struct { // LogPushEntry is push response log type LogPushEntry struct { + ID string `json:"notif_id,omitempty"` Type string `json:"type"` Platform string `json:"platform"` Token string `json:"token"` @@ -211,6 +212,7 @@ func getLogPushEntry(status, token string, req PushNotification, errPush error) } return LogPushEntry{ + ID: req.ID, Type: status, Platform: plat, Token: token, diff --git a/gorush/notification.go b/gorush/notification.go index b0762bf..1734661 100644 --- a/gorush/notification.go +++ b/gorush/notification.go @@ -52,6 +52,7 @@ type RequestPush struct { // PushNotification is single notification request type PushNotification struct { // Common + ID string `json:"notif_id,omitempty"` Tokens []string `json:"tokens" binding:"required"` Platform int `json:"platform" binding:"required"` Message string `json:"message,omitempty"` diff --git a/gorush/notification_apns.go b/gorush/notification_apns.go index 5d1b8cd..76a8170 100644 --- a/gorush/notification_apns.go +++ b/gorush/notification_apns.go @@ -13,6 +13,7 @@ import ( "github.com/sideshow/apns2/certificate" "github.com/sideshow/apns2/payload" "github.com/sideshow/apns2/token" + "github.com/sirupsen/logrus" ) // Sound sets the aps sound on the payload. @@ -288,25 +289,26 @@ Retry: // send ios notification res, err := client.Push(notification) - if err != nil { + if err != nil || res.StatusCode != 200 { + if err == nil { + // error message: + // ref: https://github.com/sideshow/apns2/blob/master/response.go#L14-L65 + err = errors.New(res.Reason) + } // apns server error LogPush(FailedPush, token, req, err) + if PushConf.Core.Sync { req.AddLog(getLogPushEntry(FailedPush, token, req, err)) + } else if PushConf.Core.FeedbackURL != "" { + go func(logger *logrus.Logger, log LogPushEntry, url string) { + err := DispatchFeedback(log, url) + if err != nil { + logger.Error(err) + } + }(LogError, getLogPushEntry(FailedPush, token, req, err), PushConf.Core.FeedbackURL) } - StatStorage.AddIosError(1) - newTokens = append(newTokens, token) - isError = true - continue - } - if res.StatusCode != 200 { - // error message: - // ref: https://github.com/sideshow/apns2/blob/master/response.go#L14-L65 - LogPush(FailedPush, token, req, errors.New(res.Reason)) - if PushConf.Core.Sync { - req.AddLog(getLogPushEntry(FailedPush, token, req, errors.New(res.Reason))) - } StatStorage.AddIosError(1) newTokens = append(newTokens, token) isError = true diff --git a/gorush/notification_fcm.go b/gorush/notification_fcm.go index e4d9f18..09b72b0 100644 --- a/gorush/notification_fcm.go +++ b/gorush/notification_fcm.go @@ -4,7 +4,8 @@ import ( "errors" "fmt" - "github.com/appleboy/go-fcm" + fcm "github.com/appleboy/go-fcm" + "github.com/sirupsen/logrus" ) // InitFCMClient use for initialize FCM Client. @@ -149,6 +150,13 @@ Retry: LogPush(FailedPush, to, req, result.Error) if PushConf.Core.Sync { req.AddLog(getLogPushEntry(FailedPush, to, req, result.Error)) + } else if PushConf.Core.FeedbackURL != "" { + go func(logger *logrus.Logger, log LogPushEntry, url string) { + err := DispatchFeedback(log, url) + if err != nil { + logger.Error(err) + } + }(LogError, getLogPushEntry(FailedPush, to, req, result.Error), PushConf.Core.FeedbackURL) } continue }