From f8f2becb4b58c649de043db4e22f9638b630ffd4 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Fri, 3 Jul 2026 20:20:23 +0100 Subject: [PATCH] Fix dropped notification errors and switch to errors.Join notify/email.go accumulated multi-recipient errors with multierror.Append(fmt.Errorf(...)) instead of multierror.Append(result, ...), so the accumulator was overwritten each iteration and only the last failing recipient's error survived; earlier failures were silently dropped. The telegram notifier did it correctly. Replace hashicorp/go-multierror with the stdlib errors.Join everywhere it was used (notify/email.go, notify/telegram.go, rest/api/rest_private.go, store/service/service.go, store/image/image.go and store/engine/bolt.go), which fixes the bug and drops the direct dependency. It stays indirect because go-pkgz/lcw/v2 still imports it. A regression test in email_test.go now sends two failing recipients and asserts both errors are reported. --- backend/_example/memory_store/go.mod | 2 -- backend/_example/memory_store/go.sum | 5 ----- backend/app/notify/email.go | 10 +++++----- backend/app/notify/email_test.go | 14 +++++++++---- backend/app/notify/telegram.go | 10 +++++----- backend/app/notify/telegram_test.go | 1 - backend/app/rest/api/rest_private.go | 15 ++++++-------- backend/app/store/engine/bolt.go | 7 +++---- backend/app/store/image/fs_store.go | 4 ++-- backend/app/store/image/image.go | 8 ++++---- backend/app/store/service/service.go | 30 ++++++++++++++-------------- backend/go.mod | 2 +- 12 files changed, 51 insertions(+), 57 deletions(-) diff --git a/backend/_example/memory_store/go.mod b/backend/_example/memory_store/go.mod index acb90065..c6a9ee44 100644 --- a/backend/_example/memory_store/go.mod +++ b/backend/_example/memory_store/go.mod @@ -21,8 +21,6 @@ require ( github.com/go-pkgz/rest v1.22.0 // indirect github.com/go-pkgz/routegroup v1.6.0 // indirect github.com/gorilla/css v1.0.1 // indirect - github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/kr/text v0.2.0 // indirect github.com/microcosm-cc/bluemonday v1.0.27 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/backend/_example/memory_store/go.sum b/backend/_example/memory_store/go.sum index ebebc0cc..ea3ea918 100644 --- a/backend/_example/memory_store/go.sum +++ b/backend/_example/memory_store/go.sum @@ -27,11 +27,6 @@ github.com/go-pkgz/routegroup v1.6.0 h1:44XHZgF6JIIldRlv+zjg6SygULASmjifnfIQjwCT github.com/go-pkgz/routegroup v1.6.0/go.mod h1:Pmu04fhgWhRtBMIJ8HXppnnzOPjnL/IEPBIdO2zmeqg= github.com/gorilla/css v1.0.1 h1:ntNaBIghp6JmvWnxbZKANoLyuXTPZ4cAMlo6RyhlbO8= github.com/gorilla/css v1.0.1/go.mod h1:BvnYkspnSzMmwRK+b8/xgNPLiIuNZr6vbZBTPQ2A3b0= -github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= -github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= -github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/jessevdk/go-flags v1.6.1 h1:Cvu5U8UGrLay1rZfv/zP7iLpSHGUZ/Ou68T0iX1bBK4= diff --git a/backend/app/notify/email.go b/backend/app/notify/email.go index 1199fd1a..fdb5016a 100644 --- a/backend/app/notify/email.go +++ b/backend/app/notify/email.go @@ -3,6 +3,7 @@ package notify import ( "bytes" "context" + "errors" "fmt" "html/template" "net/url" @@ -11,7 +12,6 @@ import ( log "github.com/go-pkgz/lgr" ntf "github.com/go-pkgz/notify" "github.com/go-pkgz/repeater/v2" - "github.com/hashicorp/go-multierror" "github.com/microcosm-cc/bluemonday" "github.com/umputun/remark42/backend/app/templates" @@ -160,23 +160,23 @@ func (e *Email) Send(ctx context.Context, req Request) error { default: } - result := new(multierror.Error) + var errs []error for _, email := range req.Emails { err := e.buildAndSendMessage(ctx, req, email, false) if err != nil { - result = multierror.Append(fmt.Errorf("problem sending user email notification to %q: %w", email, err)) + errs = append(errs, fmt.Errorf("problem sending user email notification to %q: %w", email, err)) } } for _, email := range e.AdminEmails { err := e.buildAndSendMessage(ctx, req, email, true) if err != nil { - result = multierror.Append(fmt.Errorf("problem sending admin email notification to %q: %w", email, err)) + errs = append(errs, fmt.Errorf("problem sending admin email notification to %q: %w", email, err)) } } - return result.ErrorOrNil() + return errors.Join(errs...) } func (e *Email) buildAndSendMessage(ctx context.Context, req Request, email string, forAdmin bool) error { diff --git a/backend/app/notify/email_test.go b/backend/app/notify/email_test.go index 87117d38..b40cf6af 100644 --- a/backend/app/notify/email_test.go +++ b/backend/app/notify/email_test.go @@ -110,10 +110,10 @@ func TestEmailSendErrors(t *testing.T) { e.msgTmpl, err = template.New("test").Parse("{{.Test}}") assert.NoError(t, err) assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "test"}}, Emails: []string{"bad@example.org"}}), - "1 error occurred:\n\t* problem sending user email notification to \"bad@example.org\": "+ + "problem sending user email notification to \"bad@example.org\": "+ "error executing template to build comment reply message: "+ "template: test:1:2: executing \"test\" at <.Test>: "+ - "can't evaluate field Test in type notify.msgTmplData\n\n") + "can't evaluate field Test in type notify.msgTmplData") ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -121,8 +121,14 @@ func TestEmailSendErrors(t *testing.T) { "sending email messages about comment \"999\" aborted due to canceled context") assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Emails: []string{"bad@example.org"}}), - "1 error occurred:\n\t* problem sending user email notification to \"bad@example.org\":"+ - " error creating token for unsubscribe link: token generation error\n\n") + "problem sending user email notification to \"bad@example.org\":"+ + " error creating token for unsubscribe link: token generation error") + + // errors for all failed recipients are reported, not just the last one + assert.EqualError(t, e.Send(context.Background(), + Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Emails: []string{"bad1@example.org", "bad2@example.org"}}), + "problem sending user email notification to \"bad1@example.org\": error creating token for unsubscribe link: token generation error\n"+ + "problem sending user email notification to \"bad2@example.org\": error creating token for unsubscribe link: token generation error") } func TestEmailSend_ExitConditions(t *testing.T) { diff --git a/backend/app/notify/telegram.go b/backend/app/notify/telegram.go index 924478bd..43158cff 100644 --- a/backend/app/notify/telegram.go +++ b/backend/app/notify/telegram.go @@ -2,12 +2,12 @@ package notify import ( "context" + "errors" "fmt" "time" log "github.com/go-pkgz/lgr" ntf "github.com/go-pkgz/notify" - "github.com/hashicorp/go-multierror" ) const commentTextLengthLimit = 100 @@ -47,14 +47,14 @@ func NewTelegram(params TelegramParams) (*Telegram, error) { // Send to telegram recipients func (t *Telegram) Send(ctx context.Context, req Request) error { log.Printf("[DEBUG] send telegram notification for comment ID %s", req.Comment.ID) - result := new(multierror.Error) + var errs []error msg := t.buildMessage(req) if t.AdminChannelID != "" { err := t.Telegram.Send(ctx, fmt.Sprintf("telegram:%s?parseMode=HTML", t.AdminChannelID), msg) if err != nil { - result = multierror.Append(result, + errs = append(errs, fmt.Errorf("problem sending admin telegram notification about comment ID %s to %s: %w", req.Comment.ID, t.AdminChannelID, err, ), @@ -66,7 +66,7 @@ func (t *Telegram) Send(ctx context.Context, req Request) error { for _, user := range req.Telegrams { err := t.Telegram.Send(ctx, fmt.Sprintf("telegram:%s?parseMode=HTML", user), msg) if err != nil { - result = multierror.Append(result, + errs = append(errs, fmt.Errorf("problem sending user telegram notification about comment ID %s to %q: %w", req.Comment.ID, user, err, ), @@ -74,7 +74,7 @@ func (t *Telegram) Send(ctx context.Context, req Request) error { } } } - return result.ErrorOrNil() + return errors.Join(errs...) } // buildMessage generates message for generic notification about new comment diff --git a/backend/app/notify/telegram_test.go b/backend/app/notify/telegram_test.go index dc832915..70f09dee 100644 --- a/backend/app/notify/telegram_test.go +++ b/backend/app/notify/telegram_test.go @@ -30,7 +30,6 @@ func TestTelegram_Send(t *testing.T) { err := tb.Send(context.Background(), Request{Comment: c, parent: cp, Telegrams: []string{"test_user_channel"}}) assert.Error(t, err) - assert.Contains(t, err.Error(), "2 errors occurred") assert.Contains(t, err.Error(), "problem sending user telegram notification about comment ID 999 to \"test_user_channel\"") assert.Contains(t, err.Error(), "problem sending admin telegram notification about comment ID 999 to remark_test") diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 3f511195..2590797d 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -21,7 +21,6 @@ import ( log "github.com/go-pkgz/lgr" R "github.com/go-pkgz/rest" "github.com/golang-jwt/jwt/v5" - "github.com/hashicorp/go-multierror" "github.com/umputun/remark42/backend/app/notify" "github.com/umputun/remark42/backend/app/rest" @@ -663,10 +662,8 @@ func (s *private) userAllDataCtrl(w http.ResponseWriter, r *http.Request) { return e } - var merr error - merr = multierror.Append(merr, write([]byte(`{"info": `))) // send user prefix - merr = multierror.Append(merr, write(userB)) // send user info - merr = multierror.Append(merr, write([]byte(`, "comments":`))) // send comments prefix + // send user prefix, user info and comments prefix + errs := []error{write([]byte(`{"info": `)), write(userB), write([]byte(`, "comments":`))} // get comments in 100 in each paginated request for i := range 100 { @@ -681,15 +678,15 @@ func (s *private) userAllDataCtrl(w http.ResponseWriter, r *http.Request) { return } - merr = multierror.Append(merr, write(b)) + errs = append(errs, write(b)) if len(comments) != 100 { break } } - merr = multierror.Append(merr, write([]byte(`}`))) - if merr.(*multierror.Error).ErrorOrNil() != nil { - rest.SendErrorJSON(w, r, http.StatusInternalServerError, merr, "can't write user info", rest.ErrInternal) + errs = append(errs, write([]byte(`}`))) + if err := errors.Join(errs...); err != nil { + rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "can't write user info", rest.ErrInternal) return } } diff --git a/backend/app/store/engine/bolt.go b/backend/app/store/engine/bolt.go index c84094b1..7816320b 100644 --- a/backend/app/store/engine/bolt.go +++ b/backend/app/store/engine/bolt.go @@ -9,7 +9,6 @@ import ( "time" log "github.com/go-pkgz/lgr" - "github.com/hashicorp/go-multierror" bolt "go.etcd.io/bbolt" berrors "go.etcd.io/bbolt/errors" @@ -415,14 +414,14 @@ func (b *BoltDB) Delete(req DeleteRequest) error { // Close boltdb store func (b *BoltDB) Close() error { - errs := new(multierror.Error) + var errs []error for site, db := range b.dbs { err := db.Close() if err != nil { - errs = multierror.Append(errs, fmt.Errorf("can't close site %s: %w", site, err)) + errs = append(errs, fmt.Errorf("can't close site %s: %w", site, err)) } } - return errs.ErrorOrNil() + return errors.Join(errs...) } // Last returns up to max last comments for given siteID diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go index 30a04fcf..194a1a45 100644 --- a/backend/app/store/image/fs_store.go +++ b/backend/app/store/image/fs_store.go @@ -147,8 +147,8 @@ func (f *FileSystem) Cleanup(_ context.Context, ttl time.Duration) error { age := time.Since(info.ModTime()) if age > (ttl + 100*time.Millisecond) { // delay cleanup triggering to allow commit log.Printf("[INFO] remove staging image %s, age %v", fpath, age) - rmErr := os.Remove(fpath) //nolint:gosec // staging dir is server-only, no untrusted symlinks land here - _ = os.Remove(path.Dir(fpath)) //nolint:gosec // same staging dir + rmErr := os.Remove(fpath) //nolint:gosec // staging dir is server-only, no untrusted symlinks land here + _ = os.Remove(path.Dir(fpath)) //nolint:gosec // same staging dir return rmErr } return nil diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index b56eb862..72ac500e 100644 --- a/backend/app/store/image/image.go +++ b/backend/app/store/image/image.go @@ -11,6 +11,7 @@ import ( "context" "crypto/sha1" //nolint:gosec // not used for cryptography "encoding/base64" + "errors" "fmt" "image" _ "image/gif" // register gif decoder @@ -27,7 +28,6 @@ import ( "github.com/PuerkitoBio/goquery" log "github.com/go-pkgz/lgr" - "github.com/hashicorp/go-multierror" "github.com/rs/xid" "golang.org/x/image/draw" _ "golang.org/x/image/webp" // register webp decoder so DecodeConfig accepts what readAndValidateImage allows @@ -91,14 +91,14 @@ func NewService(s Store, p ServiceParams) *Service { // Commit multiple ids immediately func (s *Service) Commit(idsFn func() []string) error { - errs := new(multierror.Error) + var errs []error for _, id := range idsFn() { err := s.store.Commit(id) if err != nil { - errs = multierror.Append(errs, fmt.Errorf("failed to commit image %s: %w", id, err)) + errs = append(errs, fmt.Errorf("failed to commit image %s: %w", id, err)) } } - return errs.ErrorOrNil() + return errors.Join(errs...) } // Submit multiple ids via function for delayed commit diff --git a/backend/app/store/service/service.go b/backend/app/store/service/service.go index 57c0390c..75a9f5a6 100644 --- a/backend/app/store/service/service.go +++ b/backend/app/store/service/service.go @@ -3,6 +3,7 @@ package service import ( + "errors" "fmt" "math" "slices" @@ -14,7 +15,6 @@ import ( "github.com/go-pkgz/lcw/v2" log "github.com/go-pkgz/lgr" "github.com/google/uuid" - "github.com/hashicorp/go-multierror" bf "github.com/russross/blackfriday/v2" "github.com/umputun/remark42/backend/app/store" @@ -249,18 +249,18 @@ func (s *DataStore) ResubmitStagingImages(sites []string) error { if ts.IsZero() { return nil } - result := new(multierror.Error) + var errs []error for _, site := range sites { locator := store.Locator{SiteID: site} comments, err := s.FindSince(locator, "time", store.User{}, ts) if err != nil { - result = multierror.Append(result, fmt.Errorf("problem finding comments for site %s: %w", site, err)) + errs = append(errs, fmt.Errorf("problem finding comments for site %s: %w", site, err)) } for _, c := range comments { s.submitImages(c) } } - return result.ErrorOrNil() + return errors.Join(errs...) } // submitImages initiated delayed commit of all images from the comment uploaded to remark42 @@ -915,32 +915,32 @@ func (s *DataStore) Metas(siteID string) (umetas []UserMetaData, pmetas []PostMe // SetMetas saves metadata for users and posts func (s *DataStore) SetMetas(siteID string, umetas []UserMetaData, pmetas []PostMetaData) (err error) { - errs := new(multierror.Error) + var errs []error // save posts metas for _, pm := range pmetas { if pm.ReadOnly { - errs = multierror.Append(errs, s.SetReadOnly(store.Locator{SiteID: siteID, URL: pm.URL}, true)) + errs = append(errs, s.SetReadOnly(store.Locator{SiteID: siteID, URL: pm.URL}, true)) } } // save users metas for _, um := range umetas { if um.Blocked.Status { - errs = multierror.Append(errs, s.SetBlock(siteID, um.ID, true, time.Until(um.Blocked.Until))) + errs = append(errs, s.SetBlock(siteID, um.ID, true, time.Until(um.Blocked.Until))) } if um.Verified { - errs = multierror.Append(errs, s.SetVerified(siteID, um.ID, true)) + errs = append(errs, s.SetVerified(siteID, um.ID, true)) } // this code doesn't delete user details in case they are not set in import but present in DB already if um.Details.Email != "" { req := engine.UserDetailRequest{Locator: store.Locator{SiteID: siteID}, UserID: um.ID, Detail: engine.UserEmail, Update: um.Details.Email} _, err := s.Engine.UserDetail(req) - errs = multierror.Append(errs, err) + errs = append(errs, err) } } - return errs.ErrorOrNil() + return errors.Join(errs...) } // User gets comment for given userID on siteID @@ -972,15 +972,15 @@ func (s *DataStore) Last(siteID string, limit int, since time.Time, user store.U // Close store service func (s *DataStore) Close() error { - errs := new(multierror.Error) + var errs []error if s.repliesCache.LoadingCache != nil { - errs = multierror.Append(errs, s.repliesCache.Close()) + errs = append(errs, s.repliesCache.Close()) } if s.TitleExtractor != nil { - errs = multierror.Append(errs, s.TitleExtractor.Close()) + errs = append(errs, s.TitleExtractor.Close()) } - errs = multierror.Append(errs, s.Engine.Close()) - return errs.ErrorOrNil() + errs = append(errs, s.Engine.Close()) + return errors.Join(errs...) } func (s *DataStore) upsAndDowns(c store.Comment) (ups, downs int) { diff --git a/backend/go.mod b/backend/go.mod index 8d680a83..8acaf852 100644 --- a/backend/go.mod +++ b/backend/go.mod @@ -19,7 +19,6 @@ require ( github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/uuid v1.6.0 github.com/gorilla/feeds v1.2.0 - github.com/hashicorp/go-multierror v1.1.1 github.com/jessevdk/go-flags v1.6.1 github.com/kyokomi/emoji/v2 v2.2.13 github.com/microcosm-cc/bluemonday v1.0.27 @@ -51,6 +50,7 @@ require ( github.com/gorilla/css v1.0.1 // indirect github.com/gorilla/websocket v1.5.3 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/golang-lru/v2 v2.0.7 // indirect github.com/klauspost/compress v1.18.7 // indirect github.com/montanaflynn/stats v0.9.0 // indirect