Skip to content

Commit

Permalink
Merge pull request #2424 from MadVikingGod/refactor-errorhandler
Browse files Browse the repository at this point in the history 
Refactor ErrorHandler
  • Loading branch information
@MadVikingGod
MadVikingGod committed Dec 6, 2021
2 parents a4f183b + c351302 commit 2b7c650
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 119 deletions.
63 changes: 27 additions & 36 deletions handler.go
View file
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
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{}
}

0 comments on commit 2b7c650

Please sign in to comment.