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

Inconsistent behaviour when throwing StatusRuntimeException. #1158

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add tests throwing RuntimeException in service calls
skjolber committed Jan 9, 2025
commit 5b6a7812f2a5a59991082c4f7a6e82594313d7fe
Original file line number Diff line number Diff line change
@@ -74,7 +74,11 @@ DynamicTestCollection unprotectedCallTests() {
.add("unprotected-default",
() -> assertNormalCallSuccess(this.serviceStub, this.blockingStub, this.futureStub))
.add("unprotected-noPerm",
() -> assertNormalCallSuccess(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub));
() -> assertNormalCallSuccess(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub))
.add("unprotected-runtimeException-default",
() -> assertStatusRuntimeExceptionCallFailure(this.serviceStub, this.blockingStub,
this.futureStub,
PERMISSION_DENIED));
}

protected void assertNormalCallSuccess(final TestServiceStub serviceStub,
@@ -96,6 +100,16 @@ protected void assertNormalCallFailure(final TestServiceStub serviceStub,
TestServiceFutureStub::normal, expectedCode);
}

protected void assertStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final TestServiceBlockingStub blockingStub,
final TestServiceFutureStub futureStub,
final Code expectedCode) {
assertUnaryFailingMethod(serviceStub,
TestServiceStub::statusRuntimeException, blockingStub,
TestServiceBlockingStub::statusRuntimeException, futureStub,
TestServiceFutureStub::statusRuntimeException, expectedCode);
}

