Skip to content

Commit 60d1e6c

Browse files
rowanseymourrowanseymour
authored
Merge pull request #827 from nyaruka/EX-response-check
Do not log response check error after 200 status
2 parents d99e7b6 + ada841b commit 60d1e6c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+302
-55
lines changed

Diff for: ‎handlers/africastalking/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
156156
// was this request successful?
157157
msgStatus, _ := jsonparser.GetString(respBody, "SMSMessageData", "Recipients", "[0]", "status")
158158
if msgStatus != "Success" {
159-
return courier.ErrResponseUnexpected
159+
return courier.ErrResponseContent
160160
}
161161

162162
// grab the external id if we can

Diff for: ‎handlers/africastalking/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ var outgoingCases = []OutgoingTestCase{
153153
ExpectedRequests: []ExpectedRequest{
154154
{Form: url.Values{"message": {`Hi`}, "username": {"Username"}, "to": {"+250788383383"}, "from": {"2020"}}},
155155
},
156-
ExpectedError: courier.ErrResponseUnexpected,
156+
ExpectedError: courier.ErrResponseContent,
157157
},
158158
{
159159
Label: "Missing status value",

Diff for: ‎handlers/arabiacell/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
100100
if response.Code == "204" {
101101
res.AddExternalID(response.MessageID)
102102
} else {
103-
return courier.ErrResponseUnexpected
103+
return courier.ErrResponseContent
104104
}
105105
}
106106

Diff for: ‎handlers/arabiacell/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ var outgoingCases = []OutgoingTestCase{
8888
httpx.NewMockResponse(200, nil, []byte(`<response><code>501</code><text>failure</text><message_id></message_id></response>`)),
8989
},
9090
},
91-
ExpectedError: courier.ErrResponseUnexpected,
91+
ExpectedError: courier.ErrResponseContent,
9292
},
9393
{
9494
Label: "Error Sending",

Diff for: ‎handlers/bongolive/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
166166

167167
msgStatus, _ := jsonparser.GetString(respBody, "results", "[0]", "status")
168168
if msgStatus != "0" {
169-
return courier.ErrResponseUnexpected
169+
return courier.ErrResponseContent
170170
}
171171

172172
// grab the external id if we can

Diff for: ‎handlers/bongolive/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ var outgoingCases = []OutgoingTestCase{
128128
},
129129
},
130130
},
131-
ExpectedError: courier.ErrResponseUnexpected,
131+
ExpectedError: courier.ErrResponseContent,
132132
},
133133
{
134134
Label: "Error status 403",

Diff for: ‎handlers/burstsms/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
9494
if response.MessageID != 0 {
9595
res.AddExternalID(fmt.Sprintf("%d", response.MessageID))
9696
} else {
97-
return courier.ErrResponseUnexpected
97+
return courier.ErrResponseContent
9898
}
9999
}
100100

Diff for: ‎handlers/burstsms/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ var outgoingCases = []OutgoingTestCase{
114114
},
115115
},
116116
},
117-
ExpectedError: courier.ErrResponseUnexpected,
117+
ExpectedError: courier.ErrResponseContent,
118118
},
119119
{
120120
Label: "Error Sending",

Diff for: ‎handlers/clickatell/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
188188
if externalID != "" {
189189
res.AddExternalID(externalID)
190190
} else {
191-
return courier.ErrResponseUnexpected
191+
return courier.ErrResponseContent
192192
}
193193
}
194194

Diff for: ‎handlers/clickatell/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ var outgoingCases = []OutgoingTestCase{
224224
ExpectedRequests: []ExpectedRequest{
225225
{Params: url.Values{"content": {"Error Message"}, "to": {"250788383383"}, "from": {"2020"}, "apiKey": {"API-KEY"}}},
226226
},
227-
ExpectedError: courier.ErrResponseUnexpected,
227+
ExpectedError: courier.ErrResponseContent,
228228
},
229229
}
230230

Diff for: ‎handlers/clickmobile/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
146146

147147
responseCode, _ := jsonparser.GetString(respBody, "code")
148148
if responseCode != "000" {
149-
return courier.ErrResponseUnexpected
149+
return courier.ErrResponseContent
150150
}
151151
}
152152

Diff for: ‎handlers/clickmobile/handler_test.go

