-
Type: Improvement
-
Resolution: Fixed
-
Priority: Unknown
-
Affects Version/s: None
-
Component/s: None
There is a mixture of defer recover blocks without a clear purpose and possibly unhandled panics in background goroutines in the Go driver (e.g. GODRIVER-2438). Improve the use of "defer recover" blocks in the Go driver to prevent recoverable panics from unnecessarily propagating, prevent hiding panics by exposing recovered panics, and add tests and documentation that exercise and describe the possible panic and recovery conditions.
There are currently 3 "defer recover" blocks in the Go Driver that do not have a clear purpose or comment and discard the caught error. That means there are possibly panics happening in the Go Driver that are caught and ignored, increasing the risk of undiscovered driver bugs or unexpected behavior.
Defer recover blocks and associated Github/Gerrithub history:
- server.go#L477-485
- Commit: Changed in a20a92e8, introduced in 3ee55f09
- Ticket:
GODRIVER-285 - Review: https://review.gerrithub.io/c/mongodb/mongo-go-driver/+/408109/
- server.go#L590-L593
- Commit: 3ee55f09
- Ticket: ?
- Review: ?
- topology.go#L566-L571
- Commit: 48e91ca9
- Ticket:
GODRIVER-625 - Review: https://review.gerrithub.io/c/mongodb/mongo-go-driver/+/450676/
All of that context doesn't provide any answers about why those defer recover blocks were added. The surrounding code has been changed significantly so they may no longer be necessary. A recent draft PR with the defer recover blocks removed passes all tests, but it's still possible the called code could panic due to some untested condition. We should collect more information about why the defer recover blocks were added, then remove them if possible.
Definition of done:
- Collect more historical context to understand why the defer recover blocks were added.
- Add tests to cover any discovered untested panic conditions.
- Add or move "defer recover" blocks that are necessary to handle panics in background goroutines. Include detailed comments describing what panic conditions we're expecting to handle.
- Remove unnecessary "defer recover" blocks.
- Fix any discovered panics that are bugs in the Go driver.
- is related to
-
GODRIVER-1439 Only close heartbeat connections after they're opened
- Closed
-
GODRIVER-2438 Pool "closeConnection" method cause panics on removePerishedConns()
- Closed
-
GODRIVER-198 Ensure panics are handled when running functions in goroutines
- Closed
-
GODRIVER-204 Eliminate internal use of panic() wherever possible
- Closed