From 47eb7b582bc32d67fea93c4c460c94d5df659817 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 19 Nov 2022 09:55:38 +0100 Subject: [PATCH] #81: Fix error handling in body writer The error handling in the msgWriter.writeBody() method was not working properly. We basically overwrote the mw.err with nil if the function that followed after a failed write attempt was successful again This patch fixes #81 --- msg_test.go | 27 ++++++++++++++++++++++++++- msgwriter.go | 31 +++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/msg_test.go b/msg_test.go index 8cb3b6d..284b3a0 100644 --- a/msg_test.go +++ b/msg_test.go @@ -8,6 +8,7 @@ import ( "bufio" "bytes" "embed" + "errors" "fmt" htpl "html/template" "io" @@ -1652,7 +1653,7 @@ func TestMsg_WriteTo(t *testing.T) { } } -// TestMsg_WriteTo tests the WriteTo() method of the Msg +// TestMsg_WriteToSkipMiddleware tests the WriteTo() method of the Msg func TestMsg_WriteToSkipMiddleware(t *testing.T) { m := NewMsg(WithMiddleware(encodeMiddleware{}), WithMiddleware(uppercaseMiddleware{})) m.Subject("This is a test") @@ -1684,6 +1685,30 @@ func TestMsg_WriteToSkipMiddleware(t *testing.T) { } } +// TestMsg_WriteTo_fails tests the WriteTo() method of the Msg but with a failing body writer function +func TestMsg_WriteTo_fails(t *testing.T) { + m := NewMsg() + m.SetBodyWriter(TypeTextPlain, func(io.Writer) (int64, error) { + return 0, errors.New("failed") + }) + _, err := m.WriteTo(io.Discard) + if err == nil { + t.Errorf("WriteTo() with failing BodyWriter function was supposed to fail, but didn't") + return + } + + // NoEncoding handles the errors separately + m = NewMsg(WithEncoding(NoEncoding)) + m.SetBodyWriter(TypeTextPlain, func(io.Writer) (int64, error) { + return 0, errors.New("failed") + }) + _, err = m.WriteTo(io.Discard) + if err == nil { + t.Errorf("WriteTo() (no encoding) with failing BodyWriter function was supposed to fail, but didn't") + return + } +} + // TestMsg_Write tests the Write() method of the Msg func TestMsg_Write(t *testing.T) { m := NewMsg() diff --git a/msgwriter.go b/msgwriter.go index 6b5a4b0..6da1c35 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -282,6 +282,7 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { var w io.Writer var ew io.WriteCloser var n int64 + var err error if mw.d == 0 { w = mw.w } @@ -298,8 +299,14 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { case EncodingB64: ew = base64.NewEncoder(base64.StdEncoding, &lb) case NoEncoding: - _, mw.err = f(&wbuf) - n, mw.err = io.Copy(w, &wbuf) + _, err = f(&wbuf) + if err != nil { + mw.err = fmt.Errorf("bodyWriter function: %w", err) + } + n, err = io.Copy(w, &wbuf) + if err != nil && mw.err == nil { + mw.err = fmt.Errorf("bodyWriter io.Copy: %w", err) + } if mw.d == 0 { mw.n += n } @@ -308,10 +315,22 @@ func (mw *msgWriter) writeBody(f func(io.Writer) (int64, error), e Encoding) { ew = quotedprintable.NewWriter(w) } - _, mw.err = f(ew) - mw.err = ew.Close() - mw.err = lb.Close() - n, mw.err = io.Copy(w, &wbuf) + _, err = f(ew) + if err != nil { + mw.err = fmt.Errorf("bodyWriter function: %w", err) + } + err = ew.Close() + if err != nil && mw.err == nil { + mw.err = fmt.Errorf("bodyWriter close encoded writer: %w", err) + } + err = lb.Close() + if err != nil && mw.err == nil { + mw.err = fmt.Errorf("bodyWriter close linebreaker: %w", err) + } + n, err = io.Copy(w, &wbuf) + if err != nil && mw.err == nil { + mw.err = fmt.Errorf("bodyWriter io.Copy: %w", err) + } // Since the part writer uses the WriteTo() method, we don't need to add the // bytes twice