Skip to content

Commit dbad6e2

Browse files
committed
feat(shutdown): add panic handling
use simplified handling while waiting for golang/go#53757 Signed-off-by: Artsiom Koltun <artsiom.koltun@intel.com>
1 parent 7609b47 commit dbad6e2

File tree

2 files changed

+85
-14
lines changed

2 files changed

+85
-14
lines changed

pkg/utils/shutdown.go

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package utils
77
import (
88
"context"
99
"errors"
10+
"fmt"
1011
"log"
1112
"net"
1213
"net/http"
@@ -141,7 +142,7 @@ func (s *ShutdownHandler) RunAndWait() error {
141142
for i := range s.serves {
142143
fn := s.serves[i]
143144
s.eg.Go(func() error {
144-
return fn()
145+
return wrapServeFuncPanic(fn)()
145146
})
146147
}
147148

@@ -163,7 +164,8 @@ func (s *ShutdownHandler) RunAndWait() error {
163164
for i := len(s.shutdowns) - 1; i >= 0; i-- {
164165
timeoutCtx, cancel := context.WithTimeout(context.Background(), s.timeoutPerShutdown)
165166
defer cancel()
166-
err = errors.Join(err, s.shutdowns[i](timeoutCtx))
167+
shutdownFn := wrapShutdownFuncPanic(s.shutdowns[i])
168+
err = errors.Join(err, shutdownFn(timeoutCtx))
167169
}
168170

169171
return err
@@ -172,6 +174,30 @@ func (s *ShutdownHandler) RunAndWait() error {
172174
return s.eg.Wait()
173175
}
174176

177+
func wrapServeFuncPanic(fn ServeFunc) ServeFunc {
178+
return func() (err error) {
179+
defer func() {
180+
if r := recover(); r != nil {
181+
err = fmt.Errorf("was panic for serve function, recovered value: %v", r)
182+
}
183+
}()
184+
err = fn()
185+
return err
186+
}
187+
}
188+
189+
func wrapShutdownFuncPanic(fn ShutdownFunc) ShutdownFunc {
190+
return func(ctx context.Context) (err error) {
191+
defer func() {
192+
if r := recover(); r != nil {
193+
err = fmt.Errorf("was panic for shutdown function, recovered value: %v", r)
194+
}
195+
}()
196+
err = fn(ctx)
197+
return err
198+
}
199+
}
200+
175201
func runWithCtx(ctx context.Context, fn func() error) error {
176202
var err error
177203

pkg/utils/shutdown_test.go

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package utils
77
import (
88
"context"
99
"errors"
10+
"log"
1011
"os"
1112
"sync"
1213
"testing"
@@ -26,19 +27,24 @@ func newServeShutdownPair(
2627
serveIDs, shutdownIDs *[]int,
2728
mu *sync.Mutex,
2829
serveErr, shutdownErr error,
30+
servePanic bool, shutdownPanic bool,
2931
) *serveShutdownPair {
3032
shutdownTrigger := make(chan struct{}, 1)
3133
s := &serveShutdownPair{
3234
shutdownTrigger: shutdownTrigger,
3335
serve: func() error {
34-
if serveErr == nil {
35-
<-shutdownTrigger
36-
}
37-
3836
mu.Lock()
3937
*serveIDs = append(*serveIDs, fnID)
4038
mu.Unlock()
4139

40+
if servePanic {
41+
log.Panic("Panic!")
42+
}
43+
44+
if serveErr == nil {
45+
<-shutdownTrigger
46+
}
47+
4248
return serveErr
4349
},
4450
shutdown: func(ctx context.Context) error {
@@ -48,38 +54,74 @@ func newServeShutdownPair(
4854

4955
shutdownTrigger <- struct{}{}
5056

57+
if shutdownPanic {
58+
log.Panic("Panic!")
59+
}
60+
5161
return shutdownErr
5262
},
5363
}
5464

5565
return s
5666
}
5767

68+
func errString(err error) string {
69+
if err == nil {
70+
return ""
71+
}
72+
73+
return err.Error()
74+
}
75+
5876
func TestRunAndWait(t *testing.T) {
5977
stubErr := errors.New("stub error")
6078
tests := map[string]struct {
6179
giveServeErr error
80+
giveServePanic bool
6281
giveShutdownErr error
82+
giveShutdownPanic bool
6383
stoppedByInterrupt bool
64-
wantErr error
84+
wantErr string
6585
}{
6686
"all services successfully completed": {
6787
giveServeErr: nil,
88+
giveServePanic: false,
6889
giveShutdownErr: nil,
90+
giveShutdownPanic: false,
6991
stoppedByInterrupt: true,
70-
wantErr: nil,
92+
wantErr: "",
7193
},
7294
"serve failed": {
7395
giveServeErr: stubErr,
96+
giveServePanic: false,
7497
giveShutdownErr: nil,
7598
stoppedByInterrupt: false,
76-
wantErr: stubErr,
99+
giveShutdownPanic: false,
100+
wantErr: stubErr.Error(),
77101
},
78102
"shutdown failed": {
79103
giveServeErr: nil,
104+
giveServePanic: false,
80105
giveShutdownErr: stubErr,
106+
giveShutdownPanic: false,
107+
stoppedByInterrupt: true,
108+
wantErr: stubErr.Error(),
109+
},
110+
"serve panic": {
111+
giveServeErr: nil,
112+
giveServePanic: true,
113+
giveShutdownErr: nil,
114+
giveShutdownPanic: false,
115+
stoppedByInterrupt: false,
116+
wantErr: "was panic for serve function, recovered value: Panic!",
117+
},
118+
"shutdown panic": {
119+
giveServeErr: nil,
120+
giveServePanic: false,
121+
giveShutdownErr: nil,
122+
giveShutdownPanic: true,
81123
stoppedByInterrupt: true,
82-
wantErr: stubErr,
124+
wantErr: "was panic for shutdown function, recovered value: Panic!",
83125
},
84126
}
85127
for testName, tt := range tests {
@@ -89,9 +131,12 @@ func TestRunAndWait(t *testing.T) {
89131
serveFnIDs := &[]int{}
90132
shutdownFnIDs := &[]int{}
91133
mu := sync.Mutex{}
92-
s0 := newServeShutdownPair(0, serveFnIDs, shutdownFnIDs, &mu, nil, nil)
93-
s1 := newServeShutdownPair(1, serveFnIDs, shutdownFnIDs, &mu, tt.giveServeErr, tt.giveShutdownErr)
94-
s2 := newServeShutdownPair(2, serveFnIDs, shutdownFnIDs, &mu, nil, nil)
134+
s0 := newServeShutdownPair(0, serveFnIDs, shutdownFnIDs, &mu, nil, nil, false, false)
135+
s1 := newServeShutdownPair(1, serveFnIDs, shutdownFnIDs, &mu,
136+
tt.giveServeErr, tt.giveShutdownErr,
137+
tt.giveServePanic, tt.giveShutdownPanic,
138+
)
139+
s2 := newServeShutdownPair(2, serveFnIDs, shutdownFnIDs, &mu, nil, nil, false, false)
95140

96141
sh.AddServe(s0.serve, s0.shutdown)
97142
sh.AddServe(s1.serve, s1.shutdown)
@@ -103,7 +148,7 @@ func TestRunAndWait(t *testing.T) {
103148

104149
err := sh.RunAndWait()
105150

106-
if !errors.Is(err, tt.wantErr) {
151+
if errString(err) != tt.wantErr {
107152
t.Errorf("Expected error: %v, received: %v", tt.wantErr, err)
108153
}
109154

0 commit comments

Comments
 (0)