From 3183f92db43b66cf799463e2a9bbebc166f0194a Mon Sep 17 00:00:00 2001 From: Nic Pottier Date: Wed, 23 Aug 2017 13:39:19 -0500 Subject: [PATCH] fix secure and insecure clients being the same, add tests --- handlers/africastalking/africastalking.go | 2 +- handlers/blackmyna/blackmyna.go | 2 +- handlers/external/external.go | 2 +- handlers/kannel/kannel.go | 2 +- handlers/shaqodoon/shaqodoon.go | 2 +- handlers/smscentral/smscentral.go | 2 +- handlers/telegram/telegram.go | 2 +- handlers/twilio/twilio.go | 2 +- handlers/yo/yo.go | 2 +- log.go | 10 +++++-- utils/http.go | 36 ++++++++++++++--------- 11 files changed, 38 insertions(+), 26 deletions(-) diff --git a/handlers/africastalking/africastalking.go b/handlers/africastalking/africastalking.go index ff73242d2..e60238904 100644 --- 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 index a5e2ae9db..8c1cad218 100644 --- 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 index 80ebba417..00ed0f7f2 100644 --- 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 index 23a21dc29..eb389d1a0 100644 --- 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 index 3a3e4336f..c60a51215 100644 --- 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 index c4214763f..79d1880c6 100644 --- 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 index 108ab51fb..552e4a0ce 100644 --- 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 index 64d6c29ba..ed7f0bea2 100644 --- 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 index 4388d5adb..173c3114b 100644 --- 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 index 367a22f2d..b1e20fd05 100644 --- 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 index cb6ee6508..66847fd89 100644 --- a/utils/http.go +++ b/utils/http.go @@ -150,21 +150,17 @@ 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 @@ -172,17 +168,29 @@ func GetHTTPClient() *http.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" +)