Skip to content

Commit 867a866

Browse files
jmesnilclaude
andcommitted
Address code review: simplify exception handling and warn on cleanup failures
- Replace catch (Exception e) + instanceof chain with catch (StartException | RuntimeException e) + throw e, using Java's precise rethrow so each exception type is raised as-is with no wrapping and no loss of suppressed exceptions. - Add LOGGER.failedToCleanupGrpcResources (WFLYGRPC-0013, WARN) to runSafely() so exceptions swallowed during stop() are visible in the server log rather than silently discarded. - Fix comment typo: 'service sof' -> 'services of'. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 82efd78 commit 867a866

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

‎subsystem/src/main/java/org/wildfly/extension/grpc/_private/GrpcLogger.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,8 @@ public interface GrpcLogger extends BasicLogger {
5252
@LogMessage(level = WARN)
5353
@Message(id = 12, value = "Weld capability not available for deployment %s; CDI injection will not be available for gRPC services.")
5454
void weldCapabilityUnavailable(@Cause Throwable cause, String deploymentName);
55+
56+
@LogMessage(level = WARN)
57+
@Message(id = 13, value = "Exception during gRPC service cleanup; resources may not have been fully released.")
58+
void failedToCleanupGrpcResources(@Cause Throwable cause);
5559
}

‎subsystem/src/main/java/org/wildfly/extension/grpc/deployment/GrpcDeploymentService.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,17 @@ class GrpcDeploymentService implements Service {
5959
public void start(final StartContext context) throws StartException {
6060
final WildFlyGrpcDeploymentRegistry serverService = serverServiceSupplier.get();
6161
try {
62-
// Interceptors are shared across all service sof the deployment — instantiate once, not per service.
62+
// Interceptors are shared across all services of the deployment — instantiate once, not per service.
6363
final List<ServerInterceptor> interceptors = createInterceptors();
6464
for (Class<? extends BindableService> serviceType : serviceClasses) {
6565
final BindableService instance = createInstance(serviceType);
6666
LOGGER.debugf("Registering gRPC service %s for deployment %s.", serviceType.getName(), deploymentName);
6767
ServerServiceDefinition ssd = serverService.addService(deploymentName, instance, interceptors);
6868
registeredServices.add(ssd);
6969
}
70-
} catch (Exception e) {
70+
} catch (StartException | RuntimeException e) {
7171
unregister(serverService, e);
72-
if (e instanceof StartException) {
73-
throw (StartException) e;
74-
}
75-
throw new StartException(e);
72+
throw e;
7673
}
7774
}
7875

@@ -97,6 +94,8 @@ private static void runSafely(final Runnable action, final Exception cause) {
9794
} catch (RuntimeException suppressed) {
9895
if (cause != null) {
9996
cause.addSuppressed(suppressed);
97+
} else {
98+
LOGGER.failedToCleanupGrpcResources(suppressed);
10099
}
101100
}
102101
}

0 commit comments

Comments
 (0)