+17
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,23 @@ var outgoingCases = []OutgoingTestCase{
206206
},
207207
},
208208
},
209+
{
210+
Label: "Response unexpected",
211+
MsgText: "Simple Message",
212+
MsgURN: "tel:+250788383383",
213+
MockResponses: map[string][]*httpx.MockResponse{
214+
"http://example.com/send": {
215+
httpx.NewMockResponse(200, nil, []byte(`{"code":"001","desc":"Database SQL Error"}`)),
216+
},
217+
},
218+
ExpectedRequests: []ExpectedRequest{
219+
{
220+
Headers: map[string]string{"Content-Type": "application/json"},
221+
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"}`,
222+
},
223+
},
224+
ExpectedError: courier.ErrResponseContent,
225+
},
209226
}
210227

211228
func TestOutgoing(t *testing.T) {

Diff for: ‎handlers/clicksend/handler.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,14 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
9393

9494
s, _ := jsonparser.GetString(respBody, "data", "messages", "[0]", "status")
9595
if s != "SUCCESS" {
96-
return courier.ErrResponseUnexpected
96+
return courier.ErrResponseContent
9797
}
9898

9999
id, _ := jsonparser.GetString(respBody, "data", "messages", "[0]", "message_id")
100100
if id != "" {
101101
res.AddExternalID(id)
102102
} else {
103-
return courier.ErrResponseUnexpected
103+
return courier.ErrResponseContent
104104
}
105105
}
106106

Diff for: ‎handlers/clicksend/handler_test.go

+47-1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,35 @@ const successResponse = `{
7272
]
7373
}`
7474

