diff --git a/commo.go b/commo.go index 19f9554..c094f0d 100644 --- a/commo.go +++ b/commo.go @@ -34,6 +34,7 @@ package commo import ( "context" "errors" + "fmt" "html/template" "github.com/jordan-wright/email" @@ -166,12 +167,21 @@ func sendSendGrid(e *Email) (err error) { } if rep.StatusCode < 200 || rep.StatusCode >= 300 { - return errors.New(rep.Body) + return sendGridResponseError(rep.StatusCode, rep.Body) } return nil } +// sendGridResponseError returns a non-retryable error for SendGrid 4xx responses. +func sendGridResponseError(statusCode int, body string) error { + err := fmt.Errorf("sendgrid: status %d: %s", statusCode, body) + if statusCode >= 400 && statusCode < 500 { + return backoff.NoRetry(err) + } + return err +} + func sendMock(*Email) (err error) { return errors.New("not implemented") } diff --git a/email.go b/email.go index 30c449c..5098428 100644 --- a/email.go +++ b/email.go @@ -80,6 +80,10 @@ func (e *Email) ToSMTP() (msg *email.Email, err error) { return nil, err } + if len(msg.Text) == 0 || len(msg.HTML) == 0 { + return nil, ErrEmptyContent + } + return msg, nil } @@ -107,6 +111,10 @@ func (e *Email) ToSendGrid() (msg *sgmail.SGMailV3, err error) { return nil, err } + if text == "" || html == "" { + return nil, ErrEmptyContent + } + msg.AddContent( sgmail.NewContent("text/plain", text), sgmail.NewContent("text/html", html), diff --git a/email_test.go b/email_test.go index 6d1b1e5..f95cd59 100644 --- a/email_test.go +++ b/email_test.go @@ -1,6 +1,7 @@ package commo_test import ( + "html/template" "testing" "github.com/stretchr/testify/require" @@ -100,3 +101,21 @@ func TestEmailValidate(t *testing.T) { }) } + +// Ensures that an error is returned when the email content is empty. +func TestToSendGridEmptyContent(t *testing.T) { + commo.WithTemplates(map[string]*template.Template{ + "empty.txt": template.Must(template.New("empty.txt").Parse("")), + "empty.html": template.Must(template.New("empty.html").Parse("")), + }) + + email := &commo.Email{ + Sender: "admin@server.com", + To: []string{"test@example.com"}, + Subject: "subject", + Template: "empty", + } + + _, err := email.ToSendGrid() + require.ErrorIs(t, err, commo.ErrEmptyContent) +} diff --git a/errors.go b/errors.go index 720da23..b73378b 100644 --- a/errors.go +++ b/errors.go @@ -10,6 +10,7 @@ var ( ErrMissingTemplate = errors.New("missing email template name") ErrNotInitialized = errors.New("email sending method has not been configured") ErrTemplatesNotLoaded = errors.New("templates have not been loaded yet") + ErrEmptyContent = errors.New("email must have non-empty text and html content") ) var ( diff --git a/export_test.go b/export_test.go new file mode 100644 index 0000000..510839c --- /dev/null +++ b/export_test.go @@ -0,0 +1,4 @@ +package commo + +// SendGridResponseError exposes sendGridResponseError for commo_test. +var SendGridResponseError = sendGridResponseError diff --git a/render.go b/render.go index 4c086e1..218c0e6 100644 --- a/render.go +++ b/render.go @@ -29,7 +29,7 @@ func RenderString(name string, data any) (text, html string, err error) { ) if tb, hb, err = Render(name, data); err != nil { - return "", "", nil + return "", "", err } return string(tb), string(hb), nil diff --git a/render_test.go b/render_test.go index 7d83b1b..a9cc74f 100644 --- a/render_test.go +++ b/render_test.go @@ -1,6 +1,7 @@ package commo_test import ( + "html/template" "path/filepath" "strings" "testing" @@ -25,6 +26,18 @@ func TestRenderUnknown(t *testing.T) { require.EqualError(t, err, "could not find \"foo.txt\" in templates", "expected unknown template") } +// Ensures that an error is returned when the template is not found. +func TestRenderStringPropagatesError(t *testing.T) { + templates := map[string]*template.Template{ + "broken.txt": template.Must(template.New("broken.txt").Parse(`{{call .Fail}}`)), + "broken.html": template.Must(template.New("broken.html").Parse(`{{call .Fail}}`)), + } + commo.WithTemplates(templates) + + _, _, err := commo.RenderString("broken", nil) + require.Error(t, err) +} + func allEmailTemplates(t *testing.T) []string { paths := make(map[string]struct{}) ls, err := filepath.Glob("templates/*.*") diff --git a/sendgrid_test.go b/sendgrid_test.go index acec51f..53f6ca2 100644 --- a/sendgrid_test.go +++ b/sendgrid_test.go @@ -1,11 +1,14 @@ package commo_test import ( + "context" + "errors" "testing" sgmail "github.com/sendgrid/sendgrid-go/helpers/mail" "github.com/stretchr/testify/require" "go.rtnl.ai/commo" + "go.rtnl.ai/x/backoff" ) func TestNewSGEmail(t *testing.T) { @@ -103,3 +106,37 @@ func TestNewSGEmails(t *testing.T) { } }) } + +// Ensures that Sendgrid errors are handled correctly. +func TestSendGridResponseError(t *testing.T) { + t.Run("4xx is not retried", func(t *testing.T) { + err := commo.SendGridResponseError(400, `{"errors":[{"message":"bad"}]}`) + var stop *backoff.NoRetryError + require.ErrorAs(t, err, &stop) + + attempts := 0 + _, err = backoff.Retry(context.Background(), func() (any, error) { + attempts++ + return nil, commo.SendGridResponseError(400, "bad request") + }) + require.Error(t, err) + require.Equal(t, 1, attempts) + }) + + t.Run("5xx is retried", func(t *testing.T) { + err := commo.SendGridResponseError(503, "unavailable") + var stop *backoff.NoRetryError + require.False(t, errors.As(err, &stop)) + + attempts := 0 + _, err = backoff.Retry(context.Background(), func() (any, error) { + attempts++ + if attempts < 3 { + return nil, commo.SendGridResponseError(503, "unavailable") + } + return nil, nil + }, backoff.WithMaxTries(3)) + require.NoError(t, err) + require.Equal(t, 3, attempts) + }) +}