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.
This commit is contained in:
committed by
Umputun
parent
6e7820d2b7
commit
f8f2becb4b
@@ -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
|
||||
|
||||
@@ -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=
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user