From c61915cb26962fa7d5ba14b6ba5a9288024247f6 Mon Sep 17 00:00:00 2001 From: Shane Howearth Date: Fri, 12 Jan 2024 16:39:52 +1100 Subject: [PATCH 1/3] Chore: Handle errors by logging them --- .golangci.yml | 3 +-- smtp.go | 28 ++++++++++++++++++++++------ writeto.go | 30 +++++++++++++++++++++++------- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index af4f554..4029d45 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -15,6 +15,7 @@ linters: enable: - depguard - dupl + - errcheck - goconst - gocritic - gofmt @@ -30,8 +31,6 @@ linters: - stylecheck - unconvert - unparam - disable: - - errcheck # TODO linters-settings: dupl: diff --git a/smtp.go b/smtp.go index c56a65b..0fa70ee 100644 --- a/smtp.go +++ b/smtp.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "fmt" "io" + "log/slog" "net" "net/smtp" "strings" @@ -94,7 +95,9 @@ func (d *Dialer) Dial() (SendCloser, error) { } if d.Timeout > 0 { - conn.SetDeadline(time.Now().Add(d.Timeout)) + if tErr := conn.SetDeadline(time.Now().Add(d.Timeout)); tErr != nil { + slog.Error("unable to set connection deadline", "error", tErr.Error()) + } } if d.LocalName != "" { @@ -113,7 +116,9 @@ func (d *Dialer) Dial() (SendCloser, error) { if ok { if err := c.StartTLS(d.tlsConfig()); err != nil { - c.Close() + if cErr := c.Close(); cErr != nil { + slog.Error("unable to close smtpClient after attempting to startTLS", "error", cErr.Error()) + } return nil, err } } @@ -138,7 +143,9 @@ func (d *Dialer) Dial() (SendCloser, error) { if d.Auth != nil { if err = c.Auth(d.Auth); err != nil { - c.Close() + if cErr := c.Close(); cErr != nil { + slog.Error("unable to close smtpClient after attempting to set Auth", "error", cErr.Error()) + } return nil, err } } @@ -205,7 +212,12 @@ func (d *Dialer) DialAndSend(m ...*Message) error { if err != nil { return err } - defer s.Close() + + defer func() { + if sErr := s.Close(); sErr != nil { + slog.Error("unable to close sendCloser after attempting to Close", "error", sErr.Error()) + } + }() return Send(s, m...) } @@ -230,7 +242,9 @@ func (c *smtpSender) retryError(err error) bool { func (c *smtpSender) Send(from string, to []string, msg io.WriterTo) error { if c.d.Timeout > 0 { - c.conn.SetDeadline(time.Now().Add(c.d.Timeout)) + if err := c.conn.SetDeadline(time.Now().Add(c.d.Timeout)); err != nil { + slog.Error("unable to set context deadline in send", "error", err.Error()) + } } if err := c.Mail(from); err != nil { @@ -260,7 +274,9 @@ func (c *smtpSender) Send(from string, to []string, msg io.WriterTo) error { } if _, err = msg.WriteTo(w); err != nil { - w.Close() + if wErr := w.Close(); wErr != nil { + slog.Error("unable to close writeCloser after attempting to write message", "error", wErr.Error()) + } return err } diff --git a/writeto.go b/writeto.go index 6b6fb47..fb707c7 100644 --- a/writeto.go +++ b/writeto.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "errors" "io" + "log/slog" "mime" "mime/multipart" "path/filepath" @@ -80,7 +81,9 @@ type messageWriter struct { func (w *messageWriter) openMultipart(mimeType, boundary string) { mw := multipart.NewWriter(w) if boundary != "" { - mw.SetBoundary(boundary) + if err := mw.SetBoundary(boundary); err != nil { + slog.Error("unable to set boundary in openMultipart", "error", err.Error()) + } } contentType := "multipart/" + mimeType + ";\r\n boundary=" + mw.Boundary() w.writers[w.depth] = mw @@ -102,7 +105,9 @@ func (w *messageWriter) createPart(h map[string][]string) { func (w *messageWriter) closeMultipart() { if w.depth > 0 { - w.writers[w.depth-1].Close() + if err := w.writers[w.depth-1].Close(); err != nil { + slog.Error("unable to close writers in closeMultipart", "error", err.Error()) + } w.depth-- } } @@ -270,13 +275,17 @@ func (w *messageWriter) writeBody(f func(io.Writer) error, enc Encoding) { case Base64: wc := base64.NewEncoder(base64.StdEncoding, newBase64LineWriter(subWriter)) w.err = f(wc) - wc.Close() + if err := wc.Close(); err != nil { + slog.Error("unable to close writer for base64 write body", "error", err.Error()) + } case Unencoded: w.err = f(subWriter) default: wc := newQPWriter(subWriter) w.err = f(wc) - wc.Close() + if err := wc.Close(); err != nil { + slog.Error("unable to close newQPWriter", "error", err.Error()) + } } } @@ -297,14 +306,21 @@ func newBase64LineWriter(w io.Writer) *base64LineWriter { func (w *base64LineWriter) Write(p []byte) (int, error) { n := 0 for len(p)+w.lineLen > maxLineLen { - w.w.Write(p[:maxLineLen-w.lineLen]) - w.w.Write([]byte("\r\n")) + if _, err := w.w.Write(p[:maxLineLen-w.lineLen]); err != nil { + slog.Error("unable to write line", "error", err.Error()) + } + + if _, err := w.w.Write([]byte("\r\n")); err != nil { + slog.Error("unable to write newline characters", "error", err.Error()) + } p = p[maxLineLen-w.lineLen:] n += maxLineLen - w.lineLen w.lineLen = 0 } - w.w.Write(p) + if _, err := w.w.Write(p); err != nil { + slog.Error("unable to write final line", "error", err.Error()) + } w.lineLen += len(p) return n + len(p), nil From 4062be396587dabba4196857f6e15c4fddce53b6 Mon Sep 17 00:00:00 2001 From: Shane Howearth Date: Fri, 12 Jan 2024 16:40:25 +1100 Subject: [PATCH 2/3] Chore: Replace empty comment line with empty line --- doc.go | 1 - 1 file changed, 1 deletion(-) diff --git a/doc.go b/doc.go index 538c12d..1a692ba 100644 --- a/doc.go +++ b/doc.go @@ -2,5 +2,4 @@ // efficiently. // // More info on Github: https://github.com/go-mail/mail -// package gomail From cf34a4208f06e88a1ae7263a09b19201ffc45649 Mon Sep 17 00:00:00 2001 From: Shane Howearth Date: Fri, 12 Jan 2024 16:41:18 +1100 Subject: [PATCH 3/3] Chore: Use non-reserved word for variable name --- message_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/message_test.go b/message_test.go index efd7990..019944e 100644 --- a/message_test.go +++ b/message_test.go @@ -351,9 +351,9 @@ func TestRename(t *testing.T) { m.SetHeader("From", "from@example.com") m.SetHeader("To", "to@example.com") m.SetBody("text/plain", "Test") - name, copy := mockCopyFile("/tmp/test.pdf") + name, copyFile := mockCopyFile("/tmp/test.pdf") rename := Rename("another.pdf") - m.Attach(name, copy, rename) + m.Attach(name, copyFile, rename) want := &message{ from: "from@example.com",