Skip to content

Commit b9c23f2

Browse files
OmniLab Teamcopybara-github
OmniLab Team
authored and
copybara-github
committed
FE v6: adopt FeServiceException in TroubleshootScriptHandler
Wrap downstream RPC failures with FeServiceException so the server returns proper HTTP status codes instead of always HTTP 500 Internal Server Error when troubleshoot scripts fail. PiperOrigin-RevId: 951310626
1 parent 86a2bd9 commit b9c23f2

3 files changed

Lines changed: 141 additions & 43 deletions

File tree

‎src/java/com/google/devtools/mobileharness/fe/v6/service/host/handlers/TroubleshootScriptHandler.java‎

Lines changed: 76 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,15 @@
1919
import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
2020
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
2121

22+
import com.google.common.flogger.FluentLogger;
23+
import com.google.common.util.concurrent.FluentFuture;
2224
import com.google.common.util.concurrent.Futures;
2325
import com.google.common.util.concurrent.ListenableFuture;
24-
import com.google.devtools.mobileharness.api.model.error.BasicErrorId;
25-
import com.google.devtools.mobileharness.api.model.error.MobileHarnessException;
2626
import com.google.devtools.mobileharness.api.model.proto.Device.DeviceStatus;
2727
import com.google.devtools.mobileharness.api.query.proto.FilterProto;
2828
import com.google.devtools.mobileharness.api.query.proto.LabQueryProto.LabQuery;
2929
import com.google.devtools.mobileharness.fe.v6.service.device.provider.DeviceOpsStubProvider;
30+
import com.google.devtools.mobileharness.fe.v6.service.errors.FeServiceException;
3031
import com.google.devtools.mobileharness.fe.v6.service.host.handlers.TroubleshootScriptRegistry.ScriptMetadata;
3132
import com.google.devtools.mobileharness.fe.v6.service.proto.host.ListTroubleshootScriptsRequest;
3233
import com.google.devtools.mobileharness.fe.v6.service.proto.host.ListTroubleshootScriptsResponse;
@@ -40,13 +41,17 @@
4041
import com.google.devtools.mobileharness.shared.labinfo.proto.LabInfoServiceProto.GetLabInfoResponse;
4142
import com.google.wireless.qa.mobileharness.lab.proto.DeviceOpsServ;
4243
import com.google.wireless.qa.mobileharness.lab.proto.DeviceOpsServ.RunTroubleshootScriptRequest.TroubleshootScript;
44+
import io.grpc.Status;
45+
import io.grpc.StatusRuntimeException;
4346
import javax.inject.Inject;
4447
import javax.inject.Singleton;
4548

