Skip to content

Commit

Permalink
Consistently handles fatal errors in catch Throwable blocks (#1494)
Browse files Browse the repository at this point in the history
I noticed we weren't consistent when catching Throwable.
  • Loading branch information
adriancole authored Jan 18, 2017
1 parent 81d67fc commit cf4df43
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import zipkin.server.ConditionalOnSelfTracing;
import zipkin.storage.StorageComponent;

import static zipkin.internal.Util.propagateIfFatal;

@Configuration
@ConditionalOnSelfTracing
@Import(ApiTracerConfiguration.class)
Expand Down Expand Up @@ -140,9 +142,9 @@ static final class LocalSender implements Sender {
spans.add(Codec.THRIFT.readSpan(encodedSpan));
}
delegate.asyncSpanConsumer().accept(spans, new CallbackAdapter(callback));
} catch (Throwable e) {
callback.onError(e);
if (e instanceof Error) throw (Error) e;
} catch (Throwable t) {
propagateIfFatal(t);
callback.onError(t);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -20,6 +20,8 @@
import okhttp3.Response;
import okhttp3.ResponseBody;

import static zipkin.internal.Util.propagateIfFatal;

class CallbackListenableFuture<V> extends AbstractFuture<V> implements okhttp3.Callback {
final Call call;

Expand Down Expand Up @@ -54,17 +56,6 @@ ListenableFuture<V> enqueue() {
}
}

// Taken from RxJava, which was taken from scala
static void propagateIfFatal(Throwable t) {
if (t instanceof VirtualMachineError) {
throw (VirtualMachineError) t;
} else if (t instanceof ThreadDeath) {
throw (ThreadDeath) t;
} else if (t instanceof LinkageError) {
throw (LinkageError) t;
}
}

V convert(ResponseBody responseBody) throws IOException {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -25,6 +25,7 @@

import static com.google.common.base.Preconditions.checkState;
import static org.apache.curator.framework.CuratorFrameworkFactory.newClient;
import static zipkin.internal.Util.propagateIfFatal;

final class ZooKeeperRule implements TestRule {
TestingCluster cluster;
Expand All @@ -48,9 +49,10 @@ public void evaluate() throws Throwable {
try {
doEvaluate(base);
break;
} catch (Throwable e) {
} catch (Throwable t) {
propagateIfFatal(t);
if (i == 3) {
throw e;
throw t;
}
Thread.sleep(1000);
}
Expand Down
13 changes: 12 additions & 1 deletion zipkin/src/main/java/zipkin/internal/Util.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down Expand Up @@ -163,6 +163,17 @@ public static void writeHexLong(char[] data, int pos, long v) {
writeHexByte(data, pos + 14, (byte) (v & 0xff));
}

// Taken from RxJava throwIfFatal, which was taken from scala
public static void propagateIfFatal(Throwable t) {
if (t instanceof VirtualMachineError) {
throw (VirtualMachineError) t;
} else if (t instanceof ThreadDeath) {
throw (ThreadDeath) t;
} else if (t instanceof LinkageError) {
throw (LinkageError) t;
}
}

static final char[] HEX_DIGITS =
{'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -14,6 +14,7 @@
package zipkin.storage;

import static zipkin.internal.Util.checkNotNull;
import static zipkin.internal.Util.propagateIfFatal;

abstract class InternalCallbackRunnable<V> implements Runnable {
final Callback<V> callback;
Expand All @@ -27,9 +28,9 @@ protected InternalCallbackRunnable(Callback<V> callback) {
@Override public void run() {
try {
callback.onSuccess(complete());
} catch (Throwable e) {
callback.onError(e);
if (e instanceof Error) throw (Error) e;
} catch (Throwable t) {
propagateIfFatal(t);
callback.onError(t);
}
}
}

0 comments on commit cf4df43

Please sign in to comment.