From 52622558bd94e28ba9159c21faf3b33804895a73 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 24 Feb 2020 22:18:50 +0800 Subject: [PATCH] chore(gorush): add timeout for http client (#473) * chore(gorush): add timeout for http client dispatch feedback url See: https://github.com/appleboy/gorush/issues/449 * docs: update readme --- README.md | 1 + config/config.go | 3 +++ config/config_test.go | 2 ++ config/testdata/config.yml | 1 + gorush/feedback.go | 18 +++++++++++++++--- gorush/feedback_test.go | 7 ++++--- gorush/notification_apns.go | 6 +++--- gorush/notification_fcm.go | 6 +++--- 8 files changed, 32 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index e0df2e5..9d9d7d1 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ core: 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. + feedback_timeout: 10 # default is 10 second mode: "release" ssl: false cert_path: "cert.pem" diff --git a/config/config.go b/config/config.go index fbf0b24..be86bfd 100644 --- a/config/config.go +++ b/config/config.go @@ -21,6 +21,7 @@ core: 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. + feedback_timeout: 10 # default is 10 second mode: "release" ssl: false cert_path: "cert.pem" @@ -118,6 +119,7 @@ type SectionCore struct { KeyBase64 string `yaml:"key_base64"` HTTPProxy string `yaml:"http_proxy"` FeedbackURL string `yaml:"feedback_hook_url"` + FeedbackTimeout int64 `yaml:"feedback_timeout"` PID SectionPID `yaml:"pid"` AutoTLS SectionAutoTLS `yaml:"auto_tls"` } @@ -262,6 +264,7 @@ func LoadConf(confPath string) (ConfYaml, error) { 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.FeedbackTimeout = int64(viper.GetInt("core.feedback_timeout")) 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 71ece0e..23d2f55 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -46,6 +46,7 @@ func (suite *ConfigTestSuite) TestValidateConfDefault() { 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(), int64(10), suite.ConfGorushDefault.Core.FeedbackTimeout) 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) @@ -120,6 +121,7 @@ func (suite *ConfigTestSuite) TestValidateConf() { 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(), int64(10), suite.ConfGorush.Core.FeedbackTimeout) 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 56c8b2d..f0ba990 100644 --- a/config/testdata/config.yml +++ b/config/testdata/config.yml @@ -8,6 +8,7 @@ core: 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. + feedback_timeout: 10 # default is 10 second mode: "release" ssl: false cert_path: "cert.pem" diff --git a/gorush/feedback.go b/gorush/feedback.go index b1c9d15..ddd1325 100644 --- a/gorush/feedback.go +++ b/gorush/feedback.go @@ -4,11 +4,13 @@ import ( "bytes" "encoding/json" "errors" + "net" "net/http" + "time" ) // DispatchFeedback sends a feedback to the configured gateway. -func DispatchFeedback(log LogPushEntry, url string) error { +func DispatchFeedback(log LogPushEntry, url string, timeout int64) error { if url == "" { return errors.New("The url can't be empty") @@ -23,8 +25,18 @@ func DispatchFeedback(log LogPushEntry, url string) error { 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) + var transport = &http.Transport{ + Dial: (&net.Dialer{ + Timeout: 5 * time.Second, + }).Dial, + TLSHandshakeTimeout: 5 * time.Second, + } + var client = &http.Client{ + Timeout: time.Duration(timeout) * time.Second, + Transport: transport, + } + + resp, err := client.Do(req) if resp != nil { defer resp.Body.Close() diff --git a/gorush/feedback_test.go b/gorush/feedback_test.go index 2484589..1a67708 100644 --- a/gorush/feedback_test.go +++ b/gorush/feedback_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/appleboy/gorush/config" + "github.com/stretchr/testify/assert" ) @@ -21,7 +22,7 @@ func TestEmptyFeedbackURL(t *testing.T) { Error: "", } - err := DispatchFeedback(logEntry, PushConf.Core.FeedbackURL) + err := DispatchFeedback(logEntry, PushConf.Core.FeedbackURL, PushConf.Core.FeedbackTimeout) assert.NotNil(t, err) } @@ -37,7 +38,7 @@ func TestHTTPErrorInFeedbackCall(t *testing.T) { Error: "", } - err := DispatchFeedback(logEntry, config.Core.FeedbackURL) + err := DispatchFeedback(logEntry, config.Core.FeedbackURL, config.Core.FeedbackTimeout) assert.NotNil(t, err) } @@ -70,6 +71,6 @@ func TestSuccessfulFeedbackCall(t *testing.T) { Error: "", } - err := DispatchFeedback(logEntry, config.Core.FeedbackURL) + err := DispatchFeedback(logEntry, config.Core.FeedbackURL, config.Core.FeedbackTimeout) assert.Nil(t, err) } diff --git a/gorush/notification_apns.go b/gorush/notification_apns.go index 6c6f6b5..ed1eb18 100644 --- a/gorush/notification_apns.go +++ b/gorush/notification_apns.go @@ -374,12 +374,12 @@ Retry: 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) + go func(logger *logrus.Logger, log LogPushEntry, url string, timeout int64) { + err := DispatchFeedback(log, url, timeout) if err != nil { logger.Error(err) } - }(LogError, getLogPushEntry(FailedPush, token, req, err), PushConf.Core.FeedbackURL) + }(LogError, getLogPushEntry(FailedPush, token, req, err), PushConf.Core.FeedbackURL, PushConf.Core.FeedbackTimeout) } StatStorage.AddIosError(1) diff --git a/gorush/notification_fcm.go b/gorush/notification_fcm.go index 6ff6284..927518d 100644 --- a/gorush/notification_fcm.go +++ b/gorush/notification_fcm.go @@ -160,12 +160,12 @@ Retry: 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) + go func(logger *logrus.Logger, log LogPushEntry, url string, timeout int64) { + err := DispatchFeedback(log, url, timeout) if err != nil { logger.Error(err) } - }(LogError, getLogPushEntry(FailedPush, to, req, result.Error), PushConf.Core.FeedbackURL) + }(LogError, getLogPushEntry(FailedPush, to, req, result.Error), PushConf.Core.FeedbackURL, PushConf.Core.FeedbackTimeout) } continue }