Describe the bug
The com.spotify.metrics.core.InstrumentedScheduledExecutorService wraps submitted tasks (Runnable/Callable) into internal wrapper classes (e.g., InstrumentedPeriodicRunnable, InstrumentedRunnable) to track metrics like duration and execution count.
However, these wrapper classes use a try-finally block to record metrics but do not catch exceptions thrown by the underlying task.
According to the JDK ScheduledExecutorService contract (specifically for scheduleAtFixedRate and scheduleWithFixedDelay), if a task throws an uncaught exception, all future executions of that task are suppressed.
Because the wrapper propagates the exception without logging or trapping it, a single transient failure in a scheduled task causes the scheduler to silently stop executing that task forever. This is a critical "Exception Not Caught" (ENC) issue for monitoring infrastructure, as the metrics collection itself will stop without warning.
To Reproduce
- Wrap a
ScheduledExecutorService with InstrumentedScheduledExecutorService.
- Schedule a periodic task (e.g., using
scheduleAtFixedRate) that throws a RuntimeException.
- Observe that the task runs once (throwing the exception) and then never runs again.
- No error logs regarding the scheduler death are produced by the wrapper.
Expected behavior
The wrapper should ideally catch Throwable, log the error to ensure visibility, and potentially suppress the exception (depending on the desired policy for metric collection resilience) to allow the ScheduledExecutorService to continue triggering future executions.
At the very least, if re-throwing is intended, it should log the error before propagating it, so operators know why metrics stopped reporting.
Root Cause Analysis
In InstrumentedScheduledExecutorService.java, inner class InstrumentedPeriodicRunnable:
@Override
public void run() {
running.inc();
final Timer.Context context = duration.time();
try {
command.run(); // <--- If this throws, execution jumps to finally
} finally {
final long elapsed = context.stop();
// ... metrics updates ...
}
// <--- Exception continues to propagate here, killing the schedule
}
Describe the bug
The
com.spotify.metrics.core.InstrumentedScheduledExecutorServicewraps submitted tasks (Runnable/Callable) into internal wrapper classes (e.g.,InstrumentedPeriodicRunnable,InstrumentedRunnable) to track metrics like duration and execution count.However, these wrapper classes use a
try-finallyblock to record metrics but do not catch exceptions thrown by the underlying task.According to the JDK
ScheduledExecutorServicecontract (specifically forscheduleAtFixedRateandscheduleWithFixedDelay), if a task throws an uncaught exception, all future executions of that task are suppressed.Because the wrapper propagates the exception without logging or trapping it, a single transient failure in a scheduled task causes the scheduler to silently stop executing that task forever. This is a critical "Exception Not Caught" (ENC) issue for monitoring infrastructure, as the metrics collection itself will stop without warning.
To Reproduce
ScheduledExecutorServicewithInstrumentedScheduledExecutorService.scheduleAtFixedRate) that throws aRuntimeException.Expected behavior
The wrapper should ideally catch
Throwable, log the error to ensure visibility, and potentially suppress the exception (depending on the desired policy for metric collection resilience) to allow theScheduledExecutorServiceto continue triggering future executions.At the very least, if re-throwing is intended, it should log the error before propagating it, so operators know why metrics stopped reporting.
Root Cause Analysis
In
InstrumentedScheduledExecutorService.java, inner classInstrumentedPeriodicRunnable: