Skip to content

Commit 1f94e38

Browse files
aaron-steinfeldaaron-steinfeld
authored
refactor: add support for better exception propagation and creation (#56)
1 parent d69cbdf commit 1f94e38

File tree

10 files changed

+279
-27
lines changed

10 files changed

+279
-27
lines changed

‎grpc-client-rx-utils/build.gradle.kts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ dependencies {
1313
implementation("io.grpc:grpc-context")
1414

1515
testImplementation("org.junit.jupiter:junit-jupiter:5.8.2")
16-
testImplementation("org.mockito:mockito-core:4.4.0")
17-
testImplementation("org.mockito:mockito-junit-jupiter:4.4.0")
16+
testImplementation("org.mockito:mockito-core:5.8.0")
17+
testImplementation("org.mockito:mockito-junit-jupiter:5.8.0")
1818
}
1919

2020
tasks.test {

‎grpc-client-utils/build.gradle.kts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ dependencies {
2323
compileOnly("org.projectlombok:lombok:1.18.24")
2424

2525
testImplementation("org.junit.jupiter:junit-jupiter:5.8.2")
26-
testImplementation("org.mockito:mockito-core:4.4.0")
27-
testImplementation("org.mockito:mockito-inline:4.4.0")
26+
testImplementation("org.mockito:mockito-core:5.8.0")
2827
testRuntimeOnly("io.grpc:grpc-netty")
2928
}
3029

‎grpc-context-utils/build.gradle.kts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ dependencies {
2323
compileOnly("org.projectlombok:lombok:1.18.24")
2424

2525
testImplementation("org.junit.jupiter:junit-jupiter:5.8.2")
26-
testImplementation("org.mockito:mockito-core:4.4.0")
26+
testImplementation("org.mockito:mockito-core:5.8.0")
27+
testImplementation("org.mockito:mockito-junit-jupiter:5.8.0")
2728
testImplementation("com.fasterxml.jackson.core:jackson-annotations:2.15.2")
2829
testAnnotationProcessor("org.projectlombok:lombok:1.18.24")
2930
testCompileOnly("org.projectlombok:lombok:1.18.24")

‎grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualExceptionDetails.java‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,37 +3,48 @@
33
import io.grpc.Metadata;
44
import io.grpc.Metadata.Key;
55
import java.util.Optional;
6-
import javax.annotation.Nonnull;
76
import javax.annotation.Nullable;
87
import lombok.AccessLevel;
98
import lombok.AllArgsConstructor;
10-
import lombok.Value;
9+
import lombok.EqualsAndHashCode;
10+
import lombok.ToString;
11+
import lombok.experimental.FieldDefaults;
1112

12-
@Value
13-
@AllArgsConstructor(access = AccessLevel.PACKAGE)
13+
@ToString
14+
@EqualsAndHashCode
15+
@FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE)
16+
@AllArgsConstructor(access = AccessLevel.PRIVATE)
1417
public class ContextualExceptionDetails {
1518
static final Key<String> EXTERNAL_MESSAGE_KEY =
1619
Key.of("external-message", Metadata.ASCII_STRING_MARSHALLER);
17-
@Nonnull RequestContext requestContext;
20+
@Nullable RequestContext requestContext;
1821
@Nullable String externalMessage;
1922

2023
public Optional<String> getExternalMessage() {
2124
return Optional.ofNullable(this.externalMessage);
2225
}
2326

24-
ContextualExceptionDetails(@Nonnull RequestContext requestContext) {
27+
public Optional<RequestContext> getRequestContext() {
28+
return Optional.ofNullable(this.requestContext);
29+
}
30+
31+
ContextualExceptionDetails(RequestContext requestContext) {
2532
this(requestContext, null);
2633
}
2734

35+
ContextualExceptionDetails() {
36+
this(null);
37+
}
38+
2839
Metadata toMetadata() {
2940
Metadata metadata = new Metadata();
3041
this.getExternalMessage().ifPresent(value -> metadata.put(EXTERNAL_MESSAGE_KEY, value));
31-
metadata.merge(this.getRequestContext().buildTrailers());
42+
this.getRequestContext().map(RequestContext::buildTrailers).ifPresent(metadata::merge);
3243
return metadata;
3344
}
3445

3546
ContextualExceptionDetails withExternalMessage(@Nullable String externalMessage) {
36-
return new ContextualExceptionDetails(this.getRequestContext(), externalMessage);
47+
return new ContextualExceptionDetails(this.getRequestContext().orElse(null), externalMessage);
3748
}
3849

3950
public static Optional<ContextualExceptionDetails> fromMetadata(Metadata metadata) {

‎grpc-context-utils/src/main/java/org/hypertrace/core/grpcutils/context/ContextualStatusExceptionBuilder.java‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
package org.hypertrace.core.grpcutils.context;
22

3+
import io.grpc.Metadata;
34
import io.grpc.Status;
45
import io.grpc.StatusException;
56
import io.grpc.StatusRuntimeException;
7+
import java.util.Optional;
68
import javax.annotation.Nonnull;
9+
import javax.annotation.Nullable;
710
import lombok.AccessLevel;
811
import lombok.AllArgsConstructor;
912
import lombok.EqualsAndHashCode;
@@ -16,11 +19,32 @@
1619
@AllArgsConstructor(access = AccessLevel.PRIVATE)
1720
public class ContextualStatusExceptionBuilder {
1821

19-
private final Status status;
20-
ContextualExceptionDetails details;
22+
@Nonnull private final Status status;
23+
@Nullable private final Metadata originalTrailers;
24+
@Nonnull ContextualExceptionDetails details;
2125

2226
public static ContextualStatusExceptionBuilder from(Status status, RequestContext context) {
23-
return new ContextualStatusExceptionBuilder(status, new ContextualExceptionDetails(context));
27+
return new ContextualStatusExceptionBuilder(
28+
status, null, new ContextualExceptionDetails(context));
29+
}
30+
31+
public static ContextualStatusExceptionBuilder from(Status status) {
32+
return new ContextualStatusExceptionBuilder(status, null, new ContextualExceptionDetails());
33+
}
34+
35+
public static ContextualStatusExceptionBuilder from(StatusException statusException) {
36+
return new ContextualStatusExceptionBuilder(
37+
statusException.getStatus(),
38+
statusException.getTrailers(),
39+
new ContextualExceptionDetails());
40+
}
41+
42+
public static ContextualStatusExceptionBuilder from(
43+
StatusRuntimeException statusRuntimeException) {
44+
return new ContextualStatusExceptionBuilder(
45+
statusRuntimeException.getStatus(),
46+
statusRuntimeException.getTrailers(),
47+
new ContextualExceptionDetails());
2448
}
2549

2650
public ContextualStatusExceptionBuilder useStatusDescriptionAsExternalMessage() {
@@ -34,10 +58,17 @@ public ContextualStatusExceptionBuilder withExternalMessage(@Nonnull String exte
3458
}
3559

3660
public StatusRuntimeException buildRuntimeException() {
37-
return status.asRuntimeException(this.details.toMetadata());
61+
return status.asRuntimeException(this.collectMetadata());
3862
}
3963

4064
public StatusException buildCheckedException() {
41-
return status.asException(this.details.toMetadata());
65+
return status.asException(this.collectMetadata());
66+
}
67+
68+
private Metadata collectMetadata() {
69+
Metadata metadataCollector = new Metadata();
70+
Optional.ofNullable(this.originalTrailers).ifPresent(metadataCollector::merge);
71+
metadataCollector.merge(this.details.toMetadata());
72+
return metadataCollector;
4273
}
4374
}

‎grpc-context-utils/src/test/java/org/hypertrace/core/grpcutils/context/ContextualExceptionDetailsTest.java‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,15 @@
1010
import io.grpc.Metadata.Key;
1111
import io.grpc.Status;
1212
import io.grpc.StatusException;
13+
import io.grpc.StatusRuntimeException;
1314
import java.util.Map;
1415
import java.util.Optional;
1516
import java.util.Set;
1617
import org.junit.jupiter.api.Test;
18+
import org.junit.jupiter.api.extension.ExtendWith;
19+
import org.mockito.junit.jupiter.MockitoExtension;
1720

21+
@ExtendWith(MockitoExtension.class)
1822
class ContextualExceptionDetailsTest {
1923
static final String TEST_REQUEST_ID = "example-request-id";
2024
static final String TEST_TENANT = "example-tenant";
@@ -86,7 +90,8 @@ void parsesMetadataWithoutMessage() {
8690
@Test
8791
void parsesMetadataWithMessage() {
8892
assertEquals(
89-
Optional.of(new ContextualExceptionDetails(TEST_CONTEXT, "test message")),
93+
Optional.of(
94+
new ContextualExceptionDetails(TEST_CONTEXT).withExternalMessage("test message")),
9095
ContextualExceptionDetails.fromMetadata(
9196
metadataFromMap(
9297
Map.of(
@@ -98,6 +103,44 @@ void parsesMetadataWithMessage() {
98103
"test message"))));
99104
}
100105

106+
@Test
107+
void buildsFromExistingException() {
108+
StatusRuntimeException exception =
109+
ContextualStatusExceptionBuilder.from(
110+
Status.INVALID_ARGUMENT
111+
.withDescription("test message")
112+
.asException(TEST_CONTEXT.buildTrailers()))
113+
.useStatusDescriptionAsExternalMessage()
114+
.buildRuntimeException();
115+
116+
assertEquals(
117+
Set.of(REQUEST_ID_HEADER_KEY, TENANT_ID_HEADER_KEY, EXTERNAL_MESSAGE_KEY.originalName()),
118+
exception.getTrailers().keys());
119+
assertEquals(TEST_CONTEXT, RequestContext.fromMetadata(exception.getTrailers()));
120+
assertEquals("test message", exception.getTrailers().get(EXTERNAL_MESSAGE_KEY));
121+
}
122+
123+
@Test
124+
void buildsFromExceptionWithCustomTrailers() {
125+
Key<String> customKey = Key.of("test", ASCII_STRING_MARSHALLER);
126+
Metadata customMetadata = new Metadata();
127+
customMetadata.put(customKey, "test-value");
128+
customMetadata.put(EXTERNAL_MESSAGE_KEY, "should be ignored");
129+
StatusException exception =
130+
ContextualStatusExceptionBuilder.from(
131+
Status.INVALID_ARGUMENT
132+
.withDescription("test message")
133+
.asRuntimeException(customMetadata))
134+
.withExternalMessage("custom message")
135+
.buildCheckedException();
136+
137+
assertEquals(
138+
Set.of(customKey.originalName(), EXTERNAL_MESSAGE_KEY.originalName()),
139+
exception.getTrailers().keys());
140+
assertEquals("custom message", exception.getTrailers().get(EXTERNAL_MESSAGE_KEY));
141+
assertEquals("test-value", exception.getTrailers().get(customKey));
142+
}
143+
101144
Metadata metadataFromMap(Map<String, String> metadataMap) {
102145
Metadata metadata = new Metadata();
103146
metadataMap.forEach((key, value) -> metadata.put(Key.of(key, ASCII_STRING_MARSHALLER), value));

‎grpc-server-rx-utils/build.gradle.kts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ dependencies {
1616
implementation("org.slf4j:slf4j-api:1.7.36")
1717

1818
testImplementation("org.junit.jupiter:junit-jupiter:5.8.2")
19-
testImplementation("org.mockito:mockito-core:4.4.0")
20-
testImplementation("org.mockito:mockito-junit-jupiter:4.4.0")
19+
testImplementation("org.mockito:mockito-core:5.8.0")
20+
testImplementation("org.mockito:mockito-junit-jupiter:5.8.0")
2121
}
2222

2323
tasks.test {

‎grpc-server-utils/build.gradle.kts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,6 @@ dependencies {
2525
compileOnly("org.projectlombok:lombok:1.18.24")
2626

2727
testImplementation("org.junit.jupiter:junit-jupiter:5.8.2")
28-
testImplementation("org.mockito:mockito-core:4.4.0")
28+
testImplementation("org.mockito:mockito-core:5.8.0")
29+
testImplementation("org.mockito:mockito-junit-jupiter:5.8.0")
2930
}

‎grpc-server-utils/src/main/java/org/hypertrace/core/grpcutils/server/ThrowableResponseInterceptor.java‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
package org.hypertrace.core.grpcutils.server;
22

3+
import static io.grpc.Status.Code.UNKNOWN;
4+
import static java.util.Objects.isNull;
5+
36
import io.grpc.ForwardingServerCall;
47
import io.grpc.Metadata;
58
import io.grpc.ServerCall;
69
import io.grpc.ServerCallHandler;
710
import io.grpc.ServerInterceptor;
811
import io.grpc.Status;
12+
import java.util.Optional;
913
import org.hypertrace.core.grpcutils.context.RequestContext;
1014

1115
/** Server Interceptor that decorates the error response status before closing the call */
@@ -22,21 +26,49 @@ public void sendMessage(RespT message) {
2226

2327
@Override
2428
public void close(Status status, Metadata trailers) {
25-
if (status.getCode() == Status.Code.UNKNOWN
29+
if (status.getCode() == UNKNOWN
2630
&& status.getDescription() == null
2731
&& status.getCause() != null) {
2832
status =
2933
Status.INTERNAL
3034
.withDescription(status.getCause().getMessage())
3135
.withCause(status.getCause());
3236
}
33-
if (!status.isOk() && trailers.keys().isEmpty()) {
34-
super.close(status, RequestContext.fromMetadata(headers).buildTrailers());
35-
}
36-
super.close(status, trailers);
37+
super.close(
38+
status,
39+
collectAndMergeMetadata(RequestContext.fromMetadata(headers), trailers, status));
3740
}
3841
};
3942

4043
return next.startCall(wrappedCall, headers);
4144
}
45+
46+
private Metadata collectAndMergeMetadata(
47+
RequestContext requestContext, Metadata originalTrailers, Status status) {
48+
Metadata mergedTrailers = new Metadata();
49+
// We build with increasing precedence (since metadata.get() reads last value)
50+
mergedTrailers.merge(requestContext.buildTrailers());
51+
mergedTrailers.merge(collectAllTrailersFromCause(status));
52+
mergedTrailers.merge(originalTrailers);
53+
54+
return mergedTrailers;
55+
}
56+
57+
private Metadata collectAllTrailersFromCause(Status status) {
58+
// Two base cases - either no cause or an unknown cause
59+
Throwable cause = status.getCause();
60+
if (isNull(cause)) {
61+
return new Metadata();
62+
}
63+
Status statusFromCause = Status.fromThrowable(cause);
64+
if (statusFromCause.getCode() == UNKNOWN) {
65+
return new Metadata();
66+
}
67+
// Otherwise, we've found a status so collect any trailers from it and merge them on top of
68+
// any trailers we can find from descendents
69+
Metadata trailersFromCauseDescendents = this.collectAllTrailersFromCause(statusFromCause);
70+
Optional.ofNullable(Status.trailersFromThrowable(cause))
71+
.ifPresent(trailersFromCauseDescendents::merge);
72+
return trailersFromCauseDescendents;
73+
}
4274
}

0 commit comments

Comments
 (0)