Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throwables other than Exceptions are not propagated to the main thread #250

Open
jannis-baratheon opened this issue Sep 17, 2018 · 0 comments · May be fixed by #252
Open

Throwables other than Exceptions are not propagated to the main thread #250

jannis-baratheon opened this issue Sep 17, 2018 · 0 comments · May be fixed by #252

Comments

@jannis-baratheon
Copy link

jannis-baratheon commented Sep 17, 2018

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);
      		}
      	};
      }
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant