mirror of
https://github.com/luxfi/consensus.git
synced 2026-09-24 00:26:58 +00:00
notifier: answer an unset logger where the loop starts, not where it logs
forwardNotification logs on its first statement and runs on the goroutine Start spawns, so a NotificationForwarder holding no logger dies there — and a panic on that goroutine takes the process with it, since nothing above it can recover. The logger can go unset two ways: NewNotificationForwarder stores whatever it is handed, and Log is exported, so a struct literal can simply omit it. Start is the one place the loop begins and the only route to run(), so answering it there covers both ways at once, which defaulting in the constructor alone would not. This is the same answer bootstrap.New, summary.New and the engine's NewWithConfig already give; the forwarder was the remaining exported constructor without it. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -55,6 +55,16 @@ func (nf *NotificationForwarder) Start() {
|
||||
return
|
||||
}
|
||||
|
||||
// The loop below logs before it does anything else, and it runs on a
|
||||
// goroutine whose panic no caller can recover. Log is exported and the
|
||||
// constructor also takes one, so there are two ways to leave it unset;
|
||||
// answering it here covers both, because this is the one place the loop
|
||||
// begins. Noop discards what it is given, so a caller that wanted silence
|
||||
// still gets it.
|
||||
if nf.Log == nil {
|
||||
nf.Log = log.Noop()
|
||||
}
|
||||
|
||||
nf.started = true
|
||||
nf.execCtx, nf.cancel = context.WithCancel(context.Background())
|
||||
nf.executing.Add(1)
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
// Copyright (C) 2019-2026, Lux Industries, Inc. All rights reserved.
|
||||
// See the file LICENSE for licensing terms.
|
||||
|
||||
package handler
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
type notifierFunc func(context.Context, VMMessage) error
|
||||
|
||||
func (f notifierFunc) Notify(ctx context.Context, m VMMessage) error { return f(ctx, m) }
|
||||
|
||||
// The forwarding loop runs on a goroutine nobody can recover from, so it must
|
||||
// not depend on the caller having passed a logger. Log is exported and the
|
||||
// constructor takes one, which is two ways to leave it unset; Start answers
|
||||
// both, since it is the one place the loop begins.
|
||||
//
|
||||
// Removing the default in Start makes these crash, not fail: the first
|
||||
// statement of forwardNotification logs, before it does anything else.
|
||||
func TestNotificationForwarderRunsWithoutALogger(t *testing.T) {
|
||||
for name, build := range map[string]func(Notifier, Subscription) *NotificationForwarder{
|
||||
"constructed with a nil logger": func(e Notifier, s Subscription) *NotificationForwarder {
|
||||
return NewNotificationForwarder(e, s, nil)
|
||||
},
|
||||
"literal leaving Log unset": func(e Notifier, s Subscription) *NotificationForwarder {
|
||||
return &NotificationForwarder{Engine: e, Subscribe: s}
|
||||
},
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
subscribed := make(chan struct{}, 1)
|
||||
subscribe := func(ctx context.Context) (VMMessage, error) {
|
||||
select {
|
||||
case subscribed <- struct{}{}:
|
||||
default:
|
||||
}
|
||||
<-ctx.Done() // park until Stop, so the loop does not spin
|
||||
return VMMessage{}, ctx.Err()
|
||||
}
|
||||
notify := notifierFunc(func(context.Context, VMMessage) error { return nil })
|
||||
|
||||
nf := build(notify, subscribe)
|
||||
nf.Start()
|
||||
defer nf.Stop()
|
||||
|
||||
select {
|
||||
case <-subscribed:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("forwarding loop never reached the subscription")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user