Skip to content
Open
Show file tree
Hide file tree
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
18 changes: 9 additions & 9 deletions src/main/java/emissary/grpc/exceptions/ServiceException.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,43 @@ 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);
}
// 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);
}

switch (code) {
case DEADLINE_EXCEEDED:
throw new ServiceException(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(),
"gRPC client connection has timed out"));
"gRPC client connection has timed out"), 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"));
"It's possible service crashed due to a misbehaving file"), 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(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), "It's likely service crashed"), 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(String.format(GRPC_ERROR_MSG_FMT, e.getMessage(), "It's likely a client side interrupt occurred"), 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"));
"It's likely we've exceeded the maximum number of requests"), 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"));
"It's likely a gpu OOM error or other resource error has occurred"), 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"));
"This is an unhandled code type. Please add it to the list of gRPC exceptions"), e);
}
}
}
2 changes: 1 addition & 1 deletion src/main/java/emissary/grpc/pool/ConnectionFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public static ManagedChannel acquireChannel(ObjectPool<ManagedChannel> 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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know not everyone agrees with me but i always follow this pattern.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rupert-griffin What @memny is getting at is that it might be best to include the exception message in all the throws in ServiceException like it is done here.

}
}

Expand Down
Loading