fix: shutdown on uncaught exception #73
Conversation
| context.addServlet(new ServletHolder(new JVMDiagnosticServlet()), "/diags/*"); | ||
|
|
||
| final Thread thread = new Thread(this::doStart); | ||
| thread.setUncaughtExceptionHandler( |
There was a problem hiding this comment.
The potential risk here is that there may be some exceptions that are ignorable today and uncaught that would lead to a server quit. IMO we should be fixing any such spots (And kube will recover for us in the interim)
| try { | ||
| this.shutdown(); | ||
| } catch (Exception e) { | ||
| // Ignore if failed to shut down |
There was a problem hiding this comment.
Potentially. The shutdown method calls various shutdown hooks, which each service is able to define itself. If any happen to throw (which seems like it's possible especially given it can now be called before startup), we don't want them to actually prevent the shutdown from completing.
There was a problem hiding this comment.
Log or it would be too noisy?
There was a problem hiding this comment.
Not a noise volume concern, i was just concerned it would be misleading. Basically something has happened to cause the server to shutdown abnormally. If we hit more errors when trying to force that shutdown (from a server that may not have started in the first place), we don't want those to distract from the root cause.
Codecov Report
@@ Coverage Diff @@
## main #73 +/- ##
============================================
- Coverage 70.57% 70.27% -0.31%
Complexity 106 106
============================================
Files 15 15
Lines 588 592 +4
Branches 32 32
============================================
+ Hits 415 416 +1
- Misses 154 157 +3
Partials 19 19
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Description
Currently if the server dies due to an uncaught exception, it's logged and the process goes on. Updating so uncaught exceptions cause process to quit.