palantir/docker-compose-rule

Throwables other than Exceptions are not propagated to the main thread

jannis-baratheon opened this issue · 0 comments

Type: bug ;; Repro rate: 100%

Repro steps:

  1. Create a HealthCheck which throws some descendant of Throwable which is not an Exception (e.g. an Error - NoSuchMethodError)
  2. Use this HealthCheck in a DockerComposeRule.

Actual result:

The rule fails to start with ConditionTimeoutException.

Expected result:

The rule fails to start with the actual Throwable thrown bt the HealthCheck.

Severity

My HealthCheck was throwing a NoSuchMethodError. It took me days to find out the actual issue behind this was. It was very hard to diagnose as the failure is quite misleading (ConditionTimeoutException) - and when you look at it it actually looks like the code hangs on the line which causes the NoSuchMethodError (also from the IDE).

Cause

DockerComposeRule uses Awaitility to wait for HealthChecks to resolve. Awaitility in turn uses a ScheduledExecutorService which has an infamous feature of swallowing exceptions thrown in the jobs submitted for execution. Awaitility does some work to propagate the errors to the main thread but as it turns out it does it only for Exception descendants, excluding other Throwables (e.g. IncompatibleClassChangeError inheritors which are caused by runtime dependency problems and thus are quite common - NoSuchMethodError, NoSuchFieldError, etc.).

Here's a simple test for this (JUnit + AssertJ):

public class AwaitilityTest {
	
	private class AnError extends Error {}

	@Test
	public void shouldNotTimeOut() {
		assertThatThrownBy(() ->
				Awaitility
						.await()
						.pollInterval(50, TimeUnit.MILLISECONDS)
						.atMost(10, TimeUnit.SECONDS)
						.until(() -> {
							throw new AnError();
						}))
				.isNotInstanceOf(ConditionTimeoutException.class);
	}
}

Solution

  • Upgrade Awaitility to 1.7.0 which copes a bit better with this (it throws an ExecutionException in the case discussed).

  • Or, better, switch to https://github.com/awaitility/awaitility (Jayway's Awaitility is not supported anymore from what I can see).

  • Or do what we chose to do in our code - wrap the HealthCheck and propagate the fault as an Exception:

      public static <T> HealthCheck<T> fixHealthCheck(HealthCheck<T> delegate) {
      	return container -> {
      		try {
      			return delegate.isHealthy(container);
      		} catch (Throwable e) {
      			throw new RuntimeException(
      					"Unexpected exception occurred while performing health check.",
      					e);
      		}
      	};
      }