Skip to content

Commit f889b58

Browse files
rowanseymourrowanseymour
authored
Merge pull request #712 from nyaruka/clog_unresolved_attachments
Unresolveable media should create channel log error
2 parents 6019d3d + 21ccce4 commit f889b58

File tree

7 files changed

+36
-35
lines changed

7 files changed

+36
-35
lines changed

Diff for: ‎channel_log.go

+5-9
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package courier
22

33
import (
44
"fmt"
5-
"strings"
65
"time"
76

87
"github.com/nyaruka/gocommon/dates"
@@ -56,18 +55,15 @@ func ErrorResponseValueMissing(key string) *ChannelError {
5655
return NewChannelError("response_value_missing", "", "Unable to find '%s' response.", key)
5756
}
5857

59-
func ErrorResponseValueUnexpected(key string, expected ...string) *ChannelError {
60-
es := make([]string, len(expected))
61-
for i := range expected {
62-
es[i] = fmt.Sprintf("'%s'", expected[i])
63-
}
64-
return NewChannelError("response_value_unexpected", "", "Expected '%s' in response to be %s.", key, strings.Join(es, " or "))
65-
}
66-
6758
func ErrorMediaUnsupported(contentType string) *ChannelError {
6859
return NewChannelError("media_unsupported", "", "Unsupported attachment media type: %s.", contentType)
6960
}
7061

62+
// ErrorMediaUnresolveable is used when media is unresolveable due to the channel's specific requirements
63+
func ErrorMediaUnresolveable(contentType string) *ChannelError {
64+
return NewChannelError("media_unresolveable", "", "Unable to find version of %s attachment compatible with channel.", contentType)
65+
}
66+
7167
func ErrorAttachmentNotDecodable() *ChannelError {
7268
return NewChannelError("attachment_not_decodable", "", "Unable to decode embedded attachment data.")
7369
}

Diff for: ‎channel_log_test.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -108,21 +108,16 @@ func TestChannelErrors(t *testing.T) {
108108
expectedCode: "response_value_missing",
109109
expectedMessage: "Unable to find 'id' response.",
110110
},
111-
{
112-
err: courier.ErrorResponseValueUnexpected("status", "SUCCESS"),
113-
expectedCode: "response_value_unexpected",
114-
expectedMessage: "Expected 'status' in response to be 'SUCCESS'.",
115-
},
116-
{
117-
err: courier.ErrorResponseValueUnexpected("status", "SUCCESS", "OK"),
118-
expectedCode: "response_value_unexpected",
119-
expectedMessage: "Expected 'status' in response to be 'SUCCESS' or 'OK'.",
120-
},
121111
{
122112
err: courier.ErrorMediaUnsupported("image/tiff"),
123113
expectedCode: "media_unsupported",
124114
expectedMessage: "Unsupported attachment media type: image/tiff.",
125115
},
116+
{
117+
err: courier.ErrorMediaUnresolveable("image/jpeg"),
118+
expectedCode: "media_unresolveable",
119+
expectedMessage: "Unable to find version of image/jpeg attachment compatible with channel.",
120+
},
126121
{
127122
err: courier.ErrorAttachmentNotDecodable(),
128123
expectedCode: "attachment_not_decodable",

Diff for: ‎handlers/line/handler.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, res *courier.Sen
293293
parts := handlers.SplitMsgByChannel(msg.Channel(), msg.Text(), maxMsgLength)
294294
qrs := msg.QuickReplies()
295295

296-
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, false)
296+
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, false, clog)
297297
if err != nil {
298298
return errors.Wrap(err, "error resolving attachments")
299299
}

Diff for: ‎handlers/media.go

+12-10
Original file line numberDiff line numberDiff line change
@@ -35,30 +35,32 @@ type Attachment struct {
3535
}
3636

