From df027a8f51d4b866ddab74e98681df6aa50151c1 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Thu, 4 May 2017 13:43:54 -0700
Subject: [PATCH] Webhook endpoints can fail, we must start the server. (#4255)

This PR fixes a regression introduced in #4060
---
 cmd/notify-webhook.go      |  8 ++++++-
 cmd/notify-webhook_test.go | 43 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/cmd/notify-webhook.go b/cmd/notify-webhook.go
index 3bc036d0d..10cb80909 100644
--- a/cmd/notify-webhook.go
+++ b/cmd/notify-webhook.go
@@ -69,10 +69,15 @@ func isNetErrorIgnored(err error) bool {
 	}
 	switch err.(type) {
 	case net.Error:
-		switch err.(type) {
+		switch e := err.(type) {
 		case *net.DNSError, *net.OpError, net.UnknownNetworkError:
 			return true
 		case *url.Error:
+			// Fixes https://github.com/minio/minio/issues/4050
+			switch e.Err.(type) {
+			case *net.DNSError, *net.OpError, net.UnknownNetworkError:
+				return true
+			}
 			// For a URL error, where it replies back "connection closed"
 			// retry again.
 			if strings.Contains(err.Error(), "Connection closed by foreign host") {
@@ -127,6 +132,7 @@ func lookupEndpoint(urlStr string) error {
 		resp, derr := client.Do(req)
 		if derr != nil {
 			if isNetErrorIgnored(derr) {
+				errorIf(derr, "Unable to lookup webhook endpoint %s", urlStr)
 				return nil
 			}
 			return derr
diff --git a/cmd/notify-webhook_test.go b/cmd/notify-webhook_test.go
index 7fd4382d1..4462b330f 100644
--- a/cmd/notify-webhook_test.go
+++ b/cmd/notify-webhook_test.go
@@ -17,6 +17,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -38,6 +39,12 @@ func (p postHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	io.Copy(w, r.Body)
 }
 
+type errorHandler struct{}
+
+func (e errorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	http.Error(w, fmt.Sprintf("Unexpected method %s", r.Method), http.StatusBadRequest)
+}
+
 // Tests web hook initialization.
 func TestNewWebHookNotify(t *testing.T) {
 	root, err := newTestConfig(globalMinioDefaultRegion)
@@ -53,8 +60,8 @@ func TestNewWebHookNotify(t *testing.T) {
 
 	serverConfig.Notify.SetWebhookByID("10", webhookNotify{Enable: true, Endpoint: "http://127.0.0.1:xxx"})
 	_, err = newWebhookNotify("10")
-	if err == nil {
-		t.Fatal("Unexpected should fail with lookupHost")
+	if err != nil {
+		t.Fatal("Unexpected should not fail with lookupEndpoint", err)
 	}
 
 	serverConfig.Notify.SetWebhookByID("15", webhookNotify{Enable: true, Endpoint: "http://%"})
@@ -77,3 +84,35 @@ func TestNewWebHookNotify(t *testing.T) {
 		"EventType": "s3:ObjectCreated:Put",
 	}).Info()
 }
+
+// Add tests for lookup endpoint.
+func TestLookupEndpoint(t *testing.T) {
+	server := httptest.NewServer(errorHandler{})
+	defer server.Close()
+
+	testCases := []struct {
+		endpoint string
+		err      error
+	}{
+		// Ignore endpoints which don't exist.
+		{
+			endpoint: "http://unknown",
+			err:      nil,
+		},
+		{
+			endpoint: "%%%",
+			err:      errors.New("parse %%%: invalid URL escape \"%%%\""),
+		},
+		{
+			endpoint: server.URL,
+			err:      fmt.Errorf("Unexpected response from webhook server %s: (400 Bad Request)", server.URL),
+		},
+	}
+	for _, test := range testCases {
+		if err := lookupEndpoint(test.endpoint); err != nil {
+			if err.Error() != test.err.Error() {
+				t.Errorf("Expected %s, got %s", test.err, err)
+			}
+		}
+	}
+}