[bugfix] Rework MultiError to wrap + unwrap errors properly (#2057)

* rework multierror a bit

* test multierror

2 files changed, 101 insertions, 14 deletions

diff --git a/internal/gtserror/multi.go b/internal/gtserror/multi.go
index 371eb2f63..1c533b285 100644
--- a/internal/gtserror/multi.go
+++ b/internal/gtserror/multi.go
@@ -20,25 +20,48 @@ package gtserror
 import (
 	"errors"
 	"fmt"
-	"strings"
 )
 
-// MultiError allows encapsulating multiple errors under a singular instance,
-// which is useful when you only want to log on errors, not return early / bubble up.
-type MultiError []string
+// MultiError allows encapsulating multiple
+// errors under a singular instance, which
+// is useful when you only want to log on
+// errors, not return early / bubble up.
+type MultiError struct {
+	e []error
+}
 
-func (e *MultiError) Append(err error) {
-	*e = append(*e, err.Error())
+// NewMultiError returns a *MultiError with
+// the capacity of its underlying error slice
+// set to the provided value.
+//
+// This capacity can be exceeded if necessary,
+// but it saves a teeny tiny bit of memory if
+// callers set it correctly.
+//
+// If you don't know in advance what the capacity
+// must be, just use new(MultiError) instead.
+func NewMultiError(capacity int) *MultiError {
+	return &MultiError{
+		e: make([]error, 0, capacity),
+	}
 }
 
-func (e *MultiError) Appendf(format string, args ...any) {
-	*e = append(*e, fmt.Sprintf(format, args...))
+// Append the given error to the MultiError.
+func (m *MultiError) Append(err error) {
+	m.e = append(m.e, err)
 }
 
-// Combine converts this multiError to a singular error instance, returning nil if empty.
-func (e MultiError) Combine() error {
-	if len(e) == 0 {
-		return nil
-	}
-	return errors.New(`"` + strings.Join(e, `","`) + `"`)
+// Append the given format string to the MultiError.
+//
+// It is valid to use %w in the format string
+// to wrap any other errors.
+func (m *MultiError) Appendf(format string, args ...any) {
+	m.e = append(m.e, fmt.Errorf(format, args...))
+}
+
+// Combine the MultiError into a single error.
+//
+// Unwrap will work on the returned error as expected.
+func (m MultiError) Combine() error {
+	return errors.Join(m.e...)
 }
diff --git a/internal/gtserror/multi_test.go b/internal/gtserror/multi_test.go
new file mode 100644
index 000000000..9c16c1a53
--- /dev/null
+++ b/internal/gtserror/multi_test.go
@@ -0,0 +1,64 @@
+// GoToSocial
+// Copyright (C) GoToSocial Authors admin@gotosocial.org
+// SPDX-License-Identifier: AGPL-3.0-or-later
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package gtserror
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/superseriousbusiness/gotosocial/internal/db"
+)
+
+func TestMultiError(t *testing.T) {
+	errs := MultiError{
+		e: []error{
+			db.ErrNoEntries,
+			errors.New("oopsie woopsie we did a fucky wucky etc"),
+		},
+	}
+	errs.Appendf("appended + wrapped error: %w", db.ErrAlreadyExists)
+
+	err := errs.Combine()
+
+	if !errors.Is(err, db.ErrNoEntries) {
+		t.Error("should be db.ErrNoEntries")
+	}
+
+	if !errors.Is(err, db.ErrAlreadyExists) {
+		t.Error("should be db.ErrAlreadyExists")
+	}
+
+	if errors.Is(err, db.ErrBusyTimeout) {
+		t.Error("should not be db.ErrBusyTimeout")
+	}
+
+	errString := err.Error()
+	expected := `sql: no rows in result set
+oopsie woopsie we did a fucky wucky etc
+appended + wrapped error: already exists`
+	if errString != expected {
+		t.Errorf("errString '%s' should be '%s'", errString, expected)
+	}
+}
+
+func TestMultiErrorEmpty(t *testing.T) {
+	err := new(MultiError).Combine()
+	if err != nil {
+		t.Errorf("should be nil")
+	}
+}