Merge pull request #827 from nyaruka/EX-response-check

Do not log response check error after 200 status
nyaruka · Jan 14, 2025 · 60d1e6c · 60d1e6c
2 parents d99e7b6 + ada841b
commit 60d1e6c
Show file tree

Hide file tree

Showing 49 changed files with 302 additions and 55 deletions.
diff --git a/handlers/africastalking/handler.go b/handlers/africastalking/handler.go
@@ -156,7 +156,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 	// was this request successful?
 	msgStatus, _ := jsonparser.GetString(respBody, "SMSMessageData", "Recipients", "[0]", "status")
 	if msgStatus != "Success" {
-		return courier.ErrResponseUnexpected
+		return courier.ErrResponseContent
 	}
 
 	// grab the external id if we can

diff --git a/handlers/africastalking/handler_test.go b/handlers/africastalking/handler_test.go
@@ -153,7 +153,7 @@ var outgoingCases = []OutgoingTestCase{
 		ExpectedRequests: []ExpectedRequest{
 			{Form: url.Values{"message": {`Hi`}, "username": {"Username"}, "to": {"+250788383383"}, "from": {"2020"}}},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Missing status value",

diff --git a/handlers/arabiacell/handler.go b/handlers/arabiacell/handler.go
@@ -100,7 +100,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		if response.Code == "204" {
 			res.AddExternalID(response.MessageID)
 		} else {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/arabiacell/handler_test.go b/handlers/arabiacell/handler_test.go
@@ -88,7 +88,7 @@ var outgoingCases = []OutgoingTestCase{
 				httpx.NewMockResponse(200, nil, []byte(`<response><code>501</code><text>failure</text><message_id></message_id></response>`)),
 			},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Error Sending",

diff --git a/handlers/bongolive/handler.go b/handlers/bongolive/handler.go
@@ -166,7 +166,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 
 		msgStatus, _ := jsonparser.GetString(respBody, "results", "[0]", "status")
 		if msgStatus != "0" {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		// grab the external id if we can

diff --git a/handlers/bongolive/handler_test.go b/handlers/bongolive/handler_test.go
@@ -128,7 +128,7 @@ var outgoingCases = []OutgoingTestCase{
 				},
 			},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Error status 403",

diff --git a/handlers/burstsms/handler.go b/handlers/burstsms/handler.go
@@ -94,7 +94,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		if response.MessageID != 0 {
 			res.AddExternalID(fmt.Sprintf("%d", response.MessageID))
 		} else {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/burstsms/handler_test.go b/handlers/burstsms/handler_test.go
@@ -114,7 +114,7 @@ var outgoingCases = []OutgoingTestCase{
 				},
 			},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Error Sending",

diff --git a/handlers/clickatell/handler.go b/handlers/clickatell/handler.go
@@ -188,7 +188,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		if externalID != "" {
 			res.AddExternalID(externalID)
 		} else {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/clickatell/handler_test.go b/handlers/clickatell/handler_test.go
@@ -224,7 +224,7 @@ var outgoingCases = []OutgoingTestCase{
 		ExpectedRequests: []ExpectedRequest{
 			{Params: url.Values{"content": {"Error Message"}, "to": {"250788383383"}, "from": {"2020"}, "apiKey": {"API-KEY"}}},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 }
 

diff --git a/handlers/clickmobile/handler.go b/handlers/clickmobile/handler.go
@@ -146,7 +146,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 
 		responseCode, _ := jsonparser.GetString(respBody, "code")
 		if responseCode != "000" {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/clickmobile/handler_test.go b/handlers/clickmobile/handler_test.go
@@ -206,6 +206,23 @@ var outgoingCases = []OutgoingTestCase{
 			},
 		},
 	},
+	{
+		Label:   "Response unexpected",
+		MsgText: "Simple Message",
+		MsgURN:  "tel:+250788383383",
+		MockResponses: map[string][]*httpx.MockResponse{
+			"http://example.com/send": {
+				httpx.NewMockResponse(200, nil, []byte(`{"code":"001","desc":"Database SQL Error"}`)),
+			},
+		},
+		ExpectedRequests: []ExpectedRequest{
+			{
+				Headers: map[string]string{"Content-Type": "application/json"},
+				Body:    `{"app_id":"001-app","org_id":"001-org","user_id":"Username","timestamp":"20180411182430","auth_key":"3e1347ddb444d13aa23d11e097602be0","operation":"send","reference":"10","message_type":"1","src_address":"2020","dst_address":"+250788383383","message":"Simple Message"}`,
+			},
+		},
+		ExpectedError: courier.ErrResponseContent,
+	},
 }
 
 func TestOutgoing(t *testing.T) {

diff --git a/handlers/clicksend/handler.go b/handlers/clicksend/handler.go
@@ -93,14 +93,14 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 
 		s, _ := jsonparser.GetString(respBody, "data", "messages", "[0]", "status")
 		if s != "SUCCESS" {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		id, _ := jsonparser.GetString(respBody, "data", "messages", "[0]", "message_id")
 		if id != "" {
 			res.AddExternalID(id)
 		} else {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/clicksend/handler_test.go b/handlers/clicksend/handler_test.go
@@ -72,6 +72,35 @@ const successResponse = `{
 	]
 }`
 
+const successResponseMissingMessageID = `{
+	"http_code": 200,
+	"response_code": "SUCCESS",
+	"response_msg": "Here are your data.",
+	"data": {
+	  "total_price": 0.28,
+	  "total_count": 2,
+	  "queued_count": 2,
+	  "messages": [
+		{
+		  "direction": "out",
+		  "date": 1436871253,
+		  "to": "+61411111111",
+		  "body": "Jelly liquorice marshmallow candy carrot cake 4Eyffjs1vL.",
+		  "from": "sendmobile",
+		  "schedule": 1436874701,
+		  "message_id": "",
+		  "message_parts": 1,
+		  "message_price": 0.07,
+		  "custom_string": "this is a test",
+		  "user_id": 1,
+		  "subaccount_id": 1,
+		  "country": "AU",
+		  "carrier": "Telstra",
+		  "status": "SUCCESS"
+		}
+	]
+}`
+
 const failureResponse = `{
 	"http_code": 200,
 	"response_code": "SUCCESS",
@@ -186,7 +215,24 @@ var outgoingCases = []OutgoingTestCase{
 				Body:    `{"messages":[{"to":"+250788383383","from":"2020","body":"Error Sending","source":"courier"}]}`,
 			},
 		},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
+	},
+	{
+		Label:   "Failure Response",
+		MsgText: "Error Sending",
+		MsgURN:  "tel:+250788383383",
+		MockResponses: map[string][]*httpx.MockResponse{
+			"https://rest.clicksend.com/v3/sms/send": {
+				httpx.NewMockResponse(200, nil, []byte(successResponseMissingMessageID)),
+			},
+		},
+		ExpectedRequests: []ExpectedRequest{
+			{
+				Headers: map[string]string{"Authorization": "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=="},
+				Body:    `{"messages":[{"to":"+250788383383","from":"2020","body":"Error Sending","source":"courier"}]}`,
+			},
+		},
+		ExpectedError: courier.ErrResponseContent,
 	},
 }
 

diff --git a/handlers/dmark/handler.go b/handlers/dmark/handler.go
@@ -142,7 +142,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		// grab the external id
 		externalID, err := jsonparser.GetString(respBody, "sms_id")
 		if err != nil {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		res.AddExternalID(externalID)

diff --git a/handlers/dmark/handler_test.go b/handlers/dmark/handler_test.go
@@ -131,7 +131,7 @@ var defaultSendTestCases = []OutgoingTestCase{
 				"dlr_url":  {"https://localhost/c/dk/8eb23e93-5ecb-45ba-b726-3b064e0c56ab/status?id=10&status=%s"},
 			},
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Error Sending",

diff --git a/handlers/external/handler.go b/handlers/external/handler.go
@@ -366,7 +366,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		}
 
 		if responseCheck != "" && !strings.Contains(string(respBody), responseCheck) {
-			return courier.ErrResponseUnexpectedUnlogged
+			return courier.ErrResponseContent
 		}
 	}
 

diff --git a/handlers/external/handler_test.go b/handlers/external/handler_test.go
@@ -834,7 +834,7 @@ var xmlSendWithResponseContentTestCases = []OutgoingTestCase{
 			Headers: map[string]string{"Content-Type": "text/xml; charset=utf-8"},
 			Body:    `<msg><to>+250788383383</to><text>Error Message</text><from>2020</from><quick_replies></quick_replies></msg>`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpectedUnlogged,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:          "Send Attachment",

diff --git a/handlers/firebase/handler.go b/handlers/firebase/handler.go
@@ -238,12 +238,12 @@ func (h *handler) sendWithAPIKey(msg courier.MsgOut, res *courier.SendResult, cl
 		// was this successful
 		success, _ := jsonparser.GetInt(respBody, "success")
 		if success != 1 {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		externalID, err := jsonparser.GetInt(respBody, "multicast_id")
 		if err != nil {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 		res.AddExternalID(fmt.Sprintf("%d", externalID))
 
@@ -319,15 +319,15 @@ func (h *handler) sendWithCredsJSON(msg courier.MsgOut, res *courier.SendResult,
 
 		responseName, err := jsonparser.GetString(respBody, "name")
 		if err != nil {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		if !strings.Contains(responseName, fmt.Sprintf("projects/%s/messages/", projectID)) {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 		externalID := strings.TrimLeft(responseName, fmt.Sprintf("projects/%s/messages/", projectID))
 		if externalID == "" {
-			return courier.ErrResponseUnexpected
+			return courier.ErrResponseContent
 		}
 
 		res.AddExternalID(externalID)

diff --git a/handlers/firebase/handler_test.go b/handlers/firebase/handler_test.go
@@ -226,7 +226,7 @@ var sendAPIkeyTestCases = []OutgoingTestCase{
 			Headers: map[string]string{"Authorization": "key=FCMKey"},
 			Body:    `{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":10,"session_status":""},"content_available":false,"to":"auth1","priority":"high"}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:      "No Multicast ID",
@@ -242,7 +242,7 @@ var sendAPIkeyTestCases = []OutgoingTestCase{
 			Headers: map[string]string{"Authorization": "key=FCMKey"},
 			Body:    `{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":10,"session_status":""},"content_available":false,"to":"auth1","priority":"high"}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:      "Request Error",
@@ -353,7 +353,7 @@ var sendTestCases = []OutgoingTestCase{
 			Headers: map[string]string{"Authorization": "Bearer FCMToken"},
 			Body:    `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:      "No Multicast ID",
@@ -369,7 +369,7 @@ var sendTestCases = []OutgoingTestCase{
 			Headers: map[string]string{"Authorization": "Bearer FCMToken"},
 			Body:    `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:      "Request Error",
@@ -387,6 +387,54 @@ var sendTestCases = []OutgoingTestCase{
 		}},
 		ExpectedError: courier.ErrConnectionFailed,
 	},
+	{
+		Label:      "Response Unexpected",
+		MsgText:    "Simple Message",
+		MsgURN:     "fcm:250788123123",
+		MsgURNAuth: "auth1",
+		MockResponses: map[string][]*httpx.MockResponse{
+			"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
+				httpx.NewMockResponse(200, nil, []byte(`{"missing_name":"projects/foo-project-id/messages/123456-a"}`)),
+			},
+		},
+		ExpectedRequests: []ExpectedRequest{{
+			Headers: map[string]string{"Authorization": "Bearer FCMToken"},
+			Body:    `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
+		}},
+		ExpectedError: courier.ErrResponseContent,
+	},
+	{
+		Label:      "Response Unexpected",
+		MsgText:    "Simple Message",
+		MsgURN:     "fcm:250788123123",
+		MsgURNAuth: "auth1",
+		MockResponses: map[string][]*httpx.MockResponse{
+			"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
+				httpx.NewMockResponse(200, nil, []byte(`{"name":"projects/not-our-project-id/messages/123456-a"}`)),
+			},
+		},
+		ExpectedRequests: []ExpectedRequest{{
+			Headers: map[string]string{"Authorization": "Bearer FCMToken"},
+			Body:    `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
+		}},
+		ExpectedError: courier.ErrResponseContent,
+	},
+	{
+		Label:      "Response Unexpected",
+		MsgText:    "Simple Message",
+		MsgURN:     "fcm:250788123123",
+		MsgURNAuth: "auth1",
+		MockResponses: map[string][]*httpx.MockResponse{
+			"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
+				httpx.NewMockResponse(200, nil, []byte(`{"name":"projects/foo-project-id/messages/"}`)),
+			},
+		},
+		ExpectedRequests: []ExpectedRequest{{
+			Headers: map[string]string{"Authorization": "Bearer FCMToken"},
+			Body:    `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
+		}},
+		ExpectedError: courier.ErrResponseContent,
+	},
 }
 
 func setupBackend(mb *test.MockBackend) {

diff --git a/handlers/infobip/handler.go b/handlers/infobip/handler.go
@@ -214,7 +214,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 
 	groupID, err := jsonparser.GetInt(respBody, "messages", "[0]", "status", "groupId")
 	if err != nil || (groupID != 1 && groupID != 3) {
-		return courier.ErrResponseUnexpected
+		return courier.ErrResponseContent
 	}
 
 	externalID, err := jsonparser.GetString(respBody, "messages", "[0]", "messageId")

diff --git a/handlers/infobip/handler_test.go b/handlers/infobip/handler_test.go
@@ -393,7 +393,7 @@ var defaultSendTestCases = []OutgoingTestCase{
 			},
 			Body: `{"messages":[{"from":"2020","destinations":[{"to":"250788383383","messageId":"10"}],"text":"Simple Message","notifyContentType":"application/json","intermediateReport":true,"notifyUrl":"https://localhost/c/ib/8eb23e93-5ecb-45ba-b726-3b064e0c56ab/delivered"}]}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 }
 

diff --git a/handlers/justcall/handler.go b/handlers/justcall/handler.go
@@ -199,7 +199,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		clog.Error(courier.ErrorResponseValueMissing("status"))
 	}
 	if respStatus != "success" {
-		return courier.ErrResponseUnexpected
+		return courier.ErrResponseContent
 
 	}
 

diff --git a/handlers/justcall/handler_test.go b/handlers/justcall/handler_test.go
@@ -345,7 +345,7 @@ var defaultSendTestCases = []OutgoingTestCase{
 			},
 			Body: `{"from":"2020","to":"+250788383383","body":"Error"}`,
 		}},
-		ExpectedError: courier.ErrResponseUnexpected,
+		ExpectedError: courier.ErrResponseContent,
 	},
 	{
 		Label:   "Error",
-Original file line number
+Diff line change
@@ Expand Up / @@ -128,7 +128,7 @@ var outgoingCases = []OutgoingTestCase{ @@
     				},
     			},
     		},
-    		ExpectedError: courier.ErrResponseUnexpected,
+    		ExpectedError: courier.ErrResponseContent,
     	},
     	{
     		Label:   "Error status 403",
@@ Expand Down @@