From 8700945cdf8030dee02a2e3359e88c341c46f902 Mon Sep 17 00:00:00 2001 From: Praveen raj Mani Date: Fri, 13 Sep 2019 05:14:51 +0530 Subject: [PATCH] Handle connection failures on webhook/url pings (#8204) Properly handle connection failures while replaying events Fixes #8194 --- cmd/gateway-common.go | 3 ++- cmd/storage-rest-client.go | 2 +- cmd/update.go | 5 ++-- cmd/utils.go | 38 ----------------------------- pkg/event/target/elasticsearch.go | 14 ++++------- pkg/event/target/webhook.go | 15 ++++-------- pkg/net/url.go | 40 +++++++++++++++++++++++++++++++ 7 files changed, 54 insertions(+), 63 deletions(-) diff --git a/cmd/gateway-common.go b/cmd/gateway-common.go index 9072e5df2..eace4360e 100644 --- a/cmd/gateway-common.go +++ b/cmd/gateway-common.go @@ -24,6 +24,7 @@ import ( xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/hash" + xnet "github.com/minio/minio/pkg/net" minio "github.com/minio/minio-go/v6" ) @@ -299,7 +300,7 @@ func ErrorRespToObjectError(err error, params ...string) error { object = params[1] } - if isNetworkOrHostDown(err) { + if xnet.IsNetworkOrHostDown(err) { return BackendDown{} } diff --git a/cmd/storage-rest-client.go b/cmd/storage-rest-client.go index 43da7c4f0..99a4b5d1a 100644 --- a/cmd/storage-rest-client.go +++ b/cmd/storage-rest-client.go @@ -45,7 +45,7 @@ func isNetworkError(err error) bool { return true } if nerr, ok := err.(*rest.NetworkError); ok { - return isNetworkOrHostDown(nerr.Err) + return xnet.IsNetworkOrHostDown(nerr.Err) } return false } diff --git a/cmd/update.go b/cmd/update.go index 78f84eee8..ce01b2544 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -35,6 +35,7 @@ import ( "github.com/inconshreveable/go-update" xhttp "github.com/minio/minio/cmd/http" "github.com/minio/minio/cmd/logger" + xnet "github.com/minio/minio/pkg/net" _ "github.com/minio/sha256-simd" // Needed for sha256 hash verifier. ) @@ -283,7 +284,7 @@ func downloadReleaseURL(releaseChecksumURL string, timeout time.Duration, mode s client := &http.Client{Transport: getUpdateTransport(timeout)} resp, err := client.Do(req) if err != nil { - if isNetworkOrHostDown(err) { + if xnet.IsNetworkOrHostDown(err) { return content, AdminError{ Code: AdminUpdateURLNotReachable, Message: err.Error(), @@ -500,7 +501,7 @@ func doUpdate(updateURL, sha256Hex, mode string) (err error) { resp, err := clnt.Do(req) if err != nil { - if isNetworkOrHostDown(err) { + if xnet.IsNetworkOrHostDown(err) { return AdminError{ Code: AdminUpdateURLNotReachable, Message: err.Error(), diff --git a/cmd/utils.go b/cmd/utils.go index 052d743dc..d57870a1f 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -29,7 +29,6 @@ import ( "io/ioutil" "net" "net/http" - "net/url" "os" "path/filepath" "reflect" @@ -426,43 +425,6 @@ func newContext(r *http.Request, w http.ResponseWriter, api string) context.Cont return logger.SetReqInfo(r.Context(), reqInfo) } -// isNetworkOrHostDown - if there was a network error or if the host is down. -func isNetworkOrHostDown(err error) bool { - if err == nil { - return false - } - // We need to figure if the error either a timeout - // or a non-temporary error. - e, ok := err.(net.Error) - if ok { - urlErr, ok := e.(*url.Error) - if ok { - switch urlErr.Err.(type) { - case *net.DNSError, *net.OpError, net.UnknownNetworkError: - return true - } - } - if e.Timeout() { - return true - } - } - ok = false - // Fallback to other mechanisms. - if strings.Contains(err.Error(), "Connection closed by foreign host") { - ok = true - } else if strings.Contains(err.Error(), "TLS handshake timeout") { - // If error is - tlsHandshakeTimeoutError. - ok = true - } else if strings.Contains(err.Error(), "i/o timeout") { - // If error is - tcp timeoutError. - ok = true - } else if strings.Contains(err.Error(), "connection timed out") { - // If err is a net.Dial timeout. - ok = true - } - return ok -} - // Used for registering with rest handlers (have a look at registerStorageRESTHandlers for usage example) // If it is passed ["aaaa", "bbbb"], it returns ["aaaa", "{aaaa:.*}", "bbbb", "{bbbb:.*}"] func restQueries(keys ...string) []string { diff --git a/pkg/event/target/elasticsearch.go b/pkg/event/target/elasticsearch.go index c9bb3ca37..6723589b3 100644 --- a/pkg/event/target/elasticsearch.go +++ b/pkg/event/target/elasticsearch.go @@ -83,11 +83,8 @@ func (target *ElasticsearchTarget) Save(eventData event.Event) error { return target.store.Put(eventData) } if dErr := target.args.URL.DialHTTP(); dErr != nil { - if urlErr, ok := dErr.(*url.Error); ok { - // To treat "connection refused" errors as errNotConnected. - if IsConnRefusedErr(urlErr.Err) { - return errNotConnected - } + if xnet.IsNetworkOrHostDown(dErr) { + return errNotConnected } return dErr } @@ -157,11 +154,8 @@ func (target *ElasticsearchTarget) Send(eventKey string) error { } if dErr := target.args.URL.DialHTTP(); dErr != nil { - if urlErr, ok := dErr.(*url.Error); ok { - // To treat "connection refused" errors as errNotConnected. - if IsConnRefusedErr(urlErr.Err) { - return errNotConnected - } + if xnet.IsNetworkOrHostDown(dErr) { + return errNotConnected } return dErr } diff --git a/pkg/event/target/webhook.go b/pkg/event/target/webhook.go index 6172422c1..118fd4a7a 100644 --- a/pkg/event/target/webhook.go +++ b/pkg/event/target/webhook.go @@ -87,11 +87,8 @@ func (target *WebhookTarget) Save(eventData event.Event) error { return pErr } if dErr := u.DialHTTP(); dErr != nil { - if urlErr, ok := dErr.(*url.Error); ok { - // To treat "connection refused" errors as errNotConnected. - if IsConnRefusedErr(urlErr.Err) { - return errNotConnected - } + if xnet.IsNetworkOrHostDown(dErr) { + return errNotConnected } return dErr } @@ -136,17 +133,13 @@ func (target *WebhookTarget) send(eventData event.Event) error { // Send - reads an event from store and sends it to webhook. func (target *WebhookTarget) Send(eventKey string) error { - u, pErr := xnet.ParseURL(target.args.Endpoint.String()) if pErr != nil { return pErr } if dErr := u.DialHTTP(); dErr != nil { - if urlErr, ok := dErr.(*url.Error); ok { - // To treat "connection refused" errors as errNotConnected. - if IsConnRefusedErr(urlErr.Err) { - return errNotConnected - } + if xnet.IsNetworkOrHostDown(dErr) { + return errNotConnected } return dErr } diff --git a/pkg/net/url.go b/pkg/net/url.go index 955e5bacc..bf0b6a7de 100644 --- a/pkg/net/url.go +++ b/pkg/net/url.go @@ -134,3 +134,43 @@ func ParseURL(s string) (u *URL, err error) { u = &v return u, nil } + +// IsNetworkOrHostDown - if there was a network error or if the host is down. +func IsNetworkOrHostDown(err error) bool { + if err == nil { + return false + } + // We need to figure if the error either a timeout + // or a non-temporary error. + e, ok := err.(net.Error) + if ok { + urlErr, ok := e.(*url.Error) + if ok { + switch urlErr.Err.(type) { + case *net.DNSError, *net.OpError, net.UnknownNetworkError: + return true + } + } + if e.Timeout() { + return true + } + } + ok = false + // Fallback to other mechanisms. + if strings.Contains(err.Error(), "Connection closed by foreign host") { + ok = true + } else if strings.Contains(err.Error(), "TLS handshake timeout") { + // If error is - tlsHandshakeTimeoutError. + ok = true + } else if strings.Contains(err.Error(), "i/o timeout") { + // If error is - tcp timeoutError. + ok = true + } else if strings.Contains(err.Error(), "connection timed out") { + // If err is a net.Dial timeout. + ok = true + } else if strings.Contains(strings.ToLower(err.Error()), "503 service unavailable") { + // Denial errors + ok = true + } + return ok +}