/**
* Tests with unary call.
*
@@ -109,6 +123,9 @@ DynamicTestCollection unaryCallTest() {
() -> assertUnaryCallSuccess(this.serviceStub, this.blockingStub, this.futureStub))
.add("unary-noPerm",
() -> assertUnaryCallFailure(this.noPermStub, this.noPermBlockingStub, this.noPermFutureStub,
PERMISSION_DENIED))
.add("unary-runtimeException-default",
() -> assertUnaryStatusRuntimeExceptionCallFailure(this.serviceStub, this.blockingStub, this.futureStub,
PERMISSION_DENIED));
}

@@ -131,6 +148,17 @@ protected void assertUnaryCallFailure(final TestServiceStub serviceStub,
TestServiceFutureStub::secure, expectedCode);
}

protected void assertUnaryStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final TestServiceBlockingStub blockingStub,
final TestServiceFutureStub futureStub,
final Code expectedCode) {
assertUnaryFailingMethod(serviceStub,
TestServiceStub::statusRuntimeException, blockingStub,
TestServiceBlockingStub::statusRuntimeException, futureStub,
TestServiceFutureStub::statusRuntimeException, expectedCode);
}


/**
* Tests with client streaming call.
*
@@ -142,7 +170,12 @@ DynamicTestCollection clientStreamingCallTests() {
return DynamicTestCollection.create()
.add("clientStreaming-default", () -> assertClientStreamingCallSuccess(this.serviceStub))
.add("clientStreaming-noPerm",
() -> assertClientStreamingCallFailure(this.noPermStub, PERMISSION_DENIED));
() -> assertClientStreamingCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("clientStreaming-runtimeException-default",
() -> assertClientStreamingStatusRuntimeExceptionCallFailure(this.serviceStub,
PERMISSION_DENIED))

;
}

protected void assertClientStreamingCallSuccess(final TestServiceStub serviceStub) {
@@ -160,6 +193,15 @@ protected void assertClientStreamingCallFailure(final TestServiceStub serviceStu
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

protected void assertClientStreamingStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<Empty> responseRecorder = StreamRecorder.create();
final StreamObserver<SomeType> requestObserver = serviceStub.statusRuntimeExceptionDrain(responseRecorder);
// Let the server throw an exception if he receives that (assert security):
sendAndComplete(requestObserver, "explode");
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

/**
* Tests with server streaming call.
*
@@ -172,7 +214,11 @@ DynamicTestCollection serverStreamingCallTests() {
.add("serverStreaming-default",
() -> assertServerStreamingCallSuccess(this.serviceStub))
.add("serverStreaming-noPerm",
() -> assertServerStreamingCallFailure(this.noPermStub, PERMISSION_DENIED));
() -> assertServerStreamingCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("serverStreaming-runtimeException-default",
() -> assertServerStreamingStatusRuntimeExceptionCallFailure(this.serviceStub,
PERMISSION_DENIED));

}

protected void assertServerStreamingCallSuccess(final TestServiceStub testStub) {
@@ -187,6 +233,13 @@ protected void assertServerStreamingCallFailure(final TestServiceStub serviceStu
assertFutureThrowsStatus(expectedCode, streamRecorder, 15, SECONDS);
}

protected void assertServerStreamingStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<SomeType> streamRecorder = StreamRecorder.create();
serviceStub.statusRuntimeExceptionSupply(EMPTY, streamRecorder);
assertFutureThrowsStatus(expectedCode, streamRecorder, 15, SECONDS);
}

/**
* Tests with bidirectional streaming call.
*
@@ -197,7 +250,9 @@ protected void assertServerStreamingCallFailure(final TestServiceStub serviceStu
DynamicTestCollection bidiStreamingCallTests() {
return DynamicTestCollection.create()
.add("bidiStreaming-default", () -> assertBidiCallSuccess(this.serviceStub))
.add("bidiStreaming-noPerm", () -> assertBidiCallFailure(this.noPermStub, PERMISSION_DENIED));
.add("bidiStreaming-noPerm", () -> assertBidiCallFailure(this.noPermStub, PERMISSION_DENIED))
.add("bidiStreaming-runtimeException-default",
() -> assertBidiStatusRuntimeExceptionCallFailure(this.serviceStub, PERMISSION_DENIED));
}

protected void assertBidiCallSuccess(final TestServiceStub testStub) {
@@ -214,6 +269,15 @@ protected void assertBidiCallFailure(final TestServiceStub serviceStub, final Co
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}

protected void assertBidiStatusRuntimeExceptionCallFailure(final TestServiceStub serviceStub,
final Code expectedCode) {
final StreamRecorder<SomeType> responseRecorder = StreamRecorder.create();
final StreamObserver<SomeType> requestObserver = serviceStub.statusRuntimeExceptionBidi(responseRecorder);
sendAndComplete(requestObserver, "explode");
assertFutureThrowsStatus(expectedCode, responseRecorder, 15, SECONDS);
}


// -------------------------------------

protected void assertUnarySuccessfulMethod(final TestServiceStub serviceStub,
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@

import io.grpc.Context;
import io.grpc.Status;
import io.grpc.StatusRuntimeException;
import io.grpc.stub.StreamObserver;
import lombok.extern.slf4j.Slf4j;
import net.devh.boot.grpc.server.security.interceptors.AuthenticatingServerInterceptor;
@@ -48,6 +49,26 @@ public TestServiceImpl() {
log.info("Created TestServiceImpl");
}

@Override
public void statusRuntimeException(Empty request, StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public StreamObserver<SomeType> statusRuntimeExceptionBidi(StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public StreamObserver<SomeType> statusRuntimeExceptionDrain(StreamObserver<Empty> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public void statusRuntimeExceptionSupply(Empty request, StreamObserver<SomeType> responseObserver) {
throw new StatusRuntimeException(Status.PERMISSION_DENIED);
}

@Override
public void normal(final Empty request, final StreamObserver<SomeType> responseObserver) {
log.debug("normal");
14 changes: 13 additions & 1 deletion tests/src/test/proto/TestService.proto
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ service TestService {

// Returns version information
rpc normal(google.protobuf.Empty) returns (SomeType) {}

// Fails with an error
rpc error(google.protobuf.Empty) returns (google.protobuf.Empty) {}

@@ -32,6 +32,18 @@ service TestService {
// Secured method with both multiple requests and answers
rpc secureBidi(stream SomeType) returns (stream SomeType) {}

// method
rpc statusRuntimeException(google.protobuf.Empty) returns (SomeType) {}

// method with only multiple requests
rpc statusRuntimeExceptionDrain(stream SomeType) returns (google.protobuf.Empty) {}

// method with only multiple answers
rpc statusRuntimeExceptionSupply(google.protobuf.Empty) returns (stream SomeType) {}

// method with both multiple requests and answers
rpc statusRuntimeExceptionBidi(stream SomeType) returns (stream SomeType) {}

}

message SomeType {