mirror of https://github.com/grpc/grpc-java.git
Strip cause from InProcessTransport between client and server. Fixes #1716
This commit is contained in:
parent
f7dc4d2cc6
commit
46eefe34fb
|
@ -319,6 +319,7 @@ class InProcessTransport implements ServerTransport, ManagedClientTransport {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void close(Status status, Metadata trailers) {
|
public void close(Status status, Metadata trailers) {
|
||||||
|
status = stripCause(status);
|
||||||
synchronized (this) {
|
synchronized (this) {
|
||||||
if (closed) {
|
if (closed) {
|
||||||
return;
|
return;
|
||||||
|
@ -461,7 +462,7 @@ class InProcessTransport implements ServerTransport, ManagedClientTransport {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void cancel(Status reason) {
|
public void cancel(Status reason) {
|
||||||
if (!internalCancel(reason)) {
|
if (!internalCancel(stripCause(reason))) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
serverStream.clientCancelled(reason);
|
serverStream.clientCancelled(reason);
|
||||||
|
@ -525,4 +526,20 @@ class InProcessTransport implements ServerTransport, ManagedClientTransport {
|
||||||
public void setDecompressor(Decompressor decompressor) {}
|
public void setDecompressor(Decompressor decompressor) {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns a new status with the same code and description, but stripped of any other information
|
||||||
|
* (i.e. cause).
|
||||||
|
*
|
||||||
|
* <p>This is, so that the InProcess transport behaves in the same way as the other transports,
|
||||||
|
* when exchanging statuses between client and server and vice versa.
|
||||||
|
*/
|
||||||
|
private static Status stripCause(Status status) {
|
||||||
|
if (status == null) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return Status
|
||||||
|
.fromCodeValue(status.getCode().value())
|
||||||
|
.withDescription(status.getDescription());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -35,6 +35,7 @@ import static com.google.common.base.Charsets.UTF_8;
|
||||||
import static org.junit.Assert.assertEquals;
|
import static org.junit.Assert.assertEquals;
|
||||||
import static org.junit.Assert.assertFalse;
|
import static org.junit.Assert.assertFalse;
|
||||||
import static org.junit.Assert.assertNotEquals;
|
import static org.junit.Assert.assertNotEquals;
|
||||||
|
import static org.junit.Assert.assertNull;
|
||||||
import static org.junit.Assert.assertTrue;
|
import static org.junit.Assert.assertTrue;
|
||||||
import static org.junit.Assert.fail;
|
import static org.junit.Assert.fail;
|
||||||
import static org.junit.Assume.assumeTrue;
|
import static org.junit.Assume.assumeTrue;
|
||||||
|
@ -492,13 +493,15 @@ public abstract class AbstractTransportTest {
|
||||||
serverStream.writeHeaders(new Metadata());
|
serverStream.writeHeaders(new Metadata());
|
||||||
verify(mockClientStreamListener, timeout(TIMEOUT_MS)).headersRead(any(Metadata.class));
|
verify(mockClientStreamListener, timeout(TIMEOUT_MS)).headersRead(any(Metadata.class));
|
||||||
|
|
||||||
Status status = Status.OK.withDescription("Hello. Goodbye.");
|
Status status = Status.OK.withDescription("Hello. Goodbye.").withCause(new Exception());
|
||||||
serverStream.close(status, new Metadata());
|
serverStream.close(status, new Metadata());
|
||||||
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
||||||
assertCodeEquals(Status.OK, statusCaptor.getValue());
|
assertCodeEquals(Status.OK, statusCaptor.getValue());
|
||||||
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
||||||
.closed(statusCaptor.capture(), any(Metadata.class));
|
.closed(statusCaptor.capture(), any(Metadata.class));
|
||||||
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
||||||
|
assertEquals("Hello. Goodbye.", statusCaptor.getValue().getDescription());
|
||||||
|
assertNull(statusCaptor.getValue().getCause());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -516,7 +519,7 @@ public abstract class AbstractTransportTest {
|
||||||
ServerStream serverStream = serverStreamCreation.stream;
|
ServerStream serverStream = serverStreamCreation.stream;
|
||||||
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
||||||
|
|
||||||
Status status = Status.OK.withDescription("Hellogoodbye");
|
Status status = Status.OK.withDescription("Hellogoodbye").withCause(new Exception());
|
||||||
Metadata trailers = new Metadata();
|
Metadata trailers = new Metadata();
|
||||||
trailers.put(asciiKey, "trailers");
|
trailers.put(asciiKey, "trailers");
|
||||||
trailers.put(asciiKey, "dupvalue");
|
trailers.put(asciiKey, "dupvalue");
|
||||||
|
@ -528,6 +531,9 @@ public abstract class AbstractTransportTest {
|
||||||
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
||||||
.closed(statusCaptor.capture(), metadataCaptor.capture());
|
.closed(statusCaptor.capture(), metadataCaptor.capture());
|
||||||
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
||||||
|
assertEquals("Hellogoodbye", statusCaptor.getValue().getDescription());
|
||||||
|
// Cause should not be transmitted to the client.
|
||||||
|
assertNull(statusCaptor.getValue().getCause());
|
||||||
assertEquals(Lists.newArrayList(trailers.getAll(asciiKey)),
|
assertEquals(Lists.newArrayList(trailers.getAll(asciiKey)),
|
||||||
Lists.newArrayList(metadataCaptor.getValue().getAll(asciiKey)));
|
Lists.newArrayList(metadataCaptor.getValue().getAll(asciiKey)));
|
||||||
assertEquals(Lists.newArrayList(trailers.getAll(binaryKey)),
|
assertEquals(Lists.newArrayList(trailers.getAll(binaryKey)),
|
||||||
|
@ -549,7 +555,8 @@ public abstract class AbstractTransportTest {
|
||||||
ServerStream serverStream = serverStreamCreation.stream;
|
ServerStream serverStream = serverStreamCreation.stream;
|
||||||
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
||||||
|
|
||||||
Status status = Status.INVALID_ARGUMENT.withDescription("I'm not listening");
|
Status status =
|
||||||
|
Status.INVALID_ARGUMENT.withDescription("I'm not listening").withCause(new Exception());
|
||||||
serverStream.close(status, new Metadata());
|
serverStream.close(status, new Metadata());
|
||||||
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
||||||
assertCodeEquals(Status.OK, statusCaptor.getValue());
|
assertCodeEquals(Status.OK, statusCaptor.getValue());
|
||||||
|
@ -557,6 +564,7 @@ public abstract class AbstractTransportTest {
|
||||||
.closed(statusCaptor.capture(), any(Metadata.class));
|
.closed(statusCaptor.capture(), any(Metadata.class));
|
||||||
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
assertEquals(status.getCode(), statusCaptor.getValue().getCode());
|
||||||
assertEquals(status.getDescription(), statusCaptor.getValue().getDescription());
|
assertEquals(status.getDescription(), statusCaptor.getValue().getDescription());
|
||||||
|
assertNull(statusCaptor.getValue().getCause());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
@ -573,12 +581,14 @@ public abstract class AbstractTransportTest {
|
||||||
= serverTransportListener.takeStreamOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
|
= serverTransportListener.takeStreamOrFail(TIMEOUT_MS, TimeUnit.MILLISECONDS);
|
||||||
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
||||||
|
|
||||||
Status status = Status.CANCELLED.withDescription("Nevermind");
|
Status status = Status.CANCELLED.withDescription("Nevermind").withCause(new Exception());
|
||||||
clientStream.cancel(status);
|
clientStream.cancel(status);
|
||||||
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
||||||
.closed(Matchers.same(status), any(Metadata.class));
|
.closed(Matchers.same(status), any(Metadata.class));
|
||||||
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(statusCaptor.capture());
|
||||||
assertNotEquals(Status.Code.OK, statusCaptor.getValue().getCode());
|
assertNotEquals(Status.Code.OK, statusCaptor.getValue().getCode());
|
||||||
|
// Cause should not be transmitted between client and server
|
||||||
|
assertNull(statusCaptor.getValue().getCause());
|
||||||
|
|
||||||
reset(mockServerStreamListener);
|
reset(mockServerStreamListener);
|
||||||
reset(mockClientStreamListener);
|
reset(mockClientStreamListener);
|
||||||
|
@ -655,7 +665,8 @@ public abstract class AbstractTransportTest {
|
||||||
ServerStream serverStream = serverStreamCreation.stream;
|
ServerStream serverStream = serverStreamCreation.stream;
|
||||||
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
ServerStreamListener mockServerStreamListener = serverStreamCreation.listener;
|
||||||
|
|
||||||
Status status = Status.DEADLINE_EXCEEDED.withDescription("It was bound to happen");
|
Status status = Status.DEADLINE_EXCEEDED.withDescription("It was bound to happen")
|
||||||
|
.withCause(new Exception());
|
||||||
serverStream.cancel(status);
|
serverStream.cancel(status);
|
||||||
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(Matchers.same(status));
|
verify(mockServerStreamListener, timeout(TIMEOUT_MS)).closed(Matchers.same(status));
|
||||||
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
verify(mockClientStreamListener, timeout(TIMEOUT_MS))
|
||||||
|
@ -663,6 +674,8 @@ public abstract class AbstractTransportTest {
|
||||||
// Presently we can't sent much back to the client in this case. Verify that is the current
|
// Presently we can't sent much back to the client in this case. Verify that is the current
|
||||||
// behavior for consistency between transports.
|
// behavior for consistency between transports.
|
||||||
assertCodeEquals(Status.CANCELLED, statusCaptor.getValue());
|
assertCodeEquals(Status.CANCELLED, statusCaptor.getValue());
|
||||||
|
// Cause should not be transmitted between server and client
|
||||||
|
assertNull(statusCaptor.getValue().getCause());
|
||||||
|
|
||||||
// Second cancellation shouldn't trigger additional callbacks
|
// Second cancellation shouldn't trigger additional callbacks
|
||||||
reset(mockServerStreamListener);
|
reset(mockServerStreamListener);
|
||||||
|
|
Loading…
Reference in New Issue