more consistent use of errors for unexpected cases

handlers/kannel/kannel.go

@@ -13,7 +13,6 @@ import (
 	"github.com/nyaruka/courier/handlers"
 	"github.com/nyaruka/courier/utils"
 	"github.com/nyaruka/phonenumbers"
-	"github.com/pkg/errors"
 )
 
 const configUseNational = "use_national"
@@ -192,6 +191,10 @@ func (h *handler) SendMsg(msg courier.Msg) (courier.MsgStatus, error) {
 	verifySSL, _ := verifySSLStr.(bool)
 
 	req, err := http.NewRequest(http.MethodGet, sendURL, nil)
+	if err != nil {
+		return nil, err
+	}
+
 	var rr *utils.RequestResponse
 
 	if verifySSL {
@@ -203,11 +206,10 @@ 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, err))
-	if err != nil {
-		return status, errors.Errorf("received error sending message")
+	if err == nil {
+		status.SetStatus(courier.MsgWired)
 	}
 
-	status.SetStatus(courier.MsgWired)
 	return status, nil
 }
 

handlers/kannel/kannel_test.go

@@ -72,7 +72,6 @@ var defaultSendTestCases = []ChannelSendTestCase{
 		Text: "Error Message", URN: "tel:+250788383383", Priority: courier.BulkPriority,
 		Status:       "E",
 		ResponseBody: "1: Unknown channel", ResponseStatus: 401,
-		Error:     "received error sending message",
 		URLParams: map[string]string{"text": `Error Message`, "to": "+250788383383", "coding": "", "priority": ""},
 		SendPrep:  setSendURL},
 	{Label: "Custom Params",

handlers/shaqodoon/shaqodoon.go

@@ -51,13 +51,13 @@ func (h *handler) ReceiveMessage(channel courier.Channel, w http.ResponseWriter,
 	// validate whether our required fields are present
 	err := handlers.Validate(shaqodoonMessage)
 	if err != nil {
-		return nil, err
+		return nil, courier.WriteError(w, r, err)
 	}
 
 	// must have one of from or sender set, error if neither
 	sender := shaqodoonMessage.From
 	if sender == "" {
-		return nil, errors.New("must have one of 'sender' or 'from' set")
+		return nil, courier.WriteError(w, r, errors.New("must have one of 'sender' or 'from' set"))
 	}
 
 	// if we have a date, parse it
@@ -70,14 +70,14 @@ func (h *handler) ReceiveMessage(channel courier.Channel, w http.ResponseWriter,
 	if dateString != "" {
 		date, err = time.Parse(time.RFC3339Nano, dateString)
 		if err != nil {
-			return nil, errors.New("invalid date format, must be RFC 3339")
+			return nil, courier.WriteError(w, r, errors.New("invalid date format, must be RFC 3339"))
 		}
 	}
 
 	// create our URN
 	urn := courier.NewTelURNForChannel(sender, channel)
 	if err != nil {
-		return nil, err
+		return nil, courier.WriteError(w, r, err)
 	}
 
 	// build our msg
@@ -119,13 +119,13 @@ func (h *handler) StatusMessage(statusString string, channel courier.Channel, w
 	// validate whether our required fields are present
 	err := handlers.Validate(statusForm)
 	if err != nil {
-		return nil, err
+		return nil, courier.WriteError(w, r, err)
 	}
 
 	// get our id
 	msgStatus, found := statusMappings[strings.ToLower(statusString)]
 	if !found {
-		return nil, fmt.Errorf("unknown status '%s', must be one failed, sent or delivered", statusString)
+		return nil, courier.WriteError(w, r, fmt.Errorf("unknown status '%s', must be one failed, sent or delivered", statusString))
 	}
 
 	// write our status

handlers/telegram/telegram.go

@@ -39,7 +39,7 @@ func (h *handler) ReceiveMessage(channel courier.Channel, w http.ResponseWriter,
 	te := &telegramEnvelope{}
 	err := handlers.DecodeAndValidateJSON(te, r)
 	if err != nil {
-		return nil, err
+		return nil, courier.WriteError(w, r, err)
 	}
 
 	// no message? ignore this
@@ -98,7 +98,7 @@ func (h *handler) ReceiveMessage(channel courier.Channel, w http.ResponseWriter,
 
 	// we had an error downloading media
 	if err != nil {
-		return nil, errors.WrapPrefix(err, "error retrieving media", 0)
+		return nil, courier.WriteError(w, r, errors.WrapPrefix(err, "error retrieving media", 0))
 	}
 
 	// build our msg

sender.go

@@ -174,11 +174,15 @@ func (w *Sender) Send() {
 			secondDuration := float64(duration) / float64(time.Second)
 
 			if err != nil {
-				// sender didn't give us a status, build one ourselves
+				msgLog.WithError(err).WithField("elapsed", duration).Error("error sending message")
 				if status == nil {
 					status = backend.NewMsgStatusForID(msg.Channel(), msg.ID(), MsgErrored)
 				}
-				msgLog.WithError(err).WithField("elapsed", duration).Error("msg errored")
+			}
+
+			// report to librato and log locally
+			if status.Status() == MsgErrored || status.Status() == MsgFailed {
+				msgLog.WithField("elapsed", duration).Warning("msg errored")
 				librato.Default.AddGauge(fmt.Sprintf("courier.msg_send_error_%s", msg.Channel().ChannelType()), secondDuration)
 			} else {
 				msgLog.WithField("elapsed", duration).Info("msg sent")

server.go

@@ -286,9 +286,13 @@ func (s *server) channelUpdateStatusWrapper(handler ChannelHandler, handlerFunc
 		duration := time.Now().Sub(start)
 		secondDuration := float64(duration) / float64(time.Second)
 
-		// create channel logs for each of our msgs or errors
+		// we received an error, write it out
 		if err != nil {
 			WriteError(ww, r, err)
+		}
+
+		// if we don't have any statuses we still want a channel log, create one
+		if len(statuses) == 0 {
 			logs = append(logs, NewChannelLog(channel, NilMsgID, r.Method, url, ww.Status(), err, string(request), response.String(), duration, start))
 			librato.Default.AddGauge(fmt.Sprintf("courier.msg_status_error_%s", channel.ChannelType()), secondDuration)
 		}
@@ -329,12 +333,18 @@ func (s *server) channelReceiveMsgWrapper(handler ChannelHandler, handlerFunc Ch
 		duration := time.Now().Sub(start)
 		secondDuration := float64(duration) / float64(time.Second)
 
-		// create channel logs for each of our msgs or errors
+		// if we received an error, write it out
 		if err != nil {
 			WriteError(ww, r, err)
+		}
+
+		// if no messages were created we still want to log this to the channel, do so
+		if len(msgs) == 0 {
 			logs = append(logs, NewChannelLog(channel, NilMsgID, r.Method, url, ww.Status(), err, string(request), prependHeaders(response.String(), ww.Status(), w), duration, start))
 			librato.Default.AddGauge(fmt.Sprintf("courier.msg_receive_error_%s", channel.ChannelType()), secondDuration)
 		}
+
+		// otherwise, log the request for each message
 		for _, msg := range msgs {
 			logs = append(logs, NewChannelLog(channel, msg.ID(), r.Method, url, ww.Status(), err, string(request), prependHeaders(response.String(), ww.Status(), w), duration, start))
 			librato.Default.AddGauge(fmt.Sprintf("courier.msg_receive_%s", channel.ChannelType()), secondDuration)