Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Scottmitch committed May 27, 2022
1 parent 6c81cea commit e2d2110
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public enum GrpcStatusCode {
* when they are done.
*/
OUT_OF_RANGE(11),
/** The method/operation is not implemented/supported/implemented. */
/** The method/operation is not implemented/supported. */
UNIMPLEMENTED(12),
/** Internal invariant violated. */
INTERNAL(13),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@
import static io.servicetalk.grpc.api.GrpcHeaderValues.SERVICETALK_USER_AGENT;
import static io.servicetalk.grpc.api.GrpcStatusCode.CANCELLED;
import static io.servicetalk.grpc.api.GrpcStatusCode.DEADLINE_EXCEEDED;
import static io.servicetalk.grpc.api.GrpcStatusCode.INTERNAL;
import static io.servicetalk.grpc.api.GrpcStatusCode.FAILED_PRECONDITION;
import static io.servicetalk.grpc.api.GrpcStatusCode.INVALID_ARGUMENT;
import static io.servicetalk.grpc.api.GrpcStatusCode.PERMISSION_DENIED;
import static io.servicetalk.grpc.api.GrpcStatusCode.UNAUTHENTICATED;
import static io.servicetalk.grpc.api.GrpcStatusCode.UNAVAILABLE;
Expand All @@ -94,13 +95,16 @@
import static io.servicetalk.http.api.HttpHeaderValues.TRAILERS;
import static io.servicetalk.http.api.HttpRequestMethod.POST;
import static io.servicetalk.http.api.HttpResponseStatus.BAD_GATEWAY;
import static io.servicetalk.http.api.HttpResponseStatus.BAD_REQUEST;
import static io.servicetalk.http.api.HttpResponseStatus.EXPECTATION_FAILED;
import static io.servicetalk.http.api.HttpResponseStatus.FORBIDDEN;
import static io.servicetalk.http.api.HttpResponseStatus.GATEWAY_TIMEOUT;
import static io.servicetalk.http.api.HttpResponseStatus.NOT_FOUND;
import static io.servicetalk.http.api.HttpResponseStatus.NOT_IMPLEMENTED;
import static io.servicetalk.http.api.HttpResponseStatus.OK;
import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE;
import static io.servicetalk.http.api.HttpResponseStatus.PRECONDITION_FAILED;
import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_TIMEOUT;
import static io.servicetalk.http.api.HttpResponseStatus.SERVICE_UNAVAILABLE;
import static io.servicetalk.http.api.HttpResponseStatus.StatusClass.CLIENT_ERROR_4XX;
import static io.servicetalk.http.api.HttpResponseStatus.TOO_MANY_REQUESTS;
import static io.servicetalk.http.api.HttpResponseStatus.UNAUTHORIZED;
import static java.lang.String.valueOf;
Expand Down Expand Up @@ -290,10 +294,14 @@ private static void validateStatusCode(HttpResponseStatus status) {
grpcStatusCode = UNAUTHENTICATED;
} else if (statusCode == FORBIDDEN.code()) {
grpcStatusCode = PERMISSION_DENIED;
} else if (statusCode == NOT_FOUND.code()) {
} else if (statusCode == NOT_FOUND.code() || statusCode == NOT_IMPLEMENTED.code()) {
grpcStatusCode = UNIMPLEMENTED;
} else if (statusCode == BAD_REQUEST.code() || statusCode == REQUEST_HEADER_FIELDS_TOO_LARGE.code()) {
grpcStatusCode = INTERNAL;
} else if (statusCode == REQUEST_TIMEOUT.code()) {
grpcStatusCode = DEADLINE_EXCEEDED;
} else if (statusCode == PRECONDITION_FAILED.code() || statusCode == EXPECTATION_FAILED.code()) {
grpcStatusCode = FAILED_PRECONDITION;
} else if (CLIENT_ERROR_4XX.contains(statusCode)) {
grpcStatusCode = INVALID_ARGUMENT;
} else {
grpcStatusCode = UNKNOWN;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,18 @@
import static io.servicetalk.grpc.api.GrpcExecutionStrategy.from;
import static io.servicetalk.grpc.api.GrpcStatusCode.CANCELLED;
import static io.servicetalk.grpc.api.GrpcStatusCode.DEADLINE_EXCEEDED;
import static io.servicetalk.grpc.api.GrpcStatusCode.FAILED_PRECONDITION;
import static io.servicetalk.grpc.api.GrpcStatusCode.INTERNAL;
import static io.servicetalk.grpc.api.GrpcStatusCode.INVALID_ARGUMENT;
import static io.servicetalk.grpc.api.GrpcStatusCode.UNKNOWN;
import static io.servicetalk.grpc.internal.DeadlineUtils.GRPC_TIMEOUT_HEADER_KEY;
import static io.servicetalk.http.api.HttpExecutionStrategies.offloadNone;
import static io.servicetalk.http.api.HttpResponseStatus.BAD_REQUEST;
import static io.servicetalk.http.api.HttpResponseStatus.EXPECTATION_FAILED;
import static io.servicetalk.http.api.HttpResponseStatus.PRECONDITION_FAILED;
import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_HEADER_FIELDS_TOO_LARGE;
import static io.servicetalk.http.api.HttpResponseStatus.REQUEST_TIMEOUT;
import static io.servicetalk.http.api.HttpResponseStatus.StatusClass.CLIENT_ERROR_4XX;
import static io.servicetalk.http.netty.HttpProtocolConfigs.h2Default;
import static io.servicetalk.test.resources.DefaultTestCerts.loadServerKey;
import static io.servicetalk.test.resources.DefaultTestCerts.loadServerPem;
Expand Down Expand Up @@ -531,9 +540,19 @@ void clientH2ReturnStatus() throws Exception {
// grpc-java maps 1xx responses to error code INTERNAL, we currently map to UNKNOWN. The test server
// isn't following the http protocol by returning only a 1xx response and each framework catches
// this exception differently internally.
assertThat("mismatch for h2 response code: " + httpCode, stCode, equalTo(UNKNOWN.value()));
} else {
assertThat("mismatch for h2 response code: " + httpCode, stCode, equalTo(grpcJavaCode));
assertThat("h2 response code: " + httpCode, stCode, equalTo(UNKNOWN.value()));
assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(INTERNAL.value()));
} else if (httpCode == REQUEST_TIMEOUT.code()) {
assertThat("h2 response code: " + httpCode, stCode, equalTo(DEADLINE_EXCEEDED.value()));
assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(UNKNOWN.value()));
} else if (httpCode == PRECONDITION_FAILED.code() || httpCode == EXPECTATION_FAILED.code()) {
assertThat("h2 response code: " + httpCode, stCode, equalTo(FAILED_PRECONDITION.value()));
assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(UNKNOWN.value()));
} else if (stCode != grpcJavaCode && CLIENT_ERROR_4XX.contains(httpCode)) {
assertThat("h2 response code: " + httpCode, stCode, equalTo(INVALID_ARGUMENT.value()));
assertThat("h2 response code: " + httpCode, grpcJavaCode, equalTo(
httpCode == BAD_REQUEST.code() || httpCode == REQUEST_HEADER_FIELDS_TOO_LARGE.code() ?
INTERNAL.value() : UNKNOWN.value()));
}
}
}
Expand Down

0 comments on commit e2d2110

Please sign in to comment.