From 3fd5f4380fb0f71a16b1778c37669da808f021b4 Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Tue, 2 Nov 2021 14:30:48 -0700 Subject: [PATCH] util/multierr: new package github.com/go-multierror/multierror served us well. But we need a few feature from it (implement Is), and it's not worth maintaining a fork of such a small module. Instead, I did a clean room implementation inspired by its API. Signed-off-by: Josh Bleecher Snyder --- cmd/tailscaled/depaware.txt | 2 +- cmd/tailscaled/tailscaled.go | 4 +- go.mod | 1 - health/health.go | 6 +- ipn/ipnlocal/local.go | 6 +- .../tailscaled_deps_test_darwin.go | 2 +- .../tailscaled_deps_test_freebsd.go | 2 +- .../integration/tailscaled_deps_test_linux.go | 2 +- .../tailscaled_deps_test_openbsd.go | 2 +- .../tailscaled_deps_test_windows.go | 2 +- util/multierr/multierr.go | 88 +++++++++++++++++++ util/multierr/multierr_test.go | 80 +++++++++++++++++ wgengine/router/ifconfig_windows.go | 4 +- wgengine/router/router_linux.go | 4 +- 14 files changed, 186 insertions(+), 19 deletions(-) create mode 100644 util/multierr/multierr.go create mode 100644 util/multierr/multierr_test.go diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index b032a9dc3..ac3c09f88 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -59,7 +59,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de L github.com/aws/smithy-go/transport/http/internal/io from github.com/aws/smithy-go/transport/http L github.com/aws/smithy-go/waiter from github.com/aws/aws-sdk-go-v2/service/ssm L github.com/coreos/go-iptables/iptables from tailscale.com/wgengine/router - github.com/go-multierror/multierror from tailscale.com/cmd/tailscaled+ W 💣 github.com/go-ole/go-ole from github.com/go-ole/go-ole/oleutil+ W 💣 github.com/go-ole/go-ole/oleutil from tailscale.com/wgengine/winnet L 💣 github.com/godbus/dbus/v5 from tailscale.com/net/dns @@ -228,6 +227,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de LW tailscale.com/util/endian from tailscale.com/net/dns+ tailscale.com/util/groupmember from tailscale.com/ipn/ipnserver tailscale.com/util/lineread from tailscale.com/hostinfo+ + tailscale.com/util/multierr from tailscale.com/cmd/tailscaled+ tailscale.com/util/osshare from tailscale.com/cmd/tailscaled+ tailscale.com/util/pidowner from tailscale.com/ipn/ipnserver tailscale.com/util/racebuild from tailscale.com/logpolicy diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 6edae3030..47babf497 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -27,7 +27,6 @@ import ( "syscall" "time" - "github.com/go-multierror/multierror" "tailscale.com/ipn" "tailscale.com/ipn/ipnserver" "tailscale.com/logpolicy" @@ -38,6 +37,7 @@ import ( "tailscale.com/paths" "tailscale.com/types/flagtype" "tailscale.com/types/logger" + "tailscale.com/util/multierr" "tailscale.com/util/osshare" "tailscale.com/version" "tailscale.com/version/distro" @@ -361,7 +361,7 @@ func createEngine(logf logger.Logf, linkMon *monitor.Mon) (e wgengine.Engine, us logf("wgengine.NewUserspaceEngine(tun %q) error: %v", name, err) errs = append(errs, err) } - return nil, false, multierror.New(errs) + return nil, false, multierr.New(errs...) } var wrapNetstack = shouldWrapNetstack() diff --git a/go.mod b/go.mod index 857cc7122..0112c7fc2 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/dave/jennifer v1.4.1 github.com/frankban/quicktest v1.13.1 github.com/gliderlabs/ssh v0.3.3 - github.com/go-multierror/multierror v1.0.2 github.com/go-ole/go-ole v1.2.6-0.20210915003542-8b1f7f90f6b1 github.com/godbus/dbus/v5 v5.0.5 github.com/google/go-cmp v0.5.6 diff --git a/health/health.go b/health/health.go index 01c79dd65..3a93779a0 100644 --- a/health/health.go +++ b/health/health.go @@ -16,8 +16,8 @@ import ( "sync/atomic" "time" - "github.com/go-multierror/multierror" "tailscale.com/tailcfg" + "tailscale.com/util/multierr" ) var ( @@ -268,7 +268,7 @@ func selfCheckLocked() { // OverallError returns a summary of the health state. // // If there are multiple problems, the error will be of type -// multierror.MultipleErrors. +// multierr.Error. func OverallError() error { mu.Lock() defer mu.Unlock() @@ -337,7 +337,7 @@ func overallErrorLocked() error { // Not super efficient (stringifying these in a sort), but probably max 2 or 3 items. return errs[i].Error() < errs[j].Error() }) - return multierror.New(errs) + return multierr.New(errs...) } var ( diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 9aa9d0a24..d3e118a58 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -25,7 +25,6 @@ import ( "syscall" "time" - "github.com/go-multierror/multierror" "inet.af/netaddr" "tailscale.com/client/tailscale/apitype" "tailscale.com/control/controlclient" @@ -49,6 +48,7 @@ import ( "tailscale.com/types/preftype" "tailscale.com/util/deephash" "tailscale.com/util/dnsname" + "tailscale.com/util/multierr" "tailscale.com/util/osshare" "tailscale.com/util/systemd" "tailscale.com/version" @@ -334,8 +334,8 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func if err := health.OverallError(); err != nil { switch e := err.(type) { - case multierror.MultipleErrors: - for _, err := range e { + case multierr.Error: + for _, err := range e.Errors() { s.Health = append(s.Health, err.Error()) } default: diff --git a/tstest/integration/tailscaled_deps_test_darwin.go b/tstest/integration/tailscaled_deps_test_darwin.go index fd6c743fb..16a75c2c3 100644 --- a/tstest/integration/tailscaled_deps_test_darwin.go +++ b/tstest/integration/tailscaled_deps_test_darwin.go @@ -17,7 +17,6 @@ import ( _ "errors" _ "flag" _ "fmt" - _ "github.com/go-multierror/multierror" _ "inet.af/netaddr" _ "io" _ "io/ioutil" @@ -54,6 +53,7 @@ import ( _ "tailscale.com/types/flagtype" _ "tailscale.com/types/key" _ "tailscale.com/types/logger" + _ "tailscale.com/util/multierr" _ "tailscale.com/util/osshare" _ "tailscale.com/version" _ "tailscale.com/version/distro" diff --git a/tstest/integration/tailscaled_deps_test_freebsd.go b/tstest/integration/tailscaled_deps_test_freebsd.go index 1a83ef516..d5e7594cd 100644 --- a/tstest/integration/tailscaled_deps_test_freebsd.go +++ b/tstest/integration/tailscaled_deps_test_freebsd.go @@ -17,7 +17,6 @@ import ( _ "errors" _ "flag" _ "fmt" - _ "github.com/go-multierror/multierror" _ "inet.af/netaddr" _ "io" _ "io/ioutil" @@ -52,6 +51,7 @@ import ( _ "tailscale.com/types/flagtype" _ "tailscale.com/types/key" _ "tailscale.com/types/logger" + _ "tailscale.com/util/multierr" _ "tailscale.com/util/osshare" _ "tailscale.com/version" _ "tailscale.com/version/distro" diff --git a/tstest/integration/tailscaled_deps_test_linux.go b/tstest/integration/tailscaled_deps_test_linux.go index 1a83ef516..d5e7594cd 100644 --- a/tstest/integration/tailscaled_deps_test_linux.go +++ b/tstest/integration/tailscaled_deps_test_linux.go @@ -17,7 +17,6 @@ import ( _ "errors" _ "flag" _ "fmt" - _ "github.com/go-multierror/multierror" _ "inet.af/netaddr" _ "io" _ "io/ioutil" @@ -52,6 +51,7 @@ import ( _ "tailscale.com/types/flagtype" _ "tailscale.com/types/key" _ "tailscale.com/types/logger" + _ "tailscale.com/util/multierr" _ "tailscale.com/util/osshare" _ "tailscale.com/version" _ "tailscale.com/version/distro" diff --git a/tstest/integration/tailscaled_deps_test_openbsd.go b/tstest/integration/tailscaled_deps_test_openbsd.go index 1a83ef516..d5e7594cd 100644 --- a/tstest/integration/tailscaled_deps_test_openbsd.go +++ b/tstest/integration/tailscaled_deps_test_openbsd.go @@ -17,7 +17,6 @@ import ( _ "errors" _ "flag" _ "fmt" - _ "github.com/go-multierror/multierror" _ "inet.af/netaddr" _ "io" _ "io/ioutil" @@ -52,6 +51,7 @@ import ( _ "tailscale.com/types/flagtype" _ "tailscale.com/types/key" _ "tailscale.com/types/logger" + _ "tailscale.com/util/multierr" _ "tailscale.com/util/osshare" _ "tailscale.com/version" _ "tailscale.com/version/distro" diff --git a/tstest/integration/tailscaled_deps_test_windows.go b/tstest/integration/tailscaled_deps_test_windows.go index a1b3dd41f..8c13e9fc5 100644 --- a/tstest/integration/tailscaled_deps_test_windows.go +++ b/tstest/integration/tailscaled_deps_test_windows.go @@ -17,7 +17,6 @@ import ( _ "errors" _ "flag" _ "fmt" - _ "github.com/go-multierror/multierror" _ "golang.org/x/sys/windows" _ "golang.org/x/sys/windows/svc" _ "golang.org/x/sys/windows/svc/mgr" @@ -56,6 +55,7 @@ import ( _ "tailscale.com/types/flagtype" _ "tailscale.com/types/key" _ "tailscale.com/types/logger" + _ "tailscale.com/util/multierr" _ "tailscale.com/util/osshare" _ "tailscale.com/util/winutil" _ "tailscale.com/version" diff --git a/util/multierr/multierr.go b/util/multierr/multierr.go new file mode 100644 index 000000000..f4c8ecf93 --- /dev/null +++ b/util/multierr/multierr.go @@ -0,0 +1,88 @@ +// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package multierr provides a simple multiple-error type. +// It was inspired by github.com/go-multierror/multierror. +package multierr + +import ( + "errors" + "strings" +) + +// An Error represents multiple errors. +type Error struct { + errs []error +} + +// Error implements the error interface. +func (e Error) Error() string { + s := new(strings.Builder) + s.WriteString("multiple errors:") + for _, err := range e.errs { + s.WriteString("\n\t") + s.WriteString(err.Error()) + } + return s.String() +} + +// Errors returns a slice containing all errors in e. +func (e Error) Errors() []error { + return append(e.errs[:0:0], e.errs...) +} + +// New returns an error composed from errs. +// Some errors in errs get special treatment: +// * nil errors are discarded +// * errors of type Error are expanded into the top level +// If the resulting slice has length 0, New returns nil. +// If the resulting slice has length 1, New returns that error. +// If the resulting slice has length > 1, New returns that slice as an Error. +func New(errs ...error) error { + dst := make([]error, 0, len(errs)) + for _, e := range errs { + switch e := e.(type) { + case nil: + continue + case Error: + dst = append(dst, e.errs...) + default: + dst = append(dst, e) + } + } + // dst has been filtered and splatted. + switch len(dst) { + case 0: + return nil + case 1: + return dst[0] + } + // Zero any remaining elements of dst left over from errs, for GC. + tail := dst[len(dst):cap(dst)] + for i := range tail { + tail[i] = nil + } + return Error{errs: dst} +} + +// Is reports whether any error in e matches target. +func (e Error) Is(target error) bool { + for _, err := range e.errs { + if errors.Is(err, target) { + return true + } + } + return false +} + +// As finds the first error in e that matches target, and if any is found, +// sets target to that error value and returns true. Otherwise, it returns false. +func (e Error) As(target interface{}) bool { + for _, err := range e.errs { + if ok := errors.As(err, target); ok { + return true + } + } + return false +} diff --git a/util/multierr/multierr_test.go b/util/multierr/multierr_test.go new file mode 100644 index 000000000..8f5e20541 --- /dev/null +++ b/util/multierr/multierr_test.go @@ -0,0 +1,80 @@ +// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package multierr_test + +import ( + "errors" + "testing" + + qt "github.com/frankban/quicktest" + "github.com/google/go-cmp/cmp/cmpopts" + "tailscale.com/util/multierr" +) + +func TestAll(t *testing.T) { + C := qt.New(t) + eqErr := qt.CmpEquals(cmpopts.EquateErrors()) + + type E = []error + N := multierr.New + + a := errors.New("a") + b := errors.New("b") + c := errors.New("c") + d := errors.New("d") + x := errors.New("x") + abcd := E{a, b, c, d} + + tests := []struct { + In E // input to New + WantNil bool // want nil returned? + WantSingle error // if non-nil, want this single error returned + WantErrors []error // if non-nil, want an Error composed of these errors returned + }{ + {In: nil, WantNil: true}, + + {In: E{nil}, WantNil: true}, + {In: E{nil, nil}, WantNil: true}, + + {In: E{a}, WantSingle: a}, + {In: E{a, nil}, WantSingle: a}, + {In: E{nil, a}, WantSingle: a}, + {In: E{nil, a, nil}, WantSingle: a}, + + {In: E{a, b}, WantErrors: E{a, b}}, + {In: E{nil, a, nil, b, nil}, WantErrors: E{a, b}}, + + {In: E{a, b, N(c, d)}, WantErrors: E{a, b, c, d}}, + {In: E{a, N(b, c), d}, WantErrors: E{a, b, c, d}}, + {In: E{N(a, b), c, d}, WantErrors: E{a, b, c, d}}, + {In: E{N(a, b), N(c, d)}, WantErrors: E{a, b, c, d}}, + {In: E{nil, N(a, nil, b), nil, N(c, d)}, WantErrors: E{a, b, c, d}}, + + {In: E{N(a, N(b, N(c, N(d))))}, WantErrors: E{a, b, c, d}}, + {In: E{N(N(N(N(a), b), c), d)}, WantErrors: E{a, b, c, d}}, + + {In: E{N(abcd...)}, WantErrors: E{a, b, c, d}}, + {In: E{N(abcd...), N(abcd...)}, WantErrors: E{a, b, c, d, a, b, c, d}}, + } + + for _, test := range tests { + got := multierr.New(test.In...) + if test.WantNil { + C.Assert(got, qt.IsNil) + continue + } + if test.WantSingle != nil { + C.Assert(got, eqErr, test.WantSingle) + continue + } + ee, _ := got.(multierr.Error) + C.Assert(ee.Errors(), eqErr, test.WantErrors) + + for _, e := range test.WantErrors { + C.Assert(ee.Is(e), qt.IsTrue) + } + C.Assert(ee.Is(x), qt.IsFalse) + } +} diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index 7f2fc417e..bffb562d2 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -15,7 +15,6 @@ import ( "sort" "time" - "github.com/go-multierror/multierror" ole "github.com/go-ole/go-ole" "golang.org/x/sys/windows" "golang.zx2c4.com/wireguard/tun" @@ -24,6 +23,7 @@ import ( "tailscale.com/health" "tailscale.com/net/interfaces" "tailscale.com/net/tsaddr" + "tailscale.com/util/multierr" "tailscale.com/wgengine/winnet" ) @@ -809,5 +809,5 @@ func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, do } } - return multierror.New(errs) + return multierr.New(errs...) } diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 2eeb08a55..c9b255c7d 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -17,7 +17,6 @@ import ( "time" "github.com/coreos/go-iptables/iptables" - "github.com/go-multierror/multierror" "github.com/tailscale/netlink" "golang.org/x/sys/unix" "golang.org/x/time/rate" @@ -27,6 +26,7 @@ import ( "tailscale.com/syncs" "tailscale.com/types/logger" "tailscale.com/types/preftype" + "tailscale.com/util/multierr" "tailscale.com/version/distro" "tailscale.com/wgengine/monitor" ) @@ -320,7 +320,7 @@ func (r *linuxRouter) Set(cfg *Config) error { } r.snatSubnetRoutes = cfg.SNATSubnetRoutes - return multierror.New(errs) + return multierr.New(errs...) } // setNetfilterMode switches the router to the given netfilter