diff --git a/src/main/java/emissary/grpc/exceptions/ServiceException.java b/src/main/java/emissary/grpc/exceptions/ServiceException.java index 2345b6b05d..bf846b1c2d 100644 --- a/src/main/java/emissary/grpc/exceptions/ServiceException.java +++ b/src/main/java/emissary/grpc/exceptions/ServiceException.java @@ -12,7 +12,7 @@ public class ServiceException extends RuntimeException { private static final long serialVersionUID = 1371863576664142288L; - static final String GRPC_ERROR_MSG_FMT = "Encountered gRPC runtime status error %s. %s."; + public static final String GRPC_ERROR_PREFIX = "Encountered gRPC runtime status error. "; public ServiceException(String errorMessage) { super(errorMessage); @@ -28,43 +28,42 @@ public static void handleGrpcStatusRuntimeException(StatusRuntimeException e) { } Status status = e.getStatus(); if (status == null) { - throw new ServiceException("Service returned a null status"); + throw new ServiceException("Service returned a null status: " + e.getMessage(), e); } // code shouldn't ever be null, but we check for safety Status.Code code = status.getCode(); if (code == null) { - throw new ServiceException("Service returned a status with a null code"); + throw new ServiceException("Service returned a status with a null code: " + e.getMessage(), e); } switch (code) { case DEADLINE_EXCEEDED: - throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), - "gRPC client connection has timed out")); + throw new ServiceException(GRPC_ERROR_PREFIX + "gRPC client connection has timed out: " + e.getMessage(), e); case UNAVAILABLE: { // Likely server has gone down. Could be a crash or resources were scaled down String desc = status.getDescription(); if (StringUtils.isNotEmpty(desc) && desc.contains("Network closed for unknown reason")) { // So-called "poison pill" files have resulted in crashes for unknown reasons. // Out of an abundance of caution, we consider these files as failures. - throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), - "It's possible service crashed due to a misbehaving file")); + throw new ServiceException(GRPC_ERROR_PREFIX + + "It's possible service crashed due to a misbehaving file: " + e.getMessage(), e); } // Otherwise, we indicate the server is not live - throw new ServiceNotLiveException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), "It's likely service crashed")); + throw new ServiceNotLiveException(GRPC_ERROR_PREFIX + "It's likely service crashed: " + e.getMessage(), e); } case CANCELLED: - throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), "It's likely a client side interrupt occurred")); + throw new ServiceException(GRPC_ERROR_PREFIX + "It's likely a client side interrupt occurred: " + e.getMessage(), e); case RESOURCE_EXHAUSTED: // Likely we've exceeded the maximum number of concurrent requests - throw new ServiceNotReadyException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), - "It's likely we've exceeded the maximum number of requests")); + throw new ServiceNotReadyException(GRPC_ERROR_PREFIX + + "It's likely we've exceeded the maximum number of requests: " + e.getMessage(), e); case INTERNAL: // Likely server killed itself due to OOM or other conditions - throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), - "It's likely a gpu OOM error or other resource error has occurred")); + throw new ServiceException(GRPC_ERROR_PREFIX + + "It's likely a gpu OOM error or other resource error has occurred: " + e.getMessage(), e); default: - throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), - "This is an unhandled code type. Please add it to the list of gRPC exceptions")); + throw new ServiceException(GRPC_ERROR_PREFIX + + "This is an unhandled code type. Please add it to the list of gRPC exceptions: " + e.getMessage(), e); } } } diff --git a/src/main/java/emissary/grpc/pool/ConnectionFactory.java b/src/main/java/emissary/grpc/pool/ConnectionFactory.java index b8e07edda9..172e515a1b 100644 --- a/src/main/java/emissary/grpc/pool/ConnectionFactory.java +++ b/src/main/java/emissary/grpc/pool/ConnectionFactory.java @@ -160,7 +160,7 @@ public static ManagedChannel acquireChannel(ObjectPool pool) { try { return pool.borrowObject(); } catch (Exception e) { - throw new PoolException(String.format("Unable to borrow channel from pool: %s", e.getMessage())); + throw new PoolException(String.format("Unable to borrow channel from pool: %s", e.getMessage()), e); } } diff --git a/src/test/java/emissary/grpc/exceptions/ServiceExceptionTest.java b/src/test/java/emissary/grpc/exceptions/ServiceExceptionTest.java index 36bbb1845c..20f25a52f4 100644 --- a/src/test/java/emissary/grpc/exceptions/ServiceExceptionTest.java +++ b/src/test/java/emissary/grpc/exceptions/ServiceExceptionTest.java @@ -17,7 +17,7 @@ void testHandleGrpcStatusRuntimeExceptionWithNullCode() { StatusRuntimeException finalException = exception; ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(finalException)); - assertEquals("Encountered gRPC runtime status error UNKNOWN. This is an unhandled code type. Please add it to the list of gRPC exceptions.", + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "This is an unhandled code type. Please add it to the list of gRPC exceptions: UNKNOWN", se.getMessage()); } @@ -25,7 +25,7 @@ void testHandleGrpcStatusRuntimeExceptionWithNullCode() { void testHandleGrpcStatusRuntimeExceptionWithDeadlineExceeded() { StatusRuntimeException exception = new StatusRuntimeException(Status.DEADLINE_EXCEEDED.withDescription("test")); ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); - assertEquals("Encountered gRPC runtime status error DEADLINE_EXCEEDED: test. gRPC client connection has timed out.", + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "gRPC client connection has timed out: DEADLINE_EXCEEDED: test", se.getMessage()); } @@ -35,7 +35,8 @@ void testHandleGrpcStatusRuntimeExceptionWithUnavailableAndNetworkClosedForUnkno new StatusRuntimeException(Status.UNAVAILABLE.withDescription("Network closed for unknown reason")); ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); assertEquals( - "Encountered gRPC runtime status error UNAVAILABLE: Network closed for unknown reason. It's possible service crashed due to a misbehaving file.", + ServiceException.GRPC_ERROR_PREFIX + + "It's possible service crashed due to a misbehaving file: UNAVAILABLE: Network closed for unknown reason", se.getMessage()); } @@ -44,14 +45,14 @@ void testHandleGrpcStatusRuntimeExceptionWithUnavailable() { StatusRuntimeException exception = new StatusRuntimeException(Status.UNAVAILABLE.withDescription("test")); ServiceNotLiveException serviceNotLiveException = assertThrows(ServiceNotLiveException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); - assertEquals("Encountered gRPC runtime status error UNAVAILABLE: test. It's likely service crashed.", serviceNotLiveException.getMessage()); + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "It's likely service crashed: UNAVAILABLE: test", serviceNotLiveException.getMessage()); } @Test void testHandleGrpcStatusRuntimeExceptionWithCancelled() { StatusRuntimeException exception = new StatusRuntimeException(Status.CANCELLED.withDescription("test")); ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); - assertEquals("Encountered gRPC runtime status error CANCELLED: test. It's likely a client side interrupt occurred.", se.getMessage()); + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "It's likely a client side interrupt occurred: CANCELLED: test", se.getMessage()); } @Test @@ -59,7 +60,7 @@ void testHandleGrpcStatusRuntimeExceptionWithResourceExhausted() { StatusRuntimeException exception = new StatusRuntimeException(Status.RESOURCE_EXHAUSTED.withDescription("test")); ServiceNotReadyException serviceNotReadyException = assertThrows(ServiceNotReadyException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); - assertEquals("Encountered gRPC runtime status error RESOURCE_EXHAUSTED: test. It's likely we've exceeded the maximum number of requests.", + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "It's likely we've exceeded the maximum number of requests: RESOURCE_EXHAUSTED: test", serviceNotReadyException.getMessage()); } @@ -67,7 +68,7 @@ void testHandleGrpcStatusRuntimeExceptionWithResourceExhausted() { void testHandleGrpcStatusRuntimeExceptionWithInternal() { StatusRuntimeException exception = new StatusRuntimeException(Status.INTERNAL.withDescription("test")); ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); - assertEquals("Encountered gRPC runtime status error INTERNAL: test. It's likely a gpu OOM error or other resource error has occurred.", + assertEquals(ServiceException.GRPC_ERROR_PREFIX + "It's likely a gpu OOM error or other resource error has occurred: INTERNAL: test", se.getMessage()); } @@ -76,7 +77,7 @@ void testHandleGrpcStatusRuntimeExceptionWithDefault() { StatusRuntimeException exception = new StatusRuntimeException(Status.UNKNOWN.withDescription("test")); ServiceException se = assertThrows(ServiceException.class, () -> ServiceException.handleGrpcStatusRuntimeException(exception)); assertEquals( - "Encountered gRPC runtime status error UNKNOWN: test. This is an unhandled code type. Please add it to the list of gRPC exceptions.", + ServiceException.GRPC_ERROR_PREFIX + "This is an unhandled code type. Please add it to the list of gRPC exceptions: UNKNOWN: test", se.getMessage()); } diff --git a/src/test/java/emissary/grpc/sample/GrpcSampleServicePlaceTest.java b/src/test/java/emissary/grpc/sample/GrpcSampleServicePlaceTest.java index 653feb4af4..487ed30d93 100644 --- a/src/test/java/emissary/grpc/sample/GrpcSampleServicePlaceTest.java +++ b/src/test/java/emissary/grpc/sample/GrpcSampleServicePlaceTest.java @@ -178,7 +178,7 @@ void testGrpcRecoverableCodes(int code) { Status status = Status.fromCodeValue(code); Runnable invocation = () -> samplePlace.throwExceptionsDuringProcess(dataObject, new StatusRuntimeException(status)); ServiceNotAvailableException e = assertThrows(ServiceNotAvailableException.class, invocation::run); - assertTrue(e.getMessage().startsWith("Encountered gRPC runtime status error " + status.getCode().name())); + assertTrue(e.getMessage().endsWith(status.getCode().name())); assertTrue(dataObject.getAlternateViewNames().isEmpty()); } @@ -188,7 +188,7 @@ void testGrpcNonRecoverableCodes(int code) { Status status = Status.fromCodeValue(code); Runnable invocation = () -> samplePlace.throwExceptionsDuringProcess(dataObject, new StatusRuntimeException(status)); ServiceException e = assertThrows(ServiceException.class, invocation::run); - assertTrue(e.getMessage().startsWith("Encountered gRPC runtime status error " + status.getCode().name())); + assertTrue(e.getMessage().endsWith(status.getCode().name())); assertTrue(dataObject.getAlternateViewNames().isEmpty()); } @@ -244,7 +244,7 @@ void testGrpcFailureAfterMaxRecoverableCodes(int code) { dataObject, new StatusRuntimeException(status), retryAttempts, attemptNumber); ServiceNotAvailableException e = assertThrows(ServiceNotAvailableException.class, invocation::run); - assertTrue(e.getMessage().startsWith("Encountered gRPC runtime status error " + status.getCode().name())); + assertTrue(e.getMessage().endsWith(status.getCode().name())); assertTrue(dataObject.getAlternateViewNames().isEmpty()); assertEquals(RETRY_ATTEMPTS, attemptNumber.get()); } @@ -259,7 +259,7 @@ void testGrpcFailureAfterNonRecoverableCodes(int code) { dataObject, new StatusRuntimeException(status), RETRY_ATTEMPTS, attemptNumber); ServiceException e = assertThrows(ServiceException.class, invocation::run); - assertTrue(e.getMessage().startsWith("Encountered gRPC runtime status error " + status.getCode().name())); + assertTrue(e.getMessage().endsWith(status.getCode().name())); assertTrue(dataObject.getAlternateViewNames().isEmpty()); assertEquals(1, attemptNumber.get()); }