Skip to content

Commit a2d7c00

Browse files
Copilotbachuv
andauthored
Address review feedback on StatusRuntimeExceptionHelper
1. Null-safe description: getDescriptionOrDefault() returns "(no description)" instead of letting null propagate into error messages. 2. Javadoc comment on toException explaining DEADLINE_EXCEEDED is included for completeness/future-proofing even though current call sites handle it first. 3. Extracted private helpers (createCancellationException, createRuntimeException, formatMessage, getDescriptionOrDefault) to eliminate duplication between toRuntimeException and toException. 4. Consistent message format: DEADLINE_EXCEEDED now uses the same "failed with a <code> gRPC status: <description>" prefix as all other status codes. Agent-Logs-Url: https://github.com/microsoft/durabletask-java/sessions/747c79d2-a3fa-427c-8b3d-63515878b1d1 Co-authored-by: bachuv <15795068+bachuv@users.noreply.github.com>
1 parent f7b7d1c commit a2d7c00

File tree

2 files changed

+74
-18
lines changed

2 files changed

+74
-18
lines changed

client/src/main/java/com/microsoft/durabletask/StatusRuntimeExceptionHelper.java

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,21 +25,19 @@ static RuntimeException toRuntimeException(StatusRuntimeException e, String oper
2525
Status.Code code = e.getStatus().getCode();
2626
switch (code) {
2727
case CANCELLED:
28-
CancellationException ce = new CancellationException(
29-
"The " + operationName + " operation was canceled.");
30-
ce.initCause(e);
31-
return ce;
28+
return createCancellationException(e, operationName);
3229
default:
33-
return new RuntimeException(
34-
"The " + operationName + " operation failed with a " + code + " gRPC status: "
35-
+ e.getStatus().getDescription(),
36-
e);
30+
return createRuntimeException(e, operationName, code);
3731
}
3832
}
3933

4034
/**
4135
* Translates a {@link StatusRuntimeException} into an appropriate SDK-level checked exception
4236
* for operations that declare {@code throws TimeoutException}.
37+
* <p>
38+
* Note: The DEADLINE_EXCEEDED case is included for completeness and future-proofing, even
39+
* though current call sites handle DEADLINE_EXCEEDED before falling through to this method.
40+
* This ensures centralized translation if call sites are refactored in the future.
4341
*
4442
* @param e the gRPC exception to translate
4543
* @param operationName the name of the operation that failed, used in exception messages
@@ -50,20 +48,37 @@ static Exception toException(StatusRuntimeException e, String operationName) {
5048
switch (code) {
5149
case DEADLINE_EXCEEDED:
5250
return new TimeoutException(
53-
"The " + operationName + " operation timed out: " + e.getStatus().getDescription());
51+
formatMessage(operationName, code, getDescriptionOrDefault(e)));
5452
case CANCELLED:
55-
CancellationException ce = new CancellationException(
56-
"The " + operationName + " operation was canceled.");
57-
ce.initCause(e);
58-
return ce;
53+
return createCancellationException(e, operationName);
5954
default:
60-
return new RuntimeException(
61-
"The " + operationName + " operation failed with a " + code + " gRPC status: "
62-
+ e.getStatus().getDescription(),
63-
e);
55+
return createRuntimeException(e, operationName, code);
6456
}
6557
}
6658

59+
private static CancellationException createCancellationException(
60+
StatusRuntimeException e, String operationName) {
61+
CancellationException ce = new CancellationException(
62+
"The " + operationName + " operation was canceled.");
63+
ce.initCause(e);
64+
return ce;
65+
}
66+
67+
private static RuntimeException createRuntimeException(
68+
StatusRuntimeException e, String operationName, Status.Code code) {
69+
return new RuntimeException(
70+
formatMessage(operationName, code, getDescriptionOrDefault(e)), e);
71+
}
72+
73+
private static String formatMessage(String operationName, Status.Code code, String description) {
74+
return "The " + operationName + " operation failed with a " + code + " gRPC status: " + description;
75+
}
76+
77+
private static String getDescriptionOrDefault(StatusRuntimeException e) {
78+
String description = e.getStatus().getDescription();
79+
return description != null ? description : "(no description)";
80+
}
81+
6782
// Cannot be instantiated
6883
private StatusRuntimeExceptionHelper() {
6984
}

client/src/test/java/com/microsoft/durabletask/StatusRuntimeExceptionHelperTest.java

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ void toRuntimeException_unavailableStatus_returnsRuntimeException() {
5656
assertNotEquals(CancellationException.class, result.getClass());
5757
assertTrue(result.getMessage().contains("terminate"));
5858
assertTrue(result.getMessage().contains("UNAVAILABLE"));
59+
assertTrue(result.getMessage().contains("Connection refused"));
5960
assertSame(grpcException, result.getCause());
6061
}
6162

@@ -96,6 +97,19 @@ void toRuntimeException_preservesOperationName() {
9697
assertTrue(result.getMessage().contains("customOperationName"));
9798
}
9899

100+
@Test
101+
void toRuntimeException_nullDescription_usesDefaultFallback() {
102+
StatusRuntimeException grpcException = new StatusRuntimeException(Status.INTERNAL);
103+
104+
RuntimeException result = StatusRuntimeExceptionHelper.toRuntimeException(
105+
grpcException, "testOp");
106+
107+
assertTrue(result.getMessage().contains("(no description)"),
108+
"Expected '(no description)' fallback but got: " + result.getMessage());
109+
assertFalse(result.getMessage().contains("null"),
110+
"Message should not contain literal 'null': " + result.getMessage());
111+
}
112+
99113
// Tests for toException (checked exception variant)
100114

101115
@Test
@@ -108,7 +122,20 @@ void toException_deadlineExceededStatus_returnsTimeoutException() {
108122

109123
assertInstanceOf(TimeoutException.class, result);
110124
assertTrue(result.getMessage().contains("waitForInstanceStart"));
111-
assertTrue(result.getMessage().contains("timed out"));
125+
assertTrue(result.getMessage().contains("DEADLINE_EXCEEDED"));
126+
}
127+
128+
@Test
129+
void toException_deadlineExceededStatus_usesConsistentMessageFormat() {
130+
StatusRuntimeException grpcException = new StatusRuntimeException(
131+
Status.DEADLINE_EXCEEDED.withDescription("timeout after 5s"));
132+
133+
Exception result = StatusRuntimeExceptionHelper.toException(
134+
grpcException, "purgeInstances");
135+
136+
assertInstanceOf(TimeoutException.class, result);
137+
assertTrue(result.getMessage().contains("failed with a DEADLINE_EXCEEDED gRPC status"),
138+
"Expected consistent message format but got: " + result.getMessage());
112139
}
113140

114141
@Test
@@ -148,4 +175,18 @@ void toException_internalStatus_returnsRuntimeExceptionWithCause() {
148175
assertInstanceOf(RuntimeException.class, result);
149176
assertSame(grpcException, result.getCause());
150177
}
178+
179+
@Test
180+
void toException_nullDescription_usesDefaultFallback() {
181+
StatusRuntimeException grpcException = new StatusRuntimeException(Status.DEADLINE_EXCEEDED);
182+
183+
Exception result = StatusRuntimeExceptionHelper.toException(
184+
grpcException, "testOp");
185+
186+
assertInstanceOf(TimeoutException.class, result);
187+
assertTrue(result.getMessage().contains("(no description)"),
188+
"Expected '(no description)' fallback but got: " + result.getMessage());
189+
assertFalse(result.getMessage().contains("null"),
190+
"Message should not contain literal 'null': " + result.getMessage());
191+
}
151192
}

0 commit comments

Comments
 (0)