fix secure and insecure clients being the same, add tests

nyaruka · Aug 23, 2017 · 3183f92 · 3183f92
1 parent 0a668db
commit 3183f92
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 26 deletions.
diff --git a/handlers/africastalking/africastalking.go b/handlers/africastalking/africastalking.go
@@ -152,7 +152,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, err
 	}

diff --git a/handlers/blackmyna/blackmyna.go b/handlers/blackmyna/blackmyna.go
@@ -131,7 +131,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, err
 	}

diff --git a/handlers/external/external.go b/handlers/external/external.go
@@ -207,7 +207,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, err
 	}

diff --git a/handlers/kannel/kannel.go b/handlers/kannel/kannel.go
@@ -202,7 +202,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, errors.Errorf("received error sending message")
 	}

diff --git a/handlers/shaqodoon/shaqodoon.go b/handlers/shaqodoon/shaqodoon.go
@@ -182,7 +182,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 	rr, err := utils.MakeInsecureHTTPRequest(req)
 
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, err
 	}

diff --git a/handlers/smscentral/smscentral.go b/handlers/smscentral/smscentral.go
@@ -106,7 +106,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 	if err != nil {
 		return status, err
 	}

diff --git a/handlers/telegram/telegram.go b/handlers/telegram/telegram.go
@@ -125,7 +125,7 @@ func (h *handler) sendMsgPart(msg courier.Msg, token string, path string, form u
 	rr, err := utils.MakeHTTPRequest(req)
 
 	// build our channel log
-	log := courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr)
+	log := courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err)
 
 	// was this request successful?
 	ok, err := jsonparser.GetBoolean([]byte(rr.Body), "ok")

diff --git a/handlers/twilio/twilio.go b/handlers/twilio/twilio.go
@@ -202,7 +202,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 
 	// record our status and log
 	status := h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+	status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 
 	// fail if we received an error
 	if err != nil {

diff --git a/handlers/yo/yo.go b/handlers/yo/yo.go
@@ -146,7 +146,7 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 		}
 		// record our status and log
 		status = h.Backend().NewMsgStatusForID(msg.Channel(), msg.ID(), courier.MsgErrored)
-		status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr))
+		status.AddLog(courier.NewChannelLogFromRR(msg.Channel(), msg.ID(), rr, err))
 		if err != nil {
 			return status, err
 		}

diff --git a/log.go b/log.go
@@ -34,19 +34,23 @@ func NewChannelLog(channel Channel, msgID MsgID, method string, url string, stat
 }
 
 // NewChannelLogFromRR creates a new channel log for the passed in channel, id, and request/response log
-func NewChannelLogFromRR(channel Channel, msgID MsgID, rr *utils.RequestResponse) *ChannelLog {
-	return &ChannelLog{
+func NewChannelLogFromRR(channel Channel, msgID MsgID, rr *utils.RequestResponse, err error) *ChannelLog {
+	log := &ChannelLog{
 		Channel:    channel,
 		MsgID:      msgID,
 		Method:     rr.Method,
 		URL:        rr.URL,
 		StatusCode: rr.StatusCode,
-		Error:      "",
 		Request:    rr.Request,
 		Response:   rr.Response,
 		CreatedOn:  time.Now(),
 		Elapsed:    rr.Elapsed,
 	}
+	if err != nil {
+		log.Error = err.Error()
+	}
+
+	return log
 }
 
 func (l *ChannelLog) String() string {

diff --git a/utils/http.go b/utils/http.go
@@ -150,39 +150,47 @@ func newRRFromResponse(method string, requestTrace string, r *http.Response) (*R
 	return &rr, err
 }
 
-var (
-	transport *http.Transport
-	client    *http.Client
-	once      sync.Once
-)
-
 // GetHTTPClient returns the shared HTTP client used by all Courier threads
 func GetHTTPClient() *http.Client {
 	once.Do(func() {
-		timeout := time.Duration(30 * time.Second)
 		transport = &http.Transport{
 			MaxIdleConns:    10,
 			IdleConnTimeout: 30 * time.Second,
 		}
-		client = &http.Client{Transport: transport, Timeout: timeout}
+		client = &http.Client{
+			Transport: transport,
+			Timeout:   30 * time.Second,
+		}
 	})
 
 	return client
 }
 
 // GetInsecureHTTPClient returns the shared HTTP client used by all Courier threads
 func GetInsecureHTTPClient() *http.Client {
-	once.Do(func() {
-		timeout := time.Duration(30 * time.Second)
-		transport = &http.Transport{
+	insecureOnce.Do(func() {
+		insecureTransport = &http.Transport{
 			MaxIdleConns:    10,
 			IdleConnTimeout: 30 * time.Second,
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		}
-		client = &http.Client{Transport: transport, Timeout: timeout}
+		insecureClient = &http.Client{
+			Transport: insecureTransport,
+			Timeout:   30 * time.Second,
+		}
 	})
 
-	return client
+	return insecureClient
 }
 
-var HTTPUserAgent = "Courier/vDev"
+var (
+	transport *http.Transport
+	client    *http.Client
+	once      sync.Once
+
+	insecureTransport *http.Transport
+	insecureClient    *http.Client
+	insecureOnce      sync.Once
+
+	HTTPUserAgent = "Courier/vDev"
+)