Allow application to pass cancellation details.

Resolves #1221

Add ClientCall.cancel(String, Throwable) and deprecate
ClientCall.cancel(). Will delete cancel() after all known third-party
overriders have switched to overriding the new one.
This commit is contained in:
Kun Zhang 2016-04-21 16:10:35 -07:00
parent 38a91f83e1
commit 16e168951a
10 changed files with 58 additions and 18 deletions

View File

@ -182,12 +182,12 @@ public class ClientAuthInterceptorTests {
interceptedCall = interceptor.interceptCall(descriptor, CallOptions.DEFAULT, channel); interceptedCall = interceptor.interceptCall(descriptor, CallOptions.DEFAULT, channel);
interceptedCall.start(listener, new Metadata()); interceptedCall.start(listener, new Metadata());
verify(credentials).getRequestMetadata(URI.create("https://example.com/a.service")); verify(credentials).getRequestMetadata(URI.create("https://example.com/a.service"));
interceptedCall.cancel(); interceptedCall.cancel("Cancel for test", null);
doReturn("example.com:123").when(channel).authority(); doReturn("example.com:123").when(channel).authority();
interceptedCall = interceptor.interceptCall(descriptor, CallOptions.DEFAULT, channel); interceptedCall = interceptor.interceptCall(descriptor, CallOptions.DEFAULT, channel);
interceptedCall.start(listener, new Metadata()); interceptedCall.start(listener, new Metadata());
verify(credentials).getRequestMetadata(URI.create("https://example.com:123/a.service")); verify(credentials).getRequestMetadata(URI.create("https://example.com:123/a.service"));
interceptedCall.cancel(); interceptedCall.cancel("Cancel for test", null);
} }
} }

View File

@ -515,7 +515,7 @@ class LoadClient {
call.sendMessage(genericRequest.slice()); call.sendMessage(genericRequest.slice());
now = System.nanoTime(); now = System.nanoTime();
if (shutdown) { if (shutdown) {
call.cancel(); call.cancel("Shutting down", null);
} }
} }

View File