75+
const successResponseMissingMessageID = `{
76+
"http_code": 200,
77+
"response_code": "SUCCESS",
78+
"response_msg": "Here are your data.",
79+
"data": {
80+
"total_price": 0.28,
81+
"total_count": 2,
82+
"queued_count": 2,
83+
"messages": [
84+
{
85+
"direction": "out",
86+
"date": 1436871253,
87+
"to": "+61411111111",
88+
"body": "Jelly liquorice marshmallow candy carrot cake 4Eyffjs1vL.",
89+
"from": "sendmobile",
90+
"schedule": 1436874701,
91+
"message_id": "",
92+
"message_parts": 1,
93+
"message_price": 0.07,
94+
"custom_string": "this is a test",
95+
"user_id": 1,
96+
"subaccount_id": 1,
97+
"country": "AU",
98+
"carrier": "Telstra",
99+
"status": "SUCCESS"
100+
}
101+
]
102+
}`
103+
75104
const failureResponse = `{
76105
"http_code": 200,
77106
"response_code": "SUCCESS",
@@ -186,7 +215,24 @@ var outgoingCases = []OutgoingTestCase{
186215
Body: `{"messages":[{"to":"+250788383383","from":"2020","body":"Error Sending","source":"courier"}]}`,
187216
},
188217
},
189-
ExpectedError: courier.ErrResponseUnexpected,
218+
ExpectedError: courier.ErrResponseContent,
219+
},
220+
{
221+
Label: "Failure Response",
222+
MsgText: "Error Sending",
223+
MsgURN: "tel:+250788383383",
224+
MockResponses: map[string][]*httpx.MockResponse{
225+
"https://rest.clicksend.com/v3/sms/send": {
226+
httpx.NewMockResponse(200, nil, []byte(successResponseMissingMessageID)),
227+
},
228+
},
229+
ExpectedRequests: []ExpectedRequest{
230+
{
231+
Headers: map[string]string{"Authorization": "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=="},
232+
Body: `{"messages":[{"to":"+250788383383","from":"2020","body":"Error Sending","source":"courier"}]}`,
233+
},
234+
},
235+
ExpectedError: courier.ErrResponseContent,
190236
},
191237
}
192238

Diff for: ‎handlers/dmark/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
142142
// grab the external id
143143
externalID, err := jsonparser.GetString(respBody, "sms_id")
144144
if err != nil {
145-
return courier.ErrResponseUnexpected
145+
return courier.ErrResponseContent
146146
}
147147

148148
res.AddExternalID(externalID)

Diff for: ‎handlers/dmark/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ var defaultSendTestCases = []OutgoingTestCase{
131131
"dlr_url": {"https://localhost/c/dk/8eb23e93-5ecb-45ba-b726-3b064e0c56ab/status?id=10&status=%s"},
132132
},
133133
}},
134-
ExpectedError: courier.ErrResponseUnexpected,
134+
ExpectedError: courier.ErrResponseContent,
135135
},
136136
{
137137
Label: "Error Sending",

Diff for: ‎handlers/external/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
366366
}
367367

368368
if responseCheck != "" && !strings.Contains(string(respBody), responseCheck) {
369-
return courier.ErrResponseUnexpectedUnlogged
369+
return courier.ErrResponseContent
370370
}
371371
}
372372

Diff for: ‎handlers/external/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,7 @@ var xmlSendWithResponseContentTestCases = []OutgoingTestCase{
834834
Headers: map[string]string{"Content-Type": "text/xml; charset=utf-8"},
835835
Body: `<msg><to>+250788383383</to><text>Error Message</text><from>2020</from><quick_replies></quick_replies></msg>`,
836836
}},
837-
ExpectedError: courier.ErrResponseUnexpectedUnlogged,
837+
ExpectedError: courier.ErrResponseContent,
838838
},
839839
{
840840
Label: "Send Attachment",

Diff for: ‎handlers/firebase/handler.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,12 @@ func (h *handler) sendWithAPIKey(msg courier.MsgOut, res *courier.SendResult, cl
238238
// was this successful
239239
success, _ := jsonparser.GetInt(respBody, "success")
240240
if success != 1 {
241-
return courier.ErrResponseUnexpected
241+
return courier.ErrResponseContent
242242
}
243243

244244
externalID, err := jsonparser.GetInt(respBody, "multicast_id")
245245
if err != nil {
246-
return courier.ErrResponseUnexpected
246+
return courier.ErrResponseContent
247247
}
248248
res.AddExternalID(fmt.Sprintf("%d", externalID))
249249

@@ -319,15 +319,15 @@ func (h *handler) sendWithCredsJSON(msg courier.MsgOut, res *courier.SendResult,
319319

320320
responseName, err := jsonparser.GetString(respBody, "name")
321321
if err != nil {
322-
return courier.ErrResponseUnexpected
322+
return courier.ErrResponseContent
323323
}
324324

325325
if !strings.Contains(responseName, fmt.Sprintf("projects/%s/messages/", projectID)) {
326-
return courier.ErrResponseUnexpected
326+
return courier.ErrResponseContent
327327
}
328328
externalID := strings.TrimLeft(responseName, fmt.Sprintf("projects/%s/messages/", projectID))
329329
if externalID == "" {
330-
return courier.ErrResponseUnexpected
330+
return courier.ErrResponseContent
331331
}
332332

333333
res.AddExternalID(externalID)

Diff for: ‎handlers/firebase/handler_test.go

+52-4
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ var sendAPIkeyTestCases = []OutgoingTestCase{
226226
Headers: map[string]string{"Authorization": "key=FCMKey"},
227227
Body: `{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":10,"session_status":""},"content_available":false,"to":"auth1","priority":"high"}`,
228228
}},
229-
ExpectedError: courier.ErrResponseUnexpected,
229+
ExpectedError: courier.ErrResponseContent,
230230
},
231231
{
232232
Label: "No Multicast ID",
@@ -242,7 +242,7 @@ var sendAPIkeyTestCases = []OutgoingTestCase{
242242
Headers: map[string]string{"Authorization": "key=FCMKey"},
243243
Body: `{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":10,"session_status":""},"content_available":false,"to":"auth1","priority":"high"}`,
244244
}},
245-
ExpectedError: courier.ErrResponseUnexpected,
245+
ExpectedError: courier.ErrResponseContent,
246246
},
247247
{
248248
Label: "Request Error",
@@ -353,7 +353,7 @@ var sendTestCases = []OutgoingTestCase{
353353
Headers: map[string]string{"Authorization": "Bearer FCMToken"},
354354
Body: `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
355355
}},
356-
ExpectedError: courier.ErrResponseUnexpected,
356+
ExpectedError: courier.ErrResponseContent,
357357
},
358358
{
359359
Label: "No Multicast ID",
@@ -369,7 +369,7 @@ var sendTestCases = []OutgoingTestCase{
369369
Headers: map[string]string{"Authorization": "Bearer FCMToken"},
370370
Body: `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Error","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
371371
}},
372-
ExpectedError: courier.ErrResponseUnexpected,
372+
ExpectedError: courier.ErrResponseContent,
373373
},
374374
{
375375
Label: "Request Error",
@@ -387,6 +387,54 @@ var sendTestCases = []OutgoingTestCase{
387387
}},
388388
ExpectedError: courier.ErrConnectionFailed,
389389
},
390+
{
391+
Label: "Response Unexpected",
392+
MsgText: "Simple Message",
393+
MsgURN: "fcm:250788123123",
394+
MsgURNAuth: "auth1",
395+
MockResponses: map[string][]*httpx.MockResponse{
396+
"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
397+
httpx.NewMockResponse(200, nil, []byte(`{"missing_name":"projects/foo-project-id/messages/123456-a"}`)),
398+
},
399+
},
400+
ExpectedRequests: []ExpectedRequest{{
401+
Headers: map[string]string{"Authorization": "Bearer FCMToken"},
402+
Body: `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
403+
}},
404+
ExpectedError: courier.ErrResponseContent,
405+
},
406+
{
407+
Label: "Response Unexpected",
408+
MsgText: "Simple Message",
409+
MsgURN: "fcm:250788123123",
410+
MsgURNAuth: "auth1",
411+
MockResponses: map[string][]*httpx.MockResponse{
412+
"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
413+
httpx.NewMockResponse(200, nil, []byte(`{"name":"projects/not-our-project-id/messages/123456-a"}`)),
414+
},
415+
},
416+
ExpectedRequests: []ExpectedRequest{{
417+
Headers: map[string]string{"Authorization": "Bearer FCMToken"},
418+
Body: `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
419+
}},
420+
ExpectedError: courier.ErrResponseContent,
421+
},
422+
{
423+
Label: "Response Unexpected",
424+
MsgText: "Simple Message",
425+
MsgURN: "fcm:250788123123",
426+
MsgURNAuth: "auth1",
427+
MockResponses: map[string][]*httpx.MockResponse{
428+
"https://fcm.googleapis.com/v1/projects/foo-project-id/messages:send": {
429+
httpx.NewMockResponse(200, nil, []byte(`{"name":"projects/foo-project-id/messages/"}`)),
430+
},
431+
},
432+
ExpectedRequests: []ExpectedRequest{{
433+
Headers: map[string]string{"Authorization": "Bearer FCMToken"},
434+
Body: `{"message":{"data":{"type":"rapidpro","title":"FCMTitle","message":"Simple Message","message_id":"10","session_status":""},"token":"auth1","android":{"priority":"high"}}}`,
435+
}},
436+
ExpectedError: courier.ErrResponseContent,
437+
},
390438
}
391439

