Unresolveable media should create channel log error

channel_log.go

@@ -2,7 +2,6 @@ package courier
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/nyaruka/gocommon/dates"
@@ -56,18 +55,15 @@ func ErrorResponseValueMissing(key string) *ChannelError {
 	return NewChannelError("response_value_missing", "", "Unable to find '%s' response.", key)
 }
 
-func ErrorResponseValueUnexpected(key string, expected ...string) *ChannelError {
-	es := make([]string, len(expected))
-	for i := range expected {
-		es[i] = fmt.Sprintf("'%s'", expected[i])
-	}
-	return NewChannelError("response_value_unexpected", "", "Expected '%s' in response to be %s.", key, strings.Join(es, " or "))
-}
-
 func ErrorMediaUnsupported(contentType string) *ChannelError {
 	return NewChannelError("media_unsupported", "", "Unsupported attachment media type: %s.", contentType)
 }
 
+// ErrorMediaUnresolveable is used when media is unresolveable due to the channel's specific requirements
+func ErrorMediaUnresolveable(contentType string) *ChannelError {
+	return NewChannelError("media_unresolveable", "", "Unable to find version of %s attachment compatible with channel.", contentType)
+}
+
 func ErrorAttachmentNotDecodable() *ChannelError {
 	return NewChannelError("attachment_not_decodable", "", "Unable to decode embedded attachment data.")
 }

channel_log_test.go

@@ -108,21 +108,16 @@ func TestChannelErrors(t *testing.T) {
 			expectedCode:    "response_value_missing",
 			expectedMessage: "Unable to find 'id' response.",
 		},
-		{
-			err:             courier.ErrorResponseValueUnexpected("status", "SUCCESS"),
-			expectedCode:    "response_value_unexpected",
-			expectedMessage: "Expected 'status' in response to be 'SUCCESS'.",
-		},
-		{
-			err:             courier.ErrorResponseValueUnexpected("status", "SUCCESS", "OK"),
-			expectedCode:    "response_value_unexpected",
-			expectedMessage: "Expected 'status' in response to be 'SUCCESS' or 'OK'.",
-		},
 		{
 			err:             courier.ErrorMediaUnsupported("image/tiff"),
 			expectedCode:    "media_unsupported",
 			expectedMessage: "Unsupported attachment media type: image/tiff.",
 		},
+		{
+			err:             courier.ErrorMediaUnresolveable("image/jpeg"),
+			expectedCode:    "media_unresolveable",
+			expectedMessage: "Unable to find version of image/jpeg attachment compatible with channel.",
+		},
 		{
 			err:             courier.ErrorAttachmentNotDecodable(),
 			expectedCode:    "attachment_not_decodable",

handlers/line/handler.go

@@ -293,7 +293,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 	parts := handlers.SplitMsgByChannel(msg.Channel(), msg.Text(), maxMsgLength)
 	qrs := msg.QuickReplies()
 
-	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, false)
+	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, false, clog)
 	if err != nil {
 		return errors.Wrap(err, "error resolving attachments")
 	}

handlers/media.go

@@ -35,30 +35,32 @@ type Attachment struct {
 }
 
 // ResolveAttachments resolves the given attachment strings (content-type:url) into attachment objects
