TEL-1026: Transfer error details

.changeset/sip-transfer-error-details.md

@@ -0,0 +1,6 @@
+---
+"github.com/livekit/protocol": patch
+"@livekit/protocol": patch
+---
+
+Report SIP transfer failures with a SIPTransferError detail on the error

livekit/sip.go

@@ -26,24 +26,102 @@ const MaxSIPMediaTimeout = 10 * time.Minute
 var (
 	_ xtwirp.ErrorMeta = (*SIPStatus)(nil)
 	_ error            = (*SIPStatus)(nil)
+
+	_ xtwirp.ErrorMeta = (*SIPTransferError)(nil)
+	_ error            = (*SIPTransferError)(nil)
 )
 
-// SIPStatusFrom unwraps an error and returns associated SIP call status, if any.
-func SIPStatusFrom(err error) *SIPStatus {
+// SIPTransferErrorFrom unwraps an error and returns the associated SIP transfer
+// details, if any. A rejected transfer also carries a SIPStatus, see SIPStatusFrom.
+func SIPTransferErrorFrom(err error) *SIPTransferError {
+	// Local error tree is cheaper than the protobuf roundtrip below, so check this first.
+	if e, ok := errors.AsType[*SIPTransferError](err); ok {
+		return e
+	}
 	st, ok := status.FromError(err)
 	if !ok {
 		return nil
 	}
 	for _, d := range st.Details() {
-		if e, ok := d.(*SIPStatus); ok {
+		if e, ok := d.(*SIPTransferError); ok {
 			return e
 		}
 	}
 	return nil
 }
 
-func (p SIPStatusCode) ShortName() string {
-	return strings.TrimPrefix(p.String(), "SIP_STATUS_")
+func (p *SIPTransferError) Error() string {
+	if p.SipStatus != nil {
+		return fmt.Sprintf("sip transfer failed: %s: %s", p.Reason, p.SipStatus.Error())
+	}
+	return fmt.Sprintf("sip transfer failed: %s", p.Reason)
+}
+
+// Unwrap returns the SIP status the transfer was rejected with, if one was
+// reported, so errors.As and errors.AsType reach it through the transfer error.
+func (p *SIPTransferError) Unwrap() error {
+	if p.SipStatus == nil {
+		return nil
+	}
+	return p.SipStatus
+}
+
+// GRPCStatus takes the code from the SIP status when the transfer was rejected
+// with one. The other reasons have no SIP response to map, and the code for
+// those is set by the caller that builds the error, so they report Unknown here.
+func (p *SIPTransferError) GRPCStatus() *status.Status {
+	code := codes.Unknown
+	if p.SipStatus != nil {
+		// Only extracting the code here and rebuilding the status below
+		// to avoid dropping the transfer reason in the details.
+		code = p.SipStatus.GRPCStatus().Code()
+	}
+	st := status.New(code, p.Error())
+	if st2, err := st.WithDetails(p); err == nil {
+		return st2
+	}
+	return st
+}
+
+func (p *SIPTransferError) TwirpErrorMeta() map[string]string {
+	m := map[string]string{
+		"sip_transfer_reason": p.Reason.String(),
+	}
+	if p.TransferId != "" {
+		m["sip_transfer_id"] = p.TransferId
+	}
+	// Report the SIP status under the same keys a dialing failure uses, so a
+	// client reads one set of keys regardless of which call failed.
+	if p.SipStatus != nil {
+		for k, v := range p.SipStatus.TwirpErrorMeta() {
+			m[k] = v
+		}
+	}
+	return m
+}
+
+// SIPStatusFrom unwraps an error and returns associated SIP call status, if any.
+func SIPStatusFrom(err error) *SIPStatus {
+	// Local error tree is cheaper than the protobuf roundtrip below, so check this first.
+	if e, ok := errors.AsType[*SIPStatus](err); ok {
+		return e
+	}
+	st, ok := status.FromError(err)
+	if !ok {
+		return nil
+	}
+	for _, d := range st.Details() {
+		switch e := d.(type) {
+		case *SIPStatus:
+			return e
+		case *SIPTransferError:
+			// A failed transfer reports its SIP status inside its own details.
+			if e.SipStatus != nil {
+				return e.SipStatus
+			}
+		}
+	}
+	return nil
 }
 
 func (p *SIPStatus) Error() string {
@@ -53,6 +131,48 @@ func (p *SIPStatus) Error() string {
 	return fmt.Sprintf("sip status: %d (%s)", p.Code, p.Code.ShortName())
 }
 
+func (p *SIPStatus) GRPCStatus() *status.Status {
+	code, ok := sipCodeToGRPCCode[p.Code]
+	if !ok {
+		code = codes.Unknown // 1xx and 2xx codes should never emit an error, something is wrong.
+		if p.Code < 200 {
+			code = codes.Unknown // 1xx are not final responses, something is wrong.
+		} else if p.Code < 300 {
+			return status.New(codes.OK, "OK") // Preserving previous behavior
+		} else if p.Code < 500 {
+			code = codes.InvalidArgument
+		} else if p.Code < 600 {
+			code = codes.FailedPrecondition // 5xx from remote server, per guideline (c) in gRPC docs
+		} else if p.Code < 700 {
+			code = codes.InvalidArgument // Same as 4xx, but authoritative
+		}
+	}
+	msg := p.Status
+	if msg == "" {
+		msg = p.Code.ShortName()
+	}
+	st := status.New(code, fmt.Sprintf("sip status %d: %s", p.Code, msg))
+	if st2, err := st.WithDetails(p); err == nil {
+		return st2
+	}
+	return st
+}
+
+func (p *SIPStatus) TwirpErrorMeta() map[string]string {
+	status := p.Status
+	if status == "" {
+		status = p.Code.String()
+	}
+	return map[string]string{
+		"sip_status_code": strconv.Itoa(int(p.Code)),
+		"sip_status":      status,
+	}
+}
+
+func (p SIPStatusCode) ShortName() string {
+	return strings.TrimPrefix(p.String(), "SIP_STATUS_")
+}
+
 // Maps SIP response codes received from remote SIP servers to GRPC error codes.
 var sipCodeToGRPCCode = map[SIPStatusCode]codes.Code{
 	// 3xx - Redirection Responses
@@ -130,44 +250,6 @@ var sipCodeToGRPCCode = map[SIPStatusCode]codes.Code{
 	SIPStatusCode_SIP_STATUS_GLOBAL_REJECTED:                codes.PermissionDenied,
 }
 
-func (p *SIPStatus) GRPCStatus() *status.Status {
-	code, ok := sipCodeToGRPCCode[p.Code]
-	if !ok {
-		code = codes.Unknown // 1xx and 2xx codes should never emit an error, something is wrong.
-		if p.Code < 200 {
-			code = codes.Unknown // 1xx are not final responses, something is wrong.
-		} else if p.Code < 300 {
-			return status.New(codes.OK, "OK") // Preserving previous behavior
-		} else if p.Code < 500 {
-			code = codes.InvalidArgument
-		} else if p.Code < 600 {
-			code = codes.FailedPrecondition // 5xx from remote server, per guideline (c) in gRPC docs
-		} else if p.Code < 700 {
-			code = codes.InvalidArgument // Same as 4xx ,but authoritative
-		}
-	}
-	msg := p.Status
-	if msg == "" {
-		msg = p.Code.ShortName()
-	}
-	st := status.New(code, fmt.Sprintf("sip status %d: %s", p.Code, msg))
-	if st2, err := st.WithDetails(p); err == nil {
-		return st2
-	}
-	return st
-}
-
-func (p *SIPStatus) TwirpErrorMeta() map[string]string {
-	status := p.Status
-	if status == "" {
-		status = p.Code.String()
-	}
-	return map[string]string{
-		"sip_status_code": strconv.Itoa(int(p.Code)),
-		"sip_status":      status,
-	}
-}
-
 // Name returns a lower-case short name for the transport.
 // It returns an empty string if transport is not specified.
 func (p SIPTransport) Name() string {

livekit/sip_test.go

@@ -14,6 +14,8 @@ import (
 	"google.golang.org/protobuf/types/known/durationpb"
 
 	"github.com/livekit/protocol/utils/prototest"
+	"github.com/livekit/protocol/utils/xtwirp"
+	"github.com/livekit/psrpc"
 )
 
 func TestSIPTrunkAs(t *testing.T) {
@@ -1539,3 +1541,67 @@ func TestValidateHeaders(t *testing.T) {
 		})
 	}
 }
+
+func TestSIPTransferErrorFrom(t *testing.T) {
+	sipStatus := &SIPStatus{
+		Code:   SIPStatusCode_SIP_STATUS_BUSY_HERE,
+		Status: "Busy Here",
+	}
+	transferErr := &SIPTransferError{
+		TransferId: "STR_test",
+		Reason:     SIPTransferReason_STR_REJECTED,
+		SipStatus:  sipStatus,
+	}
+	// One detail carries the whole outcome, SIP status included.
+	err := psrpc.NewError(psrpc.UpstreamClientError, errors.New("call transfer failed"), transferErr)
+
+	require.True(t, proto.Equal(transferErr, SIPTransferErrorFrom(err)))
+	// SIPStatusFrom still finds the status nested inside it.
+	require.True(t, proto.Equal(sipStatus, SIPStatusFrom(err)))
+
+	// The same round trip the API boundary performs: psrpc error -> twirp error
+	// with the details in metadata -> status with the details back.
+	twerr := xtwirp.ToError(err)
+	require.Equal(t, "STR_REJECTED", twerr.Meta("sip_transfer_reason"))
+	require.Equal(t, "STR_test", twerr.Meta("sip_transfer_id"))
+	require.Equal(t, "486", twerr.Meta("sip_status_code"))
+
+	st, ok := xtwirp.StatusFromError(twerr)
+	require.True(t, ok)
+	require.True(t, proto.Equal(transferErr, SIPTransferErrorFrom(st.Err())))
+	require.True(t, proto.Equal(sipStatus, SIPStatusFrom(st.Err())))
+}
+
+func TestSIPTransferErrorFromNotFound(t *testing.T) {
+	require.Nil(t, SIPTransferErrorFrom(errors.New("plain")))
+	require.Nil(t, SIPTransferErrorFrom(psrpc.NewErrorf(psrpc.Internal, "no details")))
+}
+
+func TestSIPTransferErrorAsError(t *testing.T) {
+	sipStatus := &SIPStatus{Code: SIPStatusCode_SIP_STATUS_BUSY_HERE, Status: "Busy Here"}
+	rejected := &SIPTransferError{
+		TransferId: "STR_test",
+		Reason:     SIPTransferReason_STR_REJECTED,
+		SipStatus:  sipStatus,
+	}
+	timedOut := &SIPTransferError{
+		TransferId: "STR_test",
+		Reason:     SIPTransferReason_STR_RINGING_TIMEOUT,
+	}
+
+	require.EqualError(t, rejected, "sip transfer failed: STR_REJECTED: sip status: 486: Busy Here")
+	require.EqualError(t, timedOut, "sip transfer failed: STR_RINGING_TIMEOUT")
+
+	// Unwrap exposes the SIP status to the errors package.
+	require.Equal(t, sipStatus, errors.Unwrap(rejected))
+	require.NoError(t, errors.Unwrap(timedOut))
+
+	// GRPCStatus takes the code from the SIP status and keeps the reason.
+	st := rejected.GRPCStatus()
+	require.Equal(t, sipStatus.GRPCStatus().Code(), st.Code())
+	require.True(t, proto.Equal(rejected, SIPTransferErrorFrom(st.Err())))
+
+	st2 := timedOut.GRPCStatus()
+	require.Equal(t, codes.Unknown, st2.Code())
+	require.True(t, proto.Equal(timedOut, SIPTransferErrorFrom(st2.Err())))
+}

protobufs/livekit_sip.proto

@@ -874,6 +874,9 @@ message TransferSIPParticipantRequest {
   google.protobuf.Duration ringing_timeout = 6;
 }
 
+// Added in https://github.com/livekit/protocol/pull/1730. Not needed after all:
+// will be rolled back to an empty message once STR_CALL_ENDED is reported as an
+// error like every other failed transfer.
 message TransferSIPParticipantResponse {
   string transfer_id = 1 [(logger.name) = "transferID"];
   SIPTransferStatus status = 2;
@@ -885,6 +888,17 @@ message TransferSIPParticipantResponse {
   // NEXT ID: 5
 }
 
+// Details of a failed SIP transfer, attached to the error.
+message SIPTransferError {
+  string transfer_id = 1 [(logger.name) = "transferID"];
+  SIPTransferReason reason = 2;
+
+  // Set when the outcome was reported by a SIP response.
+  SIPStatus sip_status = 3;
+
+  // NEXT ID: 4
+}
+
 message SIPCallInfo {
   string call_id = 1 [(logger.name) = "callID"];
   string trunk_id = 2 [(logger.name) = "trunkID"];

protobufs/rpc/sip.proto

@@ -183,6 +183,9 @@ message InternalTransferSIPParticipantRequest {
   map<string, string> feature_flags = 6;
 }
 
+// Added in https://github.com/livekit/protocol/pull/1730. Not needed after all:
+// will be rolled back to an empty message once STR_CALL_ENDED is reported as an
+// error like every other failed transfer.
 message InternalTransferSIPParticipantResponse {
   string transfer_id = 1 [(logger.name) = "transferID"];
   livekit.SIPTransferStatus status = 2;