Handle AMD result in first callback rather than call request response

core/ivr/ivr.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/nyaruka/gocommon/dates"
 	"github.com/nyaruka/gocommon/httpx"
 	"github.com/nyaruka/gocommon/storage"
 	"github.com/nyaruka/gocommon/urns"
@@ -82,6 +83,9 @@ type Client interface {
 	// and if available, the current call duration
 	StatusForRequest(r *http.Request) (models.ConnectionStatus, models.ConnectionError, int)
 
+	// CheckStartRequest checks the start request from the provider is as we expect and if not returns an error reason
+	CheckStartRequest(r *http.Request) models.ConnectionError
+
 	PreprocessResume(ctx context.Context, db *sqlx.DB, rp *redis.Pool, conn *models.ChannelConnection, r *http.Request) ([]byte, error)
 
 	PreprocessStatus(ctx context.Context, db *sqlx.DB, rp *redis.Pool, r *http.Request) ([]byte, error)
@@ -302,7 +306,7 @@ func StartIVRFlow(
 	channel *models.Channel, conn *models.ChannelConnection, c *models.Contact, urn urns.URN, startID models.StartID,
 	r *http.Request, w http.ResponseWriter) error {
 
-	// connection isn't in a wired status, that's an error
+	// connection isn't in a wired or in-progress status then we shouldn't be here
 	if conn.Status() != models.ConnectionStatusWired && conn.Status() != models.ConnectionStatusInProgress {
 		return WriteErrorResponse(ctx, rt.DB, client, conn, w, errors.Errorf("connection in invalid state: %s", conn.Status()))
 	}
@@ -312,12 +316,26 @@ func StartIVRFlow(
 	if err != nil {
 		return errors.Wrapf(err, "unable to load start: %d", startID)
 	}
-
 	flow, err := oa.FlowByID(start.FlowID())
 	if err != nil {
 		return errors.Wrapf(err, "unable to load flow: %d", startID)
 	}
 
+	// check that call on provider side is in the state we need to continue
+	if errorReason := client.CheckStartRequest(r); errorReason != "" {
+		err := conn.MarkErrored(ctx, rt.DB, dates.Now(), flow.IVRRetryWait(), errorReason)
+		if err != nil {
+			return errors.Wrap(err, "unable to mark connection as errored")
+		}
+
+		errMsg := fmt.Sprintf("status updated: %s", conn.Status())
+		if conn.Status() == models.ConnectionStatusErrored {
+			errMsg = fmt.Sprintf("%s, next_attempt: %s", errMsg, conn.NextAttempt())
+		}
+
+		return client.WriteErrorResponse(w, errors.New(errMsg))
+	}
+
 	// our flow contact
 	contact, err := c.FlowContact(oa)
 	if err != nil {
@@ -620,10 +638,10 @@ func HandleIVRStatus(ctx context.Context, rt *runtime.Runtime, oa *models.OrgAss
 			return errors.Wrapf(err, "unable to load flow: %d", start.FlowID())
 		}
 
-		conn.MarkErrored(ctx, rt.DB, time.Now(), flow.IVRRetryWait(), errorReason)
+		conn.MarkErrored(ctx, rt.DB, dates.Now(), flow.IVRRetryWait(), errorReason)
 
 		if conn.Status() == models.ConnectionStatusErrored {
-			return client.WriteEmptyResponse(w, fmt.Sprintf("status updated: %s next_attempt: %s", conn.Status(), conn.NextAttempt()))
+			return client.WriteEmptyResponse(w, fmt.Sprintf("status updated: %s, next_attempt: %s", conn.Status(), conn.NextAttempt()))
 		}
 
 	} else if status == models.ConnectionStatusFailed {

core/ivr/twiml/twiml.go

@@ -148,6 +148,15 @@ func (c *client) DownloadMedia(url string) (*http.Response, error) {
 	return http.Get(url)
 }
 
+func (c *client) CheckStartRequest(r *http.Request) models.ConnectionError {
+	r.ParseForm()
+	answeredBy := r.Form.Get("AnsweredBy")
+	if answeredBy == "machine_start" || answeredBy == "fax" {
+		return models.ConnectionErrorMachine
+	}
+	return ""
+}
+
 func (c *client) PreprocessStatus(ctx context.Context, db *sqlx.DB, rp *redis.Pool, r *http.Request) ([]byte, error) {
 	return nil, nil
 }
@@ -176,9 +185,8 @@ 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" validate:"required"`
-	Status     string `json:"status"`
-	AnsweredBy string `json:"answered_by"`
+	SID    string `json:"sid" validate:"required"`
+	Status string `json:"status"`
 }
 
 // RequestCall causes this client to request a new outgoing call for this provider
@@ -212,9 +220,6 @@ func (c *client) RequestCall(number urns.URN, callbackURL string, statusURL stri
 	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
 }

core/ivr/vonage/vonage.go

@@ -170,6 +170,10 @@ func (c *client) DownloadMedia(url string) (*http.Response, error) {
 	return http.DefaultClient.Do(req)
 }
 
+func (c *client) CheckStartRequest(r *http.Request) models.ConnectionError {
+	return ""
+}
+
 func (c *client) PreprocessStatus(ctx context.Context, db *sqlx.DB, rp *redis.Pool, r *http.Request) ([]byte, error) {
 	// parse out the call status, we are looking for a leg of one of our conferences ending in the "forward" case
 	// get our recording url out

core/tasks/ivr/worker_test.go

@@ -106,6 +106,10 @@ func (c *MockClient) StatusForRequest(r *http.Request) (models.ConnectionStatus,
 	return models.ConnectionStatusFailed, models.ConnectionErrorProvider, 10
 }
 
+func (c *MockClient) CheckStartRequest(r *http.Request) models.ConnectionError {
+	return ""
+}
+
 func (c *MockClient) PreprocessResume(ctx context.Context, db *sqlx.DB, rp *redis.Pool, conn *models.ChannelConnection, r *http.Request) ([]byte, error) {
 	return nil, nil
 }

web/ivr/ivr_test.go

@@ -250,6 +250,29 @@ func TestTwilioIVR(t *testing.T) {
 			expectedResponse:   `<Response><!--call completed--><Say>An error has occurred, please try again later.</Say><Hangup></Hangup></Response>`,
 			expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W"},
 		},
+		{
+			label: "call3 started with answered_by telling us it's a machine",
+			url:   fmt.Sprintf("/ivr/c/%s/handle?action=start&connection=3", testdata.TwilioChannel.UUID),
+			form: url.Values{
+				"CallStatus": []string{"in-progress"},
+				"AnsweredBy": []string{"machine_start"},
+			},
+			expectedStatus:     200,
+			expectedContains:   []string{`<Response><!--status updated: E, next_attempt: `, `<Say>An error has occurred, please try again later.</Say><Hangup></Hangup></Response>`},
+			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:   `<Response><!--status D ignored, already errored--></Response>`,
+			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),
@@ -260,7 +283,7 @@ func TestTwilioIVR(t *testing.T) {
 			},
 			expectedStatus:     200,
 			expectedResponse:   `<Response><!--missed call handled--></Response>`,
-			expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W", "Call4": "I"},
+			expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E", "Call4": "I"},
 		},
 		{
 			label: "",
@@ -272,7 +295,7 @@ func TestTwilioIVR(t *testing.T) {
 			},
 			expectedStatus:     200,
 			expectedResponse:   `<Response><!--no flow start found, status updated: F--></Response>`,
-			expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "W", "Call4": "F"},
+			expectedConnStatus: map[string]string{"Call1": "D", "Call2": "D", "Call3": "E", "Call4": "F"},
 		},
 	}