-func ResolveAttachments(ctx context.Context, b courier.Backend, attachments []string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) ([]*Attachment, error) {
+func ResolveAttachments(ctx context.Context, b courier.Backend, attachments []string, support map[MediaType]MediaTypeSupport, allowURLOnly bool, clog *courier.ChannelLog) ([]*Attachment, error) {
 	resolved := make([]*Attachment, 0, len(attachments))
 
 	for _, as := range attachments {
-		att, err := resolveAttachment(ctx, b, as, support, allowURLOnly)
+		// split into content-type and URL
+		parts := strings.SplitN(as, ":", 2)
+		if len(parts) <= 1 || strings.HasPrefix(parts[1], "//") {
+			return nil, errors.Errorf("invalid attachment format: %s", as)
+		}
+		contentType, mediaUrl := parts[0], parts[1]
+
+		att, err := resolveAttachment(ctx, b, contentType, mediaUrl, support, allowURLOnly)
 		if err != nil {
 			return nil, err
 		}
 		if att != nil {
 			resolved = append(resolved, att)
+		} else {
+			clog.Error(courier.ErrorMediaUnresolveable(contentType))
 		}
 	}
 
 	return resolved, nil
 }
 
-func resolveAttachment(ctx context.Context, b courier.Backend, attachment string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) (*Attachment, error) {
-	// split into content-type and URL
-	parts := strings.SplitN(attachment, ":", 2)
-	if len(parts) <= 1 || strings.HasPrefix(parts[1], "//") {
-		return nil, errors.Errorf("invalid attachment format: %s", attachment)
-	}
-	contentType, mediaUrl := parts[0], parts[1]
-
+func resolveAttachment(ctx context.Context, b courier.Backend, contentType, mediaUrl string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) (*Attachment, error) {
 	media, err := b.ResolveMedia(ctx, mediaUrl)
 	if err != nil {
 		return nil, err

handlers/media_test.go

@@ -34,6 +34,7 @@ func TestResolveAttachments(t *testing.T) {
 		mediaSupport map[handlers.MediaType]handlers.MediaTypeSupport
 		allowURLOnly bool
 		resolved     []*handlers.Attachment
+		errors       []*courier.ChannelError
 		err          string
 	}{
 		{ // 0: user entered image URL
@@ -57,10 +58,11 @@ func TestResolveAttachments(t *testing.T) {
 			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png"}}}, // ignored
 			allowURLOnly: false,
 			resolved:     []*handlers.Attachment{},
+			errors:       []*courier.ChannelError{courier.ErrorMediaUnresolveable("image")},
 		},
 		{ // 3: resolveable uploaded image URL
 			attachments:  []string{"image/jpeg:http://mock.com/1234/test.jpg"},
-			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/jpeg", "image/png"}}},
+			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png", "image/jpeg"}}},
 			allowURLOnly: true,
 			resolved: []*handlers.Attachment{
 				{Type: handlers.MediaTypeImage, Name: "test.jpg", ContentType: "image/jpeg", URL: "http://mock.com/1234/test.jpg", Media: imageJPG, Thumbnail: nil},
@@ -79,12 +81,14 @@ func TestResolveAttachments(t *testing.T) {
 			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/jpeg", "image/png"}}},
 			allowURLOnly: false,
 			resolved:     []*handlers.Attachment{},
+			errors:       []*courier.ChannelError{courier.ErrorMediaUnresolveable("image/jpeg")},
 		},
 		{ // 6: resolveable uploaded image URL, type not in supported types
 			attachments:  []string{"image/jpeg:http://mock.com/1234/test.jpg"},
 			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png"}}},
 			allowURLOnly: true,
 			resolved:     []*handlers.Attachment{},
+			errors:       []*courier.ChannelError{courier.ErrorMediaUnresolveable("image/jpeg")},
 		},
 		{ // 7: resolveable uploaded audio URL, type in supported types
 			attachments:  []string{"audio/mp3:http://mock.com/3456/test.mp3"},
@@ -123,6 +127,7 @@ func TestResolveAttachments(t *testing.T) {
 			mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeVideo: {Types: []string{"video/quicktime"}, MaxBytes: 10 * 1024 * 1024}},
 			allowURLOnly: true,
 			resolved:     []*handlers.Attachment{},
+			errors:       []*courier.ChannelError{courier.ErrorMediaUnresolveable("video/quicktime")},
 		},
 		{ // 12: invalid attachment format
 			attachments:  []string{"image"},
@@ -137,12 +142,15 @@ func TestResolveAttachments(t *testing.T) {
 	}
 
 	for i, tc := range tcs {
-		resolved, err := handlers.ResolveAttachments(ctx, mb, tc.attachments, tc.mediaSupport, tc.allowURLOnly)
+		clog := courier.NewChannelLog(courier.ChannelLogTypeMsgSend, nil, nil)
+
+		resolved, err := handlers.ResolveAttachments(ctx, mb, tc.attachments, tc.mediaSupport, tc.allowURLOnly, clog)
 		if tc.err != "" {
 			assert.EqualError(t, err, tc.err, "expected error for test %d", i)
 		} else {
 			assert.NoError(t, err, "unexpected error for test %d", i)
-			assert.Equal(t, tc.resolved, resolved, "mismatch for test %d", i)
+			assert.Equal(t, tc.resolved, resolved, "resolved mismatch for test %d", i)
+			assert.Equal(t, tc.errors, clog.Errors(), "errors mismatch for test %d", i)
 		}
 	}
 }

handlers/telegram/handler.go

@@ -188,7 +188,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 		return courier.ErrChannelConfig
 	}
 
-	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true)
+	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true, clog)
 	if err != nil {
 		return errors.Wrap(err, "error resolving attachments")
 	}

handlers/twiml/handlers.go

@@ -225,7 +225,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
 
 	channel := msg.Channel()
 
-	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true)
+	attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true, clog)
 	if err != nil {
 		return err
 	}