392440
func setupBackend(mb *test.MockBackend) {

Diff for: ‎handlers/infobip/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
214214

215215
groupID, err := jsonparser.GetInt(respBody, "messages", "[0]", "status", "groupId")
216216
if err != nil || (groupID != 1 && groupID != 3) {
217-
return courier.ErrResponseUnexpected
217+
return courier.ErrResponseContent
218218
}
219219

220220
externalID, err := jsonparser.GetString(respBody, "messages", "[0]", "messageId")

Diff for: ‎handlers/infobip/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ var defaultSendTestCases = []OutgoingTestCase{
393393
},
394394
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"}]}`,
395395
}},
396-
ExpectedError: courier.ErrResponseUnexpected,
396+
ExpectedError: courier.ErrResponseContent,
397397
},
398398
}
399399

Diff for: ‎handlers/justcall/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
199199
clog.Error(courier.ErrorResponseValueMissing("status"))
200200
}
201201
if respStatus != "success" {
202-
return courier.ErrResponseUnexpected
202+
return courier.ErrResponseContent
203203

204204
}
205205

Diff for: ‎handlers/justcall/handler_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ var defaultSendTestCases = []OutgoingTestCase{
345345
},
346346
Body: `{"from":"2020","to":"+250788383383","body":"Error"}`,
347347
}},
348-
ExpectedError: courier.ErrResponseUnexpected,
348+
ExpectedError: courier.ErrResponseContent,
349349
},
350350
{
351351
Label: "Error",

0 commit comments

Comments
 (0)