3737
// ResolveAttachments resolves the given attachment strings (content-type:url) into attachment objects
38-
func ResolveAttachments(ctx context.Context, b courier.Backend, attachments []string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) ([]*Attachment, error) {
38+
func ResolveAttachments(ctx context.Context, b courier.Backend, attachments []string, support map[MediaType]MediaTypeSupport, allowURLOnly bool, clog *courier.ChannelLog) ([]*Attachment, error) {
3939
resolved := make([]*Attachment, 0, len(attachments))
4040

4141
for _, as := range attachments {
42-
att, err := resolveAttachment(ctx, b, as, support, allowURLOnly)
42+
// split into content-type and URL
43+
parts := strings.SplitN(as, ":", 2)
44+
if len(parts) <= 1 || strings.HasPrefix(parts[1], "//") {
45+
return nil, errors.Errorf("invalid attachment format: %s", as)
46+
}
47+
contentType, mediaUrl := parts[0], parts[1]
48+
49+
att, err := resolveAttachment(ctx, b, contentType, mediaUrl, support, allowURLOnly)
4350
if err != nil {
4451
return nil, err
4552
}
4653
if att != nil {
4754
resolved = append(resolved, att)
55+
} else {
56+
clog.Error(courier.ErrorMediaUnresolveable(contentType))
4857
}
4958
}
5059

5160
return resolved, nil
5261
}
5362

54-
func resolveAttachment(ctx context.Context, b courier.Backend, attachment string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) (*Attachment, error) {
55-
// split into content-type and URL
56-
parts := strings.SplitN(attachment, ":", 2)
57-
if len(parts) <= 1 || strings.HasPrefix(parts[1], "//") {
58-
return nil, errors.Errorf("invalid attachment format: %s", attachment)
59-
}
60-
contentType, mediaUrl := parts[0], parts[1]
61-
63+
func resolveAttachment(ctx context.Context, b courier.Backend, contentType, mediaUrl string, support map[MediaType]MediaTypeSupport, allowURLOnly bool) (*Attachment, error) {
6264
media, err := b.ResolveMedia(ctx, mediaUrl)
6365
if err != nil {
6466
return nil, err

Diff for: ‎handlers/media_test.go

+11-3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ func TestResolveAttachments(t *testing.T) {
3434
mediaSupport map[handlers.MediaType]handlers.MediaTypeSupport
3535
allowURLOnly bool
3636
resolved []*handlers.Attachment
37+
errors []*courier.ChannelError
3738
err string
3839
}{
3940
{ // 0: user entered image URL
@@ -57,10 +58,11 @@ func TestResolveAttachments(t *testing.T) {
5758
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png"}}}, // ignored
5859
allowURLOnly: false,
5960
resolved: []*handlers.Attachment{},
61+
errors: []*courier.ChannelError{courier.ErrorMediaUnresolveable("image")},
6062
},
6163
{ // 3: resolveable uploaded image URL
6264
attachments: []string{"image/jpeg:http://mock.com/1234/test.jpg"},
63-
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/jpeg", "image/png"}}},
65+
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png", "image/jpeg"}}},
6466
allowURLOnly: true,
6567
resolved: []*handlers.Attachment{
6668
{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) {
7981
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/jpeg", "image/png"}}},
8082
allowURLOnly: false,
8183
resolved: []*handlers.Attachment{},
84+
errors: []*courier.ChannelError{courier.ErrorMediaUnresolveable("image/jpeg")},
8285
},
8386
{ // 6: resolveable uploaded image URL, type not in supported types
8487
attachments: []string{"image/jpeg:http://mock.com/1234/test.jpg"},
8588
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeImage: {Types: []string{"image/png"}}},
8689
allowURLOnly: true,
8790
resolved: []*handlers.Attachment{},
91+
errors: []*courier.ChannelError{courier.ErrorMediaUnresolveable("image/jpeg")},
8892
},
8993
{ // 7: resolveable uploaded audio URL, type in supported types
9094
attachments: []string{"audio/mp3:http://mock.com/3456/test.mp3"},
@@ -123,6 +127,7 @@ func TestResolveAttachments(t *testing.T) {
123127
mediaSupport: map[handlers.MediaType]handlers.MediaTypeSupport{handlers.MediaTypeVideo: {Types: []string{"video/quicktime"}, MaxBytes: 10 * 1024 * 1024}},
124128
allowURLOnly: true,
125129
resolved: []*handlers.Attachment{},
130+
errors: []*courier.ChannelError{courier.ErrorMediaUnresolveable("video/quicktime")},
126131
},
127132
{ // 12: invalid attachment format
128133
attachments: []string{"image"},
@@ -137,12 +142,15 @@ func TestResolveAttachments(t *testing.T) {
137142
}
138143

139144
for i, tc := range tcs {
140-
resolved, err := handlers.ResolveAttachments(ctx, mb, tc.attachments, tc.mediaSupport, tc.allowURLOnly)
145+
clog := courier.NewChannelLog(courier.ChannelLogTypeMsgSend, nil, nil)
146+
147+
resolved, err := handlers.ResolveAttachments(ctx, mb, tc.attachments, tc.mediaSupport, tc.allowURLOnly, clog)
141148
if tc.err != "" {
142149
assert.EqualError(t, err, tc.err, "expected error for test %d", i)
143150
} else {
144151
assert.NoError(t, err, "unexpected error for test %d", i)
145-
assert.Equal(t, tc.resolved, resolved, "mismatch for test %d", i)
152+
assert.Equal(t, tc.resolved, resolved, "resolved mismatch for test %d", i)
153+
assert.Equal(t, tc.errors, clog.Errors(), "errors mismatch for test %d", i)
146154
}
147155
}
148156
}

Diff for: ‎handlers/telegram/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
return courier.ErrChannelConfig
189189
}
190190

191-
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true)
191+
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true, clog)
192192
if err != nil {
193193
return errors.Wrap(err, "error resolving attachments")
194194
}

Diff for: ‎handlers/twiml/handlers.go

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

226226
channel := msg.Channel()
227227

228-
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true)
228+
attachments, err := handlers.ResolveAttachments(ctx, h.Backend(), msg.Attachments(), mediaSupport, true, clog)
229229
if err != nil {
230230
return err
231231
}

0 commit comments

Comments
 (0)