4649
/** Handler for troubleshoot script operations (run and list). */
4750
@Singleton
4851
public class TroubleshootScriptHandler {
4952

53+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
54+
5055
private final LabInfoProvider labInfoProvider;
5156
private final DeviceOpsStubProvider deviceOpsStubProvider;
5257

@@ -62,47 +67,55 @@ public ListenableFuture<RunTroubleshootScriptResponse> runTroubleshootScript(
6267
RunTroubleshootScriptRequest request, UniverseScope universe) {
6368
if (!(universe instanceof UniverseScope.SelfUniverse)) {
6469
return immediateFailedFuture(
65-
new IllegalArgumentException(
70+
FeServiceException.unimplemented(
6671
"Troubleshoot scripts are only supported in the self universe."));
6772
}
6873
String hostName = request.getHostName();
6974

70-
return Futures.transformAsync(
71-
labInfoProvider.getLabInfoAsync(createGetLabInfoRequest(hostName), universe),
72-
labInfoResponse -> {
73-
DeviceOpsServ.RunTroubleshootScriptRequest.TroubleshootScript labScript =
74-
switch (request.getScript()) {
75-
case RESET_USB_HUB ->
76-
DeviceOpsServ.RunTroubleshootScriptRequest.TroubleshootScript.RESET_USB_HUB;
77-
default ->
78-
throw new MobileHarnessException(
79-
BasicErrorId.COMMAND_EXEC_FAIL,
80-
"Unsupported troubleshoot script: " + request.getScript());
81-
};
82-
83-
DeviceOpsServ.RunTroubleshootScriptRequest labRequest =
84-
DeviceOpsServ.RunTroubleshootScriptRequest.newBuilder().setScript(labScript).build();
85-
86-
DeviceOpsStub stub = deviceOpsStubProvider.createStub(hostName, universe);
87-
return Futures.transform(
88-
stub.runTroubleshootScriptAsync(labRequest),
89-
labResponse ->
90-
RunTroubleshootScriptResponse.newBuilder()
91-
.setExitCode(labResponse.getExitCode())
92-
.setStdout(labResponse.getStdout())
93-
.setStderr(labResponse.getStderr())
94-
.build(),
95-
directExecutor());
96-
},
97-
directExecutor());
75+
return FluentFuture.from(
76+
labInfoProvider.getLabInfoAsync(createGetLabInfoRequest(hostName), universe))
77+
.transformAsync(
78+
labInfoResponse -> {
79+
DeviceOpsServ.RunTroubleshootScriptRequest.TroubleshootScript labScript =
80+
switch (request.getScript()) {
81+
case RESET_USB_HUB ->
82+
DeviceOpsServ.RunTroubleshootScriptRequest.TroubleshootScript.RESET_USB_HUB;
83+
default ->
84+
throw FeServiceException.invalidArgument(
85+
"Unsupported troubleshoot script: " + request.getScript());
86+
};
87+
88+
DeviceOpsServ.RunTroubleshootScriptRequest labRequest =
89+
DeviceOpsServ.RunTroubleshootScriptRequest.newBuilder()
90+
.setScript(labScript)
91+
.build();
92+
93+
DeviceOpsStub stub = deviceOpsStubProvider.createStub(hostName, universe);
94+
return Futures.transform(
95+
stub.runTroubleshootScriptAsync(labRequest),
96+
labResponse ->
97+
RunTroubleshootScriptResponse.newBuilder()
98+
.setExitCode(labResponse.getExitCode())
99+
.setStdout(labResponse.getStdout())
100+
.setStderr(labResponse.getStderr())
101+
.build(),
102+
directExecutor());
103+
},
104+
directExecutor())
105+
.catching(
106+
Exception.class,
107+
e -> {
108+
throw toFeServiceException(hostName, e);
109+
},
110+
directExecutor());
98111
}
99112

100113
/** Lists available troubleshoot scripts, disabling them if any device is BUSY. */
101114
public ListenableFuture<ListTroubleshootScriptsResponse> listTroubleshootScripts(
102115
ListTroubleshootScriptsRequest request, UniverseScope universe) {
103116
if (!(universe instanceof UniverseScope.SelfUniverse)) {
104117
return immediateFailedFuture(
105-
new IllegalArgumentException(
118+
FeServiceException.unimplemented(
106119
"Troubleshoot scripts are only supported in the self universe."));
107120
}
108121
String hostName = request.getHostName();
@@ -151,6 +164,37 @@ record Precondition(boolean canRun, String disabledReason) {}
151164
directExecutor());
152165
}
153166

167+
/**
168+
* Converts downstream exceptions into {@link FeServiceException} so that the gRPC/AF boundary
169+
* (FeGrpcInvoker / SafeExceptionHandler) maps them to the correct HTTP status code.
170+
*
171+
* <p>If the exception is already a {@code FeServiceException} (e.g. from validation above), it is
172+
* re-thrown as-is. gRPC {@link StatusRuntimeException}s from the lab server RPC preserve their
173+
* original code. All other exceptions become {@code INTERNAL}.
174+
*/
175+
private static FeServiceException toFeServiceException(String hostName, Exception e) {
176+
if (e instanceof FeServiceException feEx) {
177+
return feEx;
178+
}
179+
if (e instanceof StatusRuntimeException sre) {
180+
Status.Code code = sre.getStatus().getCode();
181+
String description = sre.getStatus().getDescription();
182+
String message =
183+
String.format(
184+
"Troubleshoot script failed on host %s: %s%s",
185+
hostName, code, description != null ? " — " + description : "");
186+
return new FeServiceException(code, message, sre);
187+
}
188+
logger.atWarning().withCause(e).log(
189+
"Unexpected error running troubleshoot script on host %s", hostName);
190+
return new FeServiceException(
191+
Status.Code.INTERNAL,
192+
String.format(
193+
"Troubleshoot script failed on host %s: %s",
194+
hostName, e.getMessage() != null ? e.getMessage() : e.getClass().getSimpleName()),
195+
e);
196+
}
197+
154198
/** Returns true if no devices are BUSY on the host, meaning it is safe to run RESET_USB_HUB. */
155199
private static boolean canRunResetUsbHub(GetLabInfoResponse labInfoResponse) {
156200
return labInfoResponse.getLabQueryResult().getLabView().getLabDataList().stream()

‎src/javatests/com/google/devtools/mobileharness/fe/v6/service/host/handlers/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ java_library(
3737
"//src/devtools/mobileharness/fe/v6/service/proto/host:host_service_java_proto",
3838
"//src/devtools/mobileharness/infra/master/rpc/proto:lab_sync_service_java_proto",
3939
"//src/devtools/mobileharness/shared/labinfo/proto:lab_info_service_java_proto",
40-
"//src/java/com/google/devtools/mobileharness/api/model/error",
4140
"//src/java/com/google/devtools/mobileharness/fe/v6/service/device/handlers",
4241
"//src/java/com/google/devtools/mobileharness/fe/v6/service/device/provider",
4342
"//src/java/com/google/devtools/mobileharness/fe/v6/service/errors",

‎src/javatests/com/google/devtools/mobileharness/fe/v6/service/host/handlers/TroubleshootScriptHandlerTest.java‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,17 @@
1717
package com.google.devtools.mobileharness.fe.v6.service.host.handlers;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static com.google.common.util.concurrent.Futures.immediateFailedFuture;
2021
import static com.google.common.util.concurrent.Futures.immediateFuture;
2122
import static org.junit.Assert.assertThrows;
2223
import static org.mockito.ArgumentMatchers.any;
2324
import static org.mockito.ArgumentMatchers.anyString;
2425
import static org.mockito.Mockito.when;
2526

26-
import com.google.devtools.mobileharness.api.model.error.MobileHarnessException;
2727
import com.google.devtools.mobileharness.api.model.proto.Device.DeviceStatus;
2828
import com.google.devtools.mobileharness.api.query.proto.LabQueryProto;
2929
import com.google.devtools.mobileharness.fe.v6.service.device.provider.DeviceOpsStubProvider;
30+
import com.google.devtools.mobileharness.fe.v6.service.errors.FeServiceException;
3031
import com.google.devtools.mobileharness.fe.v6.service.proto.host.ListTroubleshootScriptsRequest;
3132
import com.google.devtools.mobileharness.fe.v6.service.proto.host.ListTroubleshootScriptsResponse;
3233
import com.google.devtools.mobileharness.fe.v6.service.proto.host.RunTroubleshootScriptRequest;
@@ -37,6 +38,7 @@
3738
import com.google.devtools.mobileharness.infra.lab.rpc.stub.DeviceOpsStub;
3839
import com.google.devtools.mobileharness.shared.labinfo.proto.LabInfoServiceProto.GetLabInfoResponse;
3940
import com.google.wireless.qa.mobileharness.lab.proto.DeviceOpsServ;
41+
import io.grpc.Status;
4042
import java.util.concurrent.ExecutionException;
4143
import org.junit.Before;
4244
import org.junit.Rule;
@@ -94,7 +96,7 @@ public void runTroubleshootScript_noDeviceBusy_success() throws Exception {
9496
}
9597

9698
@Test
97-
public void runTroubleshootScript_unsupportedScript_fails() throws Exception {
99+
public void runTroubleshootScript_unsupportedScript_throwsInvalidArgument() throws Exception {
98100
RunTroubleshootScriptRequest request =
99101
RunTroubleshootScriptRequest.newBuilder()
100102
.setHostName("host")
@@ -104,8 +106,57 @@ public void runTroubleshootScript_unsupportedScript_fails() throws Exception {
104106
ExecutionException e =
105107
assertThrows(
106108
ExecutionException.class, () -> handler.runTroubleshootScript(request, UNIVERSE).get());
107-
assertThat(e).hasCauseThat().isInstanceOf(MobileHarnessException.class);
108-
assertThat(e).hasCauseThat().hasMessageThat().contains("Unsupported troubleshoot script");
109+
assertThat(e).hasCauseThat().isInstanceOf(FeServiceException.class);
110+
FeServiceException feEx = (FeServiceException) e.getCause();
111+
assertThat(feEx.getCode()).isEqualTo(Status.Code.INVALID_ARGUMENT);
112+
assertThat(feEx).hasMessageThat().contains("Unsupported troubleshoot script");
113+
}
114+
115+
@Test
116+
public void runTroubleshootScript_downstreamRpcFails_wrapsAsFeServiceException()
117+
throws Exception {
118+
RunTroubleshootScriptRequest request =
119+
RunTroubleshootScriptRequest.newBuilder()
120+
.setHostName("host")
121+
.setScript(TroubleshootScript.RESET_USB_HUB)
122+
.build();
123+
124+
when(deviceOpsStubProvider.createStub(anyString(), any())).thenReturn(deviceOpsStub);
125+
when(deviceOpsStub.runTroubleshootScriptAsync(any()))
126+
.thenReturn(
127+
immediateFailedFuture(
128+
Status.UNAVAILABLE.withDescription("Lab server unreachable").asRuntimeException()));
129+
130+
ExecutionException e =
131+
assertThrows(
132+
ExecutionException.class, () -> handler.runTroubleshootScript(request, UNIVERSE).get());
133+
assertThat(e).hasCauseThat().isInstanceOf(FeServiceException.class);
134+
FeServiceException feEx = (FeServiceException) e.getCause();
135+
assertThat(feEx.getCode()).isEqualTo(Status.Code.UNAVAILABLE);
136+
assertThat(feEx).hasMessageThat().contains("host");
137+
assertThat(feEx).hasMessageThat().contains("Lab server unreachable");
138+
}
139+
140+
@Test
141+
public void runTroubleshootScript_unexpectedException_wrapsAsInternal() throws Exception {
142+
RunTroubleshootScriptRequest request =
143+
RunTroubleshootScriptRequest.newBuilder()
144+
.setHostName("myhost")
145+
.setScript(TroubleshootScript.RESET_USB_HUB)
146+
.build();
147+
148+
when(deviceOpsStubProvider.createStub(anyString(), any())).thenReturn(deviceOpsStub);
149+
when(deviceOpsStub.runTroubleshootScriptAsync(any()))
150+
.thenReturn(immediateFailedFuture(new RuntimeException("something broke")));
151+
152+
ExecutionException e =
153+
assertThrows(
154+
ExecutionException.class, () -> handler.runTroubleshootScript(request, UNIVERSE).get());
155+
assertThat(e).hasCauseThat().isInstanceOf(FeServiceException.class);
156+
FeServiceException feEx = (FeServiceException) e.getCause();
157+
assertThat(feEx.getCode()).isEqualTo(Status.Code.INTERNAL);
158+
assertThat(feEx).hasMessageThat().contains("myhost");
159+
assertThat(feEx).hasMessageThat().contains("something broke");
109160
}
110161

111162
@Test
@@ -137,7 +188,7 @@ public void listTroubleshootScripts_deviceBusy_allDisabled() throws Exception {
137188
}
138189

139190
@Test
140-
public void runTroubleshootScript_routedUniverse_fails() throws Exception {
191+
public void runTroubleshootScript_routedUniverse_throwsUnimplemented() throws Exception {
141192
RunTroubleshootScriptRequest request =
142193
RunTroubleshootScriptRequest.newBuilder()
143194
.setHostName("host")
@@ -151,12 +202,14 @@ public void runTroubleshootScript_routedUniverse_fails() throws Exception {
151202
handler
152203
.runTroubleshootScript(request, new UniverseScope.RoutedUniverse("partner"))
153204
.get());
154-
assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
155-
assertThat(e).hasCauseThat().hasMessageThat().contains("self universe");
205+
assertThat(e).hasCauseThat().isInstanceOf(FeServiceException.class);
206+
FeServiceException feEx = (FeServiceException) e.getCause();
207+
assertThat(feEx.getCode()).isEqualTo(Status.Code.UNIMPLEMENTED);
208+
assertThat(feEx).hasMessageThat().contains("self universe");
156209
}
157210

158211
@Test
159-
public void listTroubleshootScripts_routedUniverse_fails() throws Exception {
212+
public void listTroubleshootScripts_routedUniverse_throwsUnimplemented() throws Exception {
160213
ListTroubleshootScriptsRequest request =
161214
ListTroubleshootScriptsRequest.newBuilder().setHostName("host").build();
162215

@@ -167,8 +220,10 @@ public void listTroubleshootScripts_routedUniverse_fails() throws Exception {
167220
handler
168221
.listTroubleshootScripts(request, new UniverseScope.RoutedUniverse("partner"))
169222
.get());
170-
assertThat(e).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
171-
assertThat(e).hasCauseThat().hasMessageThat().contains("self universe");
223+
assertThat(e).hasCauseThat().isInstanceOf(FeServiceException.class);
224+
FeServiceException feEx = (FeServiceException) e.getCause();
225+
assertThat(feEx.getCode()).isEqualTo(Status.Code.UNIMPLEMENTED);
226+
assertThat(feEx).hasMessageThat().contains("self universe");
172227
}
173228

174229
private static GetLabInfoResponse buildLabInfoWithBusyDevice() {

0 commit comments

Comments
 (0)