Skip to content

Commit b12ee6d

Browse files
GODRIVER-2432 Improve panic handling in background processes (#1471)
1 parent 3f6e80a commit b12ee6d

File tree

3 files changed

+112
-11
lines changed

3 files changed

+112
-11
lines changed

x/mongo/driver/topology/server.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -549,9 +549,7 @@ func (s *Server) update() {
549549
checkNow := s.checkNow
550550
done := s.done
551551

552-
defer func() {
553-
_ = recover()
554-
}()
552+
defer logUnexpectedFailure(s.cfg.logger, "Encountered unexpected failure updating server")
555553

556554
closeServer := func() {
557555
s.subLock.Lock()
@@ -683,10 +681,7 @@ func (s *Server) updateDescription(desc description.Server) {
683681
return
684682
}
685683

686-
defer func() {
687-
// ¯\_(ツ)_/¯
688-
_ = recover()
689-
}()
684+
defer logUnexpectedFailure(s.cfg.logger, "Encountered unexpected failure updating server description")
690685

691686
// Anytime we update the server description to something other than "unknown", set the pool to
692687
// "ready". Do this before updating the description so that connections can be checked out as

x/mongo/driver/topology/topology.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,32 @@ func logServerSelectionFailed(
269269
logger.KeyFailure, err.Error())
270270
}
271271

272+
// logUnexpectedFailure is a defer-recover function for logging unexpected
273+
// failures encountered while maintaining a topology.
274+
//
275+
// Most topology maintenance actions, such as updating a server, should not take
276+
// down a client's application. This function provides a best-effort to log
277+
// unexpected failures. If the logger passed to this function is nil, then the
278+
// recovery will be silent.
279+
func logUnexpectedFailure(log *logger.Logger, msg string, callbacks ...func()) {
280+
r := recover()
281+
if r == nil {
282+
return
283+
}
284+
285+
defer func() {
286+
for _, clbk := range callbacks {
287+
clbk()
288+
}
289+
}()
290+
291+
if log == nil {
292+
return
293+
}
294+
295+
log.Print(logger.LevelInfo, logger.ComponentTopology, fmt.Sprintf("%s: %v", msg, r))
296+
}
297+
272298
// Connect initializes a Topology and starts the monitoring process. This function
273299
// must be called to properly monitor the topology.
274300
func (t *Topology) Connect() error {
@@ -768,12 +794,11 @@ func (t *Topology) pollSRVRecords(hosts string) {
768794
defer pollTicker.Stop()
769795
t.pollHeartbeatTime.Store(false)
770796
var doneOnce bool
771-
defer func() {
772-
// ¯\_(ツ)_/¯
773-
if r := recover(); r != nil && !doneOnce {
797+
defer logUnexpectedFailure(t.cfg.logger, "Encountered unexpected failure polling SRV records", func() {
798+
if !doneOnce {
774799
<-t.pollingDone
775800
}
776-
}()
801+
})
777802

778803
for {
779804
select {

x/mongo/driver/topology/topology_test.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
package topology
88

99
import (
10+
"bufio"
11+
"bytes"
1012
"context"
1113
"encoding/json"
1214
"errors"
@@ -19,6 +21,7 @@ import (
1921

2022
"go.mongodb.org/mongo-driver/bson/primitive"
2123
"go.mongodb.org/mongo-driver/internal/assert"
24+
"go.mongodb.org/mongo-driver/internal/logger"
2225
"go.mongodb.org/mongo-driver/internal/require"
2326
"go.mongodb.org/mongo-driver/internal/spectest"
2427
"go.mongodb.org/mongo-driver/mongo/address"
@@ -1195,3 +1198,81 @@ func BenchmarkSelectServerFromDescription(b *testing.B) {
11951198
})
11961199
}
11971200
}
1201+
1202+
func TestLogUnexpectedFailure(t *testing.T) {
1203+
t.Parallel()
1204+
1205+
// newIOLogger will log data using an io sink.
1206+
newIOLogger := func() (*logger.Logger, *bytes.Buffer, *bufio.Writer) {
1207+
buf := bytes.NewBuffer(nil)
1208+
w := bufio.NewWriter(buf)
1209+
1210+
ioSink := logger.NewIOSink(w)
1211+
1212+
ioLogger, err := logger.New(ioSink, logger.DefaultMaxDocumentLength, map[logger.Component]logger.Level{
1213+
logger.ComponentTopology: logger.LevelDebug,
1214+
})
1215+
1216+
assert.NoError(t, err)
1217+
1218+
return ioLogger, buf, w
1219+
}
1220+
1221+
// newNilLogger will return a nil logger with empty buffer and writer.
1222+
newNilLogger := func() (*logger.Logger, *bytes.Buffer, *bufio.Writer) {
1223+
return nil, &bytes.Buffer{}, &bufio.Writer{}
1224+
}
1225+
1226+
tests := []struct {
1227+
name string
1228+
msg string
1229+
newLogger func() (*logger.Logger, *bytes.Buffer, *bufio.Writer)
1230+
panicValue interface{}
1231+
want interface{} // Either a string or nil
1232+
}{
1233+
{
1234+
name: "nil logger",
1235+
msg: "",
1236+
newLogger: newNilLogger,
1237+
panicValue: 1,
1238+
want: nil,
1239+
},
1240+
{
1241+
name: "valid logger",
1242+
msg: "test",
1243+
newLogger: newIOLogger,
1244+
panicValue: 1,
1245+
want: "test: 1",
1246+
},
1247+
{
1248+
name: "valid logger with error panic",
1249+
msg: "test",
1250+
newLogger: newIOLogger,
1251+
panicValue: errors.New("err"),
1252+
want: "test: err",
1253+
},
1254+
}
1255+
1256+
for _, test := range tests {
1257+
test := test
1258+
1259+
t.Run(test.name, func(t *testing.T) {
1260+
t.Parallel()
1261+
1262+
log, buf, w := test.newLogger()
1263+
1264+
func() {
1265+
defer logUnexpectedFailure(log, test.msg)
1266+
1267+
panic(test.panicValue)
1268+
}()
1269+
1270+
assert.NoError(t, w.Flush())
1271+
1272+
got := map[string]interface{}{}
1273+
_ = json.Unmarshal(buf.Bytes(), &got)
1274+
1275+
assert.Equal(t, test.want, got[logger.KeyMessage])
1276+
})
1277+
}
1278+
}

0 commit comments

Comments
 (0)