From 9d7e7447a1bf54e78c18599e6d3d17df95290e13 Mon Sep 17 00:00:00 2001 From: Rowan Seymour Date: Mon, 23 Aug 2021 14:09:00 -0500 Subject: [PATCH] Use synchronous answering machine detection on Twilio channels --- core/ivr/twiml/twiml.go | 33 ++++------- web/ivr/ivr_test.go | 121 ++++++++++++---------------------------- 2 files changed, 47 insertions(+), 107 deletions(-) diff --git a/core/ivr/twiml/twiml.go b/core/ivr/twiml/twiml.go index a54b51479..5cb05f29c 100644 --- a/core/ivr/twiml/twiml.go +++ b/core/ivr/twiml/twiml.go @@ -6,7 +6,6 @@ import ( "crypto/hmac" "crypto/sha1" "encoding/base64" - "encoding/json" "encoding/xml" "fmt" "net/http" @@ -177,8 +176,9 @@ func (c *client) URNForRequest(r *http.Request) (urns.URN, error) { // CallResponse is our struct for a Twilio call response type CallResponse struct { - SID string `json:"sid"` - Status string `json:"status"` + SID string `json:"sid" validate:"required"` + Status string `json:"status"` + AnsweredBy string `json:"answered_by"` } // RequestCall causes this client to request a new outgoing call for this provider @@ -191,8 +191,6 @@ func (c *client) RequestCall(number urns.URN, callbackURL string, statusURL stri if machineDetection { form.Set("MachineDetection", "Enable") - form.Set("AsyncAmd", "true") - form.Set("AsyncAmdStatusCallback", statusURL) } sendURL := c.baseURL + strings.Replace(callPath, "{AccountSID}", c.accountSID, -1) @@ -206,16 +204,17 @@ func (c *client) RequestCall(number urns.URN, callbackURL string, statusURL stri return ivr.NilCallID, trace, errors.Errorf("received non 201 status for call start: %d", trace.Response.StatusCode) } - // parse out our call sid + // parse the response from Twilio call := &CallResponse{} - err = json.Unmarshal(trace.ResponseBody, call) - if err != nil || call.SID == "" { - return ivr.NilCallID, trace, errors.Errorf("unable to read call id") + if err := utils.UnmarshalAndValidate(trace.ResponseBody, call); err != nil { + return ivr.NilCallID, trace, errors.Wrap(err, "unable parse Twilio response") } - if call.Status == statusFailed { return ivr.NilCallID, trace, errors.Errorf("call status returned as failed") } + if call.AnsweredBy == "machine_start" || call.AnsweredBy == "fax" { + return ivr.NilCallID, trace, errors.Errorf("call answered by machine") + } return ivr.CallID(call.SID), trace, nil } @@ -293,25 +292,13 @@ func (c *client) ResumeForRequest(r *http.Request) (ivr.Resume, error) { // StatusForRequest returns the call status for the passed in request, and if it's an error the reason, // and if available, the current call duration func (c *client) StatusForRequest(r *http.Request) (models.ConnectionStatus, models.ConnectionError, int) { - // we re-use our status callback for AMD results which will have an AnsweredBy field but no CallStatus field - answeredBy := r.Form.Get("AnsweredBy") - if answeredBy != "" { - switch answeredBy { - case "machine_start", "fax": - return models.ConnectionStatusErrored, models.ConnectionErrorMachine, 0 - } - return models.ConnectionStatusInProgress, "", 0 - } - status := r.Form.Get("CallStatus") switch status { case "queued", "ringing": return models.ConnectionStatusWired, "", 0 - case "in-progress", "initiated": return models.ConnectionStatusInProgress, "", 0 - case "completed": duration, _ := strconv.Atoi(r.Form.Get("CallDuration")) return models.ConnectionStatusCompleted, "", duration @@ -321,7 +308,7 @@ func (c *client) StatusForRequest(r *http.Request) (models.ConnectionStatus, mod case "no-answer": return models.ConnectionStatusErrored, models.ConnectionErrorNoAnswer, 0 case "canceled", "failed": - return models.ConnectionStatusErrored, "", 0 + return models.ConnectionStatusErrored, models.ConnectionErrorProvider, 0 default: logrus.WithField("call_status", status).Error("unknown call status in status callback") diff --git a/web/ivr/ivr_test.go b/web/ivr/ivr_test.go index 2ef31f101..2e7aa0a5e 100644 --- a/web/ivr/ivr_test.go +++ b/web/ivr/ivr_test.go @@ -123,8 +123,6 @@ func TestTwilioIVR(t *testing.T) { testdata.Cathy.ID, models.ConnectionStatusWired, "Call1").Returns(1) testsuite.AssertQuery(t, db, `SELECT COUNT(*) FROM channels_channelconnection WHERE contact_id = $1 AND status = $2 AND external_id = $3`, testdata.Bob.ID, models.ConnectionStatusWired, "Call2").Returns(1) - testsuite.AssertQuery(t, db, `SELECT COUNT(*) FROM channels_channelconnection WHERE contact_id = $1 AND status = $2 AND external_id = $3`, - testdata.George.ID, models.ConnectionStatusWired, "Call3").Returns(1) tcs := []struct { label string @@ -252,51 +250,6 @@ func TestTwilioIVR(t *testing.T) { expectedResponse: `An error has occurred, please try again later.`, expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W"}, }, - { - label: "call3 started", - url: fmt.Sprintf("/ivr/c/%s/handle?action=start&connection=3", testdata.TwilioChannel.UUID), - form: nil, - expectedStatus: 200, - expectedContains: []string{`Hello there. Please enter one or two. This flow was triggered by Cathy`}, - expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "I"}, - }, - { - label: "answer machine detection sent to tell us we're talking to a voicemail", - url: fmt.Sprintf("/ivr/c/%s/status", testdata.TwilioChannel.UUID), - form: url.Values{ - "CallSid": []string{"Call3"}, - "AccountSid": []string{"sid"}, - "AnsweredBy": []string{"machine_start"}, - "MachineDetectionDuration": []string{"2000"}, - }, - expectedStatus: 200, - expectedContains: []string{"An error has occurred, please try again later.`, - expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E"}, - }, - { - label: "then Twilio will call the status callback to say that we're done but don't overwrite the error status", - url: fmt.Sprintf("/ivr/c/%s/status", testdata.TwilioChannel.UUID), - form: url.Values{ - "CallSid": []string{"Call3"}, - "CallStatus": []string{"completed"}, - "CallDuration": []string{"50"}, - }, - expectedStatus: 200, - expectedResponse: ``, - expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E"}, - }, { label: "we don't have a call trigger so incoming call creates a missed call event", url: fmt.Sprintf("/ivr/c/%s/incoming", testdata.TwilioChannel.UUID), @@ -307,7 +260,7 @@ func TestTwilioIVR(t *testing.T) { }, expectedStatus: 200, expectedResponse: ``, - expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E", "Call4": "I"}, + expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W", "Call4": "I"}, }, { label: "", @@ -319,7 +272,7 @@ func TestTwilioIVR(t *testing.T) { }, expectedStatus: 200, expectedResponse: ``, - expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E", "Call4": "F"}, + expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W", "Call4": "F"}, }, } @@ -363,8 +316,41 @@ func TestTwilioIVR(t *testing.T) { testsuite.AssertQuery(t, db, `SELECT count(*) FROM msgs_msg WHERE contact_id = $1 AND msg_type = 'V' AND connection_id = 2 AND ((status = 'H' AND direction = 'I') OR (status = 'W' AND direction = 'O'))`, testdata.Bob.ID).Returns(2) +} - testsuite.AssertQuery(t, db, `SELECT status, error_reason FROM channels_channelconnection WHERE contact_id = $1 AND next_attempt IS NOT NULL`, testdata.George.ID).Columns(map[string]interface{}{"status": "E", "error_reason": "M"}) +func mockVonageHandler(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("recording") != "" { + w.WriteHeader(http.StatusOK) + w.Write([]byte{}) + } else { + type CallForm struct { + To []struct { + Number string `json:"number"` + } `json:"to"` + Action string `json:"action,omitempty"` + } + body, _ := io.ReadAll(r.Body) + form := &CallForm{} + json.Unmarshal(body, form) + logrus.WithField("method", r.Method).WithField("url", r.URL.String()).WithField("body", string(body)).WithField("form", form).Info("test server called") + + // end of a leg + if form.Action == "transfer" { + w.WriteHeader(http.StatusNoContent) + } else if form.To[0].Number == "16055741111" { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{ "uuid": "Call1","status": "started","direction": "outbound","conversation_uuid": "Conversation1"}`)) + } else if form.To[0].Number == "16055743333" { + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{ "uuid": "Call2","status": "started","direction": "outbound","conversation_uuid": "Conversation2"}`)) + } else if form.To[0].Number == "2065551212" { + // start of a transfer leg + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{ "uuid": "Call3","status": "started","direction": "outbound","conversation_uuid": "Conversation3"}`)) + } else { + w.WriteHeader(http.StatusNotFound) + } + } } func TestVonageIVR(t *testing.T) { @@ -384,40 +370,7 @@ func TestVonageIVR(t *testing.T) { db.MustExec(`UPDATE channels_channel SET config = '{"nexmo_app_id": "app_id", "nexmo_app_private_key": "-----BEGIN PRIVATE KEY-----\nMIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKNwapOQ6rQJHetP\nHRlJBIh1OsOsUBiXb3rXXE3xpWAxAha0MH+UPRblOko+5T2JqIb+xKf9Vi3oTM3t\nKvffaOPtzKXZauscjq6NGzA3LgeiMy6q19pvkUUOlGYK6+Xfl+B7Xw6+hBMkQuGE\nnUS8nkpR5mK4ne7djIyfHFfMu4ptAgMBAAECgYA+s0PPtMq1osG9oi4xoxeAGikf\nJB3eMUptP+2DYW7mRibc+ueYKhB9lhcUoKhlQUhL8bUUFVZYakP8xD21thmQqnC4\nf63asad0ycteJMLb3r+z26LHuCyOdPg1pyLk3oQ32lVQHBCYathRMcVznxOG16VK\nI8BFfstJTaJu0lK/wQJBANYFGusBiZsJQ3utrQMVPpKmloO2++4q1v6ZR4puDQHx\nTjLjAIgrkYfwTJBLBRZxec0E7TmuVQ9uJ+wMu/+7zaUCQQDDf2xMnQqYknJoKGq+\noAnyC66UqWC5xAnQS32mlnJ632JXA0pf9pb1SXAYExB1p9Dfqd3VAwQDwBsDDgP6\nHD8pAkEA0lscNQZC2TaGtKZk2hXkdcH1SKru/g3vWTkRHxfCAznJUaza1fx0wzdG\nGcES1Bdez0tbW4llI5By/skZc2eE3QJAFl6fOskBbGHde3Oce0F+wdZ6XIJhEgCP\niukIcKZoZQzoiMJUoVRrA5gqnmaYDI5uRRl/y57zt6YksR3KcLUIuQJAd242M/WF\n6YAZat3q/wEeETeQq1wrooew+8lHl05/Nt0cCpV48RGEhJ83pzBm3mnwHf8lTBJH\nx6XroMXsmbnsEw==\n-----END PRIVATE KEY-----", "callback_domain": "localhost:8090"}', role='SRCA' WHERE id = $1`, testdata.VonageChannel.ID) // start test server - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Query().Get("recording") != "" { - w.WriteHeader(http.StatusOK) - w.Write([]byte{}) - } else { - type CallForm struct { - To []struct { - Number string `json:"number"` - } `json:"to"` - Action string `json:"action,omitempty"` - } - body, _ := io.ReadAll(r.Body) - form := &CallForm{} - json.Unmarshal(body, form) - logrus.WithField("method", r.Method).WithField("url", r.URL.String()).WithField("body", string(body)).WithField("form", form).Info("test server called") - - // end of a leg - if form.Action == "transfer" { - w.WriteHeader(http.StatusNoContent) - } else if form.To[0].Number == "16055741111" { - w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{ "uuid": "Call1","status": "started","direction": "outbound","conversation_uuid": "Conversation1"}`)) - } else if form.To[0].Number == "16055743333" { - w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{ "uuid": "Call2","status": "started","direction": "outbound","conversation_uuid": "Conversation2"}`)) - } else if form.To[0].Number == "2065551212" { - // start of a transfer leg - w.WriteHeader(http.StatusCreated) - w.Write([]byte(`{ "uuid": "Call3","status": "started","direction": "outbound","conversation_uuid": "Conversation3"}`)) - } else { - w.WriteHeader(http.StatusNotFound) - } - } - })) + ts := httptest.NewServer(http.HandlerFunc(mockVonageHandler)) defer ts.Close() wg := &sync.WaitGroup{}