Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor ErrorHandler #2424

Merged
merged 4 commits into from Dec 6, 2021
Merged

Refactor ErrorHandler #2424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3090cea
Migrated to a global errorHandler delegate
MadVikingGod Nov 29, 2021
cbf8bad
Removed benchmark that doesn't test anything anymore
MadVikingGod Nov 29, 2021
bd3c3b2
Removed unsued test structure
MadVikingGod Dec 1, 2021
c351302
Merge branch 'main' into refactor-errorhandler
MadVikingGod Dec 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 27 additions & 36 deletions handler.go
View file
Open in desktop
Expand Up @@ -18,7 +18,6 @@ import (
"log"
"os"
"sync"
"sync/atomic"
)

var (
Expand All @@ -28,44 +27,45 @@ var (
// `Handle` and will be delegated to the registered ErrorHandler.
globalErrorHandler = defaultErrorHandler()

// delegateErrorHandlerOnce ensures that a user provided ErrorHandler is
// only ever registered once.
delegateErrorHandlerOnce sync.Once

// Compile-time check that delegator implements ErrorHandler.
_ ErrorHandler = (*delegator)(nil)
// Compile-time check that errLogger implements ErrorHandler.
_ ErrorHandler = (*errLogger)(nil)
)

type holder struct {
eh ErrorHandler
type delegator struct {
lock *sync.RWMutex
eh ErrorHandler
}

func defaultErrorHandler() *atomic.Value {
v := &atomic.Value{}
v.Store(holder{eh: &delegator{l: log.New(os.Stderr, "", log.LstdFlags)}})
return v
func (d *delegator) Handle(err error) {
d.lock.RLock()
defer d.lock.RUnlock()
d.eh.Handle(err)
}

// delegator logs errors if no delegate is set, otherwise they are delegated.
type delegator struct {
delegate atomic.Value
// setDelegate sets the ErrorHandler delegate.
func (d *delegator) setDelegate(eh ErrorHandler) {
d.lock.Lock()
defer d.lock.Unlock()
d.eh = eh
}

func defaultErrorHandler() *delegator {
return &delegator{
lock: &sync.RWMutex{},
eh: &errLogger{l: log.New(os.Stderr, "", log.LstdFlags)},
}

l *log.Logger
}

// setDelegate sets the ErrorHandler delegate.
func (h *delegator) setDelegate(d ErrorHandler) {
// It is critical this is guarded with delegateErrorHandlerOnce, if it is
// called again with a different concrete type it will panic.
h.delegate.Store(d)
// errLogger logs errors if no delegate is set, otherwise they are delegated.
type errLogger struct {
l *log.Logger
}

// Handle logs err if no delegate is set, otherwise it is delegated.
func (h *delegator) Handle(err error) {
if d := h.delegate.Load(); d != nil {
d.(ErrorHandler).Handle(err)
return
}
func (h *errLogger) Handle(err error) {
h.l.Print(err)
}

Expand All @@ -79,7 +79,7 @@ func (h *delegator) Handle(err error) {
// Subsequent calls to SetErrorHandler after the first will not forward errors
// to the new ErrorHandler for prior returned instances.
func GetErrorHandler() ErrorHandler {
return globalErrorHandler.Load().(holder).eh
return globalErrorHandler
}

// SetErrorHandler sets the global ErrorHandler to h.
Expand All @@ -89,16 +89,7 @@ func GetErrorHandler() ErrorHandler {
// ErrorHandler. Subsequent calls will set the global ErrorHandler, but not
// delegate errors to h.
func SetErrorHandler(h ErrorHandler) {
delegateErrorHandlerOnce.Do(func() {
current := GetErrorHandler()
if current == h {
return
}
if internalHandler, ok := current.(*delegator); ok {
internalHandler.setDelegate(h)
}
})
globalErrorHandler.Store(holder{eh: h})
globalErrorHandler.setDelegate(h)
}

// Handle is a convenience function for ErrorHandler().Handle(err)
Expand Down
126 changes: 43 additions & 83 deletions handler_test.go
View file
Open in desktop
Expand Up @@ -19,37 +19,28 @@ import (
"errors"
"io/ioutil"
"log"
"sync"
"sync/atomic"
"os"
"testing"

"github.com/stretchr/testify/suite"
)

type errLogger []string
type testErrCatcher []string

func (l *errLogger) Write(p []byte) (int, error) {
func (l *testErrCatcher) Write(p []byte) (int, error) {
msg := bytes.TrimRight(p, "\n")
(*l) = append(*l, string(msg))
return len(msg), nil
}

func (l *errLogger) Reset() {
*l = errLogger([]string{})
func (l *testErrCatcher) Reset() {
*l = testErrCatcher([]string{})
}

func (l *errLogger) Got() []string {
func (l *testErrCatcher) Got() []string {
return []string(*l)
}

type logger struct {
l *log.Logger
}

func (l *logger) Handle(err error) {
l.l.Print(err)
}

func causeErr(text string) {
Handle(errors.New(text))
}
Expand All @@ -58,92 +49,91 @@ type HandlerTestSuite struct {
suite.Suite

origHandler ErrorHandler
errLogger *errLogger
errCatcher *testErrCatcher
}

func (s *HandlerTestSuite) SetupSuite() {
s.errLogger = new(errLogger)
s.origHandler = globalErrorHandler.Load().(holder).eh
s.errCatcher = new(testErrCatcher)
s.origHandler = globalErrorHandler.eh

globalErrorHandler.Store(holder{eh: &delegator{l: log.New(s.errLogger, "", 0)}})
globalErrorHandler.setDelegate(&errLogger{l: log.New(s.errCatcher, "", 0)})
}

func (s *HandlerTestSuite) TearDownSuite() {
globalErrorHandler.Store(holder{eh: s.origHandler})
delegateErrorHandlerOnce = sync.Once{}
globalErrorHandler.setDelegate(s.origHandler)
}

func (s *HandlerTestSuite) SetupTest() {
s.errLogger.Reset()
s.errCatcher.Reset()
}

func (s *HandlerTestSuite) TearDownTest() {
globalErrorHandler.Store(holder{eh: &delegator{l: log.New(s.errLogger, "", 0)}})
delegateErrorHandlerOnce = sync.Once{}
globalErrorHandler.setDelegate(&errLogger{l: log.New(s.errCatcher, "", 0)})
}

func (s *HandlerTestSuite) TestGlobalHandler() {
errs := []string{"one", "two"}
GetErrorHandler().Handle(errors.New(errs[0]))
Handle(errors.New(errs[1]))
s.Assert().Equal(errs, s.errLogger.Got())
s.Assert().Equal(errs, s.errCatcher.Got())
}

func (s *HandlerTestSuite) TestDelegatedHandler() {
eh := GetErrorHandler()

newErrLogger := new(errLogger)
SetErrorHandler(&logger{l: log.New(newErrLogger, "", 0)})
newErrLogger := new(testErrCatcher)
SetErrorHandler(&errLogger{l: log.New(newErrLogger, "", 0)})

errs := []string{"TestDelegatedHandler"}
eh.Handle(errors.New(errs[0]))
s.Assert().Equal(errs, newErrLogger.Got())
}

func (s *HandlerTestSuite) TestSettingDefaultIsANoOp() {
SetErrorHandler(GetErrorHandler())
d := globalErrorHandler.Load().(holder).eh.(*delegator)
s.Assert().Nil(d.delegate.Load())
}

func (s *HandlerTestSuite) TestNoDropsOnDelegate() {
causeErr("")
s.Require().Len(s.errLogger.Got(), 1)
s.Require().Len(s.errCatcher.Got(), 1)

// Change to another Handler. We are testing this is loss-less.
newErrLogger := new(errLogger)
secondary := &logger{
newErrLogger := new(testErrCatcher)
secondary := &errLogger{
l: log.New(newErrLogger, "", 0),
}
SetErrorHandler(secondary)

causeErr("")
s.Assert().Len(s.errLogger.Got(), 1, "original Handler used after delegation")
s.Assert().Len(s.errCatcher.Got(), 1, "original Handler used after delegation")
s.Assert().Len(newErrLogger.Got(), 1, "new Handler not used after delegation")
}

func (s *HandlerTestSuite) TestAllowMultipleSets() {
notUsed := new(errLogger)
notUsed := new(testErrCatcher)

secondary := &logger{l: log.New(notUsed, "", 0)}
secondary := &errLogger{l: log.New(notUsed, "", 0)}
SetErrorHandler(secondary)
s.Require().Same(GetErrorHandler(), secondary, "new Handler not set")
s.Require().Same(GetErrorHandler(), globalErrorHandler, "set changed globalErrorHandler")
s.Require().Same(globalErrorHandler.eh, secondary, "new Handler not set")

tertiary := &logger{l: log.New(notUsed, "", 0)}
tertiary := &errLogger{l: log.New(notUsed, "", 0)}
SetErrorHandler(tertiary)
s.Assert().Same(GetErrorHandler(), tertiary, "user Handler not overridden")
s.Require().Same(GetErrorHandler(), globalErrorHandler, "set changed globalErrorHandler")
s.Assert().Same(globalErrorHandler.eh, tertiary, "user Handler not overridden")
}

func TestHandlerTestSuite(t *testing.T) {
suite.Run(t, new(HandlerTestSuite))
}

func TestHandlerRace(t *testing.T) {
go SetErrorHandler(&errLogger{log.New(os.Stderr, "", 0)})
go Handle(errors.New("Error"))
}

func BenchmarkErrorHandler(b *testing.B) {
primary := &delegator{l: log.New(ioutil.Discard, "", 0)}
secondary := &logger{l: log.New(ioutil.Discard, "", 0)}
tertiary := &logger{l: log.New(ioutil.Discard, "", 0)}
primary := &errLogger{l: log.New(ioutil.Discard, "", 0)}
secondary := &errLogger{l: log.New(ioutil.Discard, "", 0)}
tertiary := &errLogger{l: log.New(ioutil.Discard, "", 0)}

globalErrorHandler.Store(holder{eh: primary})
globalErrorHandler.setDelegate(primary)

err := errors.New("BenchmarkErrorHandler")

Expand All @@ -161,14 +151,9 @@ func BenchmarkErrorHandler(b *testing.B) {
GetErrorHandler().Handle(err)
Handle(err)

b.StopTimer()
primary.delegate = atomic.Value{}
globalErrorHandler.Store(holder{eh: primary})
delegateErrorHandlerOnce = sync.Once{}
b.StartTimer()
globalErrorHandler.setDelegate(primary)
}

b.StopTimer()
reset()
}

Expand All @@ -182,22 +167,21 @@ func BenchmarkGetDefaultErrorHandler(b *testing.B) {
}

func BenchmarkGetDelegatedErrorHandler(b *testing.B) {
SetErrorHandler(&logger{l: log.New(ioutil.Discard, "", 0)})
SetErrorHandler(&errLogger{l: log.New(ioutil.Discard, "", 0)})

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
eh = GetErrorHandler()
}

b.StopTimer()
reset()
}

func BenchmarkDefaultErrorHandlerHandle(b *testing.B) {
globalErrorHandler.Store(holder{
eh: &delegator{l: log.New(ioutil.Discard, "", 0)},
})
globalErrorHandler.setDelegate(
&errLogger{l: log.New(ioutil.Discard, "", 0)},
)

eh := GetErrorHandler()
err := errors.New("BenchmarkDefaultErrorHandlerHandle")
Expand All @@ -208,13 +192,12 @@ func BenchmarkDefaultErrorHandlerHandle(b *testing.B) {
eh.Handle(err)
}

b.StopTimer()
reset()
}

func BenchmarkDelegatedErrorHandlerHandle(b *testing.B) {
eh := GetErrorHandler()
SetErrorHandler(&logger{l: log.New(ioutil.Discard, "", 0)})
SetErrorHandler(&errLogger{l: log.New(ioutil.Discard, "", 0)})
err := errors.New("BenchmarkDelegatedErrorHandlerHandle")

b.ReportAllocs()
Expand All @@ -223,44 +206,21 @@ func BenchmarkDelegatedErrorHandlerHandle(b *testing.B) {
eh.Handle(err)
}

b.StopTimer()
reset()
}

func BenchmarkSetErrorHandlerDelegation(b *testing.B) {
alt := &logger{l: log.New(ioutil.Discard, "", 0)}
alt := &errLogger{l: log.New(ioutil.Discard, "", 0)}

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
SetErrorHandler(alt)

b.StopTimer()
reset()
b.StartTimer()
}
}

func BenchmarkSetErrorHandlerNoDelegation(b *testing.B) {
eh := []ErrorHandler{
&logger{l: log.New(ioutil.Discard, "", 0)},
&logger{l: log.New(ioutil.Discard, "", 0)},
}
mod := len(eh)
// Do not measure delegation.
SetErrorHandler(eh[1])

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
SetErrorHandler(eh[i%mod])
}

b.StopTimer()
reset()
}

func reset() {
globalErrorHandler = defaultErrorHandler()
delegateErrorHandlerOnce = sync.Once{}
}