@ -31,6 +31,8 @@
package io.grpc; package io.grpc;
import javax.annotation.Nullable;
/** /**
* An instance of a call to to a remote method. A call will send zero or more * An instance of a call to to a remote method. A call will send zero or more
* request messages to the server and receive zero or more response messages back. * request messages to the server and receive zero or more response messages back.
@ -156,16 +158,35 @@ public abstract class ClientCall<ReqT, RespT> {
*/ */
public abstract void request(int numMessages); public abstract void request(int numMessages);
/**
* Equivalent as {@link #cancel(String, Throwable)} without passing any useful information.
*
* @deprecated Use or override {@link #cancel(String, Throwable)} instead. See
* https://github.com/grpc/grpc-java/issues/1221
*/
@Deprecated
public void cancel() {
cancel("Cancelled by ClientCall.cancel()", null);
}
/** /**
* Prevent any further processing for this {@code ClientCall}. No further messages may be sent or * Prevent any further processing for this {@code ClientCall}. No further messages may be sent or
* will be received. The server is informed of cancellations, but may not stop processing the * will be received. The server is informed of cancellations, but may not stop processing the
* call. Cancellation is permitted if previously {@link #halfClose}d. Cancelling an already {@code * call. Cancellation is permitted if previously {@link #halfClose}d. Cancelling an already {@code
* cancel()}ed {@code ClientCall} has no effect. * cancel()}ed {@code ClientCall} has no effect.
* *
*
* <p>No other methods on this class can be called after this method has been called. * <p>No other methods on this class can be called after this method has been called.
*
* <p>It is recommended that at least one of the arguments to be non-{@code null}, to provide
* useful debug information. Both argument being null may log warnings and result in suboptimal
* performance. Also note that the provided information will not be sent to the server.
*
* @param message if not {@code null}, will appear as the description of the CANCELLED status
* @param cause if not {@code null}, will appear as the cause of the CANCELLED status
*/ */
public abstract void cancel(); public void cancel(@Nullable String message, @Nullable Throwable cause) {
throw new UnsupportedOperationException(getClass() + " should implement this method");
}
/** /**
* Close the call for request message sending. Incoming response messages are unaffected. This * Close the call for request message sending. Incoming response messages are unaffected. This

View File

@ -134,7 +134,7 @@ public class ClientInterceptors {
public void request(int numMessages) {} public void request(int numMessages) {}
@Override @Override
public void cancel() {} public void cancel(String message, Throwable cause) {}
@Override @Override
public void halfClose() {} public void halfClose() {}

View File

@ -31,6 +31,8 @@
package io.grpc; package io.grpc;
import javax.annotation.Nullable;
/** /**
* A {@link ClientCall} which forwards all of it's methods to another {@link ClientCall}. * A {@link ClientCall} which forwards all of it's methods to another {@link ClientCall}.
*/ */
@ -50,11 +52,17 @@ public abstract class ForwardingClientCall<ReqT, RespT> extends ClientCall<ReqT,
delegate().request(numMessages); delegate().request(numMessages);
} }
@Deprecated
@Override @Override
public void cancel() { public void cancel() {
delegate().cancel(); delegate().cancel();
} }
@Override
public void cancel(@Nullable String message, @Nullable Throwable cause) {
delegate().cancel(message, cause);
}
@Override @Override
public void halfClose() { public void halfClose() {
delegate().halfClose(); delegate().halfClose();

View File

@ -304,7 +304,7 @@ final class ClientCallImpl<ReqT, RespT> extends ClientCall<ReqT, RespT>
} }
@Override @Override
public void cancel() { public void cancel(@Nullable String message, @Nullable Throwable cause) {
if (cancelCalled) { if (cancelCalled) {
return; return;
} }
@ -313,8 +313,20 @@ final class ClientCallImpl<ReqT, RespT> extends ClientCall<ReqT, RespT>
// Cancel is called in exception handling cases, so it may be the case that the // Cancel is called in exception handling cases, so it may be the case that the
// stream was never successfully created. // stream was never successfully created.
if (stream != null) { if (stream != null) {
stream.cancel(Status.CANCELLED.withCause( Status status = Status.CANCELLED;
new CancellationException("Client requested cancellation"))); if (message != null) {
status = status.withDescription(message);
}
if (cause != null) {
status = status.withCause(cause);
}
if (message == null && cause == null) {
// TODO(zhangkun83): log a warning with this exception once cancel() has been deleted from
// ClientCall.
status = status.withCause(
new CancellationException("Client called cancel() without any detail"));
}
stream.cancel(status);
} }
} finally { } finally {
if (context != null) { if (context != null) {

View File

@ -399,7 +399,7 @@ public class ClientInterceptorsTest {
// Make sure nothing bad happens after the exception. // Make sure nothing bad happens after the exception.
ClientCall<?, ?> noop = ((CheckedForwardingClientCall<?, ?>)interceptedCall).delegate(); ClientCall<?, ?> noop = ((CheckedForwardingClientCall<?, ?>)interceptedCall).delegate();
// Should not throw, even on bad input // Should not throw, even on bad input
noop.cancel(); noop.cancel("Cancel for test", null);
noop.start(null, null); noop.start(null, null);
noop.request(-1); noop.request(-1);
noop.halfClose(); noop.halfClose();

View File

@ -289,7 +289,7 @@ public class ManagedChannelImplTest {
ClientStream mockStream = mock(ClientStream.class); ClientStream mockStream = mock(ClientStream.class);
when(mockTransport.newStream(same(method), same(headers))).thenReturn(mockStream); when(mockTransport.newStream(same(method), same(headers))).thenReturn(mockStream);
call.start(mockCallListener, headers); call.start(mockCallListener, headers);
call.cancel(); call.cancel("Cancel for test", null);
verify(mockTransport, timeout(1000)).start(transportListenerCaptor.capture()); verify(mockTransport, timeout(1000)).start(transportListenerCaptor.capture());
final ManagedClientTransport.Listener transportListener = transportListenerCaptor.getValue(); final ManagedClientTransport.Listener transportListener = transportListenerCaptor.getValue();

View File

@ -113,7 +113,7 @@ public class ClientCalls {
try { try {
return getUnchecked(futureUnaryCall(call, param)); return getUnchecked(futureUnaryCall(call, param));
} catch (Throwable t) { } catch (Throwable t) {
call.cancel(); call.cancel(null, t);
throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t); throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t);
} }
} }
@ -139,7 +139,7 @@ public class ClientCalls {
} }
return getUnchecked(responseFuture); return getUnchecked(responseFuture);
} catch (Throwable t) { } catch (Throwable t) {
call.cancel(); call.cancel(null, t);
throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t); throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t);
} }
} }
@ -226,7 +226,7 @@ public class ClientCalls {
call.sendMessage(param); call.sendMessage(param);
call.halfClose(); call.halfClose();
} catch (Throwable t) { } catch (Throwable t) {
call.cancel(); call.cancel(null, t);
throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t); throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t);
} }
} }
@ -265,8 +265,7 @@ public class ClientCalls {
@Override @Override
public void onError(Throwable t) { public void onError(Throwable t) {
// TODO(ejona86): log? call.cancel("Cancelled by client with StreamObserver.onError()", t);
call.cancel();
} }
@Override @Override
@ -368,7 +367,7 @@ public class ClientCalls {
@Override @Override
protected void interruptTask() { protected void interruptTask() {
call.cancel(); call.cancel("GrpcFuture was cancelled", null);
} }
@Override @Override

View File

@ -101,7 +101,7 @@ public class ClientCallsTest {
verify(call).start(listenerCaptor.capture(), any(Metadata.class)); verify(call).start(listenerCaptor.capture(), any(Metadata.class));
ClientCall.Listener<String> listener = listenerCaptor.getValue(); ClientCall.Listener<String> listener = listenerCaptor.getValue();
future.cancel(true); future.cancel(true);
verify(call).cancel(); verify(call).cancel("GrpcFuture was cancelled", null);
listener.onMessage("bar"); listener.onMessage("bar");
listener.onClose(Status.OK, new Metadata()); listener.onClose(Status.OK, new Metadata());
try { try {