Convert call quality gRPC service to new error model

This commit is contained in:
Ravi Khadiwala
2026-01-26 12:52:52 -06:00
committed by ravi-signal
parent 517c5b8056
commit 8023a9346f
8 changed files with 50 additions and 24 deletions

View File

@@ -29,6 +29,7 @@ import org.whispersystems.textsecuregcm.auth.AuthenticatedDevice;
import org.whispersystems.textsecuregcm.filters.RemoteAddressFilter;
import org.whispersystems.textsecuregcm.limits.RateLimitedByIp;
import org.whispersystems.textsecuregcm.limits.RateLimiters;
import org.whispersystems.textsecuregcm.metrics.CallQualityInvalidArgumentsException;
import org.whispersystems.textsecuregcm.metrics.CallQualitySurveyManager;
@Path("/v1/call_quality_survey")
@@ -73,7 +74,7 @@ public class CallQualitySurveyController {
try {
callQualitySurveyManager.submitCallQualitySurvey(submitCallQualitySurveyRequest, remoteAddress, userAgentString);
} catch (final IllegalArgumentException e) {
} catch (final CallQualityInvalidArgumentsException e) {
throw new WebApplicationException(e.getMessage(), 422);
}
}

View File

@@ -12,6 +12,7 @@ import org.signal.chat.calling.quality.SubmitCallQualitySurveyRequest;
import org.signal.chat.calling.quality.SubmitCallQualitySurveyResponse;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.limits.RateLimiters;
import org.whispersystems.textsecuregcm.metrics.CallQualityInvalidArgumentsException;
import org.whispersystems.textsecuregcm.metrics.CallQualitySurveyManager;
public class CallQualitySurveyGrpcService extends SimpleCallQualityGrpc.CallQualityImplBase {
@@ -38,8 +39,10 @@ public class CallQualitySurveyGrpcService extends SimpleCallQualityGrpc.CallQual
callQualitySurveyManager.submitCallQualitySurvey(request,
remoteAddress,
RequestAttributesUtil.getUserAgent().orElse(null));
} catch (final IllegalArgumentException e) {
throw Status.INVALID_ARGUMENT.withDescription(e.getMessage()).asRuntimeException();
} catch (final CallQualityInvalidArgumentsException e) {
throw e.getField()
.map(fieldName -> GrpcExceptions.fieldViolation(fieldName, e.getMessage()))
.orElseGet(() -> GrpcExceptions.invalidArguments(e.getMessage()));
}
return SubmitCallQualitySurveyResponse.getDefaultInstance();

View File

@@ -0,0 +1,25 @@
/*
* Copyright 2026 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.whispersystems.textsecuregcm.metrics;
import javax.annotation.Nullable;
import java.util.Optional;
public class CallQualityInvalidArgumentsException extends Exception {
private final @Nullable String field;
public CallQualityInvalidArgumentsException(final String message) {
this(message, null);
}
public CallQualityInvalidArgumentsException(final String message, final String field) {
super(message);
this.field = field;
}
public Optional<String> getField() {
return Optional.ofNullable(field);
}
}

View File

@@ -49,7 +49,7 @@ public class CallQualitySurveyManager {
public void submitCallQualitySurvey(final SubmitCallQualitySurveyRequest submitCallQualitySurveyRequest,
final String remoteAddress,
final String userAgentString) {
final String userAgentString) throws CallQualityInvalidArgumentsException {
validateRequest(submitCallQualitySurveyRequest);
@@ -152,21 +152,21 @@ public class CallQualitySurveyManager {
}
@VisibleForTesting
static void validateRequest(final SubmitCallQualitySurveyRequest request) {
static void validateRequest(final SubmitCallQualitySurveyRequest request) throws CallQualityInvalidArgumentsException {
if (request.getStartTimestamp() == 0) {
throw new IllegalArgumentException("Start timestamp not specified");
throw new CallQualityInvalidArgumentsException("Start timestamp not specified", "startTimestamp");
}
if (request.getEndTimestamp() == 0) {
throw new IllegalArgumentException("End timestamp not specified");
throw new CallQualityInvalidArgumentsException("End timestamp not specified", "endTimestamp");
}
if (StringUtils.isBlank(request.getCallType())) {
throw new IllegalArgumentException("Call type not specified");
throw new CallQualityInvalidArgumentsException("Call type not specified", "callType");
}
if (StringUtils.isBlank(request.getCallEndReason())) {
throw new IllegalArgumentException("Call end reason not specified");
throw new CallQualityInvalidArgumentsException("Call end reason not specified", "callEndReason");
}
}
}

View File

@@ -13,11 +13,6 @@ package org.signal.chat.calling.quality;
service CallQuality {
// Submits a call quality survey response.
//
// This RPC may fail with a `RESOURCE_EXHAUSTED` status if a rate limit for
// submitting survey responses has been exceeded, in which case a
// `retry-after` header containing an ISO 8601 duration string will be present
// in the response trailers.
rpc SubmitCallQualitySurvey(SubmitCallQualitySurveyRequest) returns (SubmitCallQualitySurveyResponse) {}
}

View File

@@ -30,6 +30,7 @@ import org.junit.jupiter.params.provider.MethodSource;
import org.signal.chat.calling.quality.SubmitCallQualitySurveyRequest;
import org.whispersystems.textsecuregcm.auth.AuthenticatedDevice;
import org.whispersystems.textsecuregcm.mappers.RateLimitExceededExceptionMapper;
import org.whispersystems.textsecuregcm.metrics.CallQualityInvalidArgumentsException;
import org.whispersystems.textsecuregcm.metrics.CallQualitySurveyManager;
import org.whispersystems.textsecuregcm.tests.util.AuthHelper;
import org.whispersystems.textsecuregcm.util.SystemMapper;
@@ -60,7 +61,7 @@ class CallQualitySurveyControllerTest {
}
@Test
void submitCallQualitySurvey() {
void submitCallQualitySurvey() throws CallQualityInvalidArgumentsException {
final SubmitCallQualitySurveyRequest request = SubmitCallQualitySurveyRequest.getDefaultInstance();
try (final Response response = RESOURCE_EXTENSION.getJerseyTest()
@@ -75,7 +76,7 @@ class CallQualitySurveyControllerTest {
}
@Test
void submitCallQualitySurveyAuthenticated() {
void submitCallQualitySurveyAuthenticated() throws CallQualityInvalidArgumentsException {
final SubmitCallQualitySurveyRequest request = SubmitCallQualitySurveyRequest.getDefaultInstance();
try (final Response response = RESOURCE_EXTENSION.getJerseyTest()
@@ -91,10 +92,10 @@ class CallQualitySurveyControllerTest {
}
@Test
void submitCallQualitySurveyInvalidArgument() {
void submitCallQualitySurveyInvalidArgument() throws CallQualityInvalidArgumentsException {
final SubmitCallQualitySurveyRequest request = SubmitCallQualitySurveyRequest.getDefaultInstance();
doThrow(new IllegalArgumentException())
doThrow(new CallQualityInvalidArgumentsException("test"))
.when(CALL_QUALITY_SURVEY_MANAGER).submitCallQualitySurvey(request, REMOTE_ADDRESS, USER_AGENT);
try (final Response response = RESOURCE_EXTENSION.getJerseyTest()

View File

@@ -22,6 +22,7 @@ import org.signal.chat.calling.quality.SubmitCallQualitySurveyRequest;
import org.whispersystems.textsecuregcm.controllers.RateLimitExceededException;
import org.whispersystems.textsecuregcm.limits.RateLimiter;
import org.whispersystems.textsecuregcm.limits.RateLimiters;
import org.whispersystems.textsecuregcm.metrics.CallQualityInvalidArgumentsException;
import org.whispersystems.textsecuregcm.metrics.CallQualitySurveyManager;
class CallQualitySurveyGrpcServiceTest extends SimpleBaseGrpcTest<CallQualitySurveyGrpcService, CallQualityGrpc.CallQualityBlockingStub> {
@@ -50,7 +51,7 @@ class CallQualitySurveyGrpcServiceTest extends SimpleBaseGrpcTest<CallQualitySur
}
@Test
void submitCallQualitySurvey() {
void submitCallQualitySurvey() throws CallQualityInvalidArgumentsException {
final SubmitCallQualitySurveyRequest request = SubmitCallQualitySurveyRequest.getDefaultInstance();
assertDoesNotThrow(() -> unauthenticatedServiceStub().submitCallQualitySurvey(request));
@@ -70,10 +71,10 @@ class CallQualitySurveyGrpcServiceTest extends SimpleBaseGrpcTest<CallQualitySur
}
@Test
void submitCallQualitySurveyInvalidArgument() {
void submitCallQualitySurveyInvalidArgument() throws CallQualityInvalidArgumentsException {
final SubmitCallQualitySurveyRequest request = SubmitCallQualitySurveyRequest.getDefaultInstance();
doThrow(new IllegalArgumentException())
doThrow(new CallQualityInvalidArgumentsException("test"))
.when(callQualitySurveyManager).submitCallQualitySurvey(request, REMOTE_ADDRESS, USER_AGENT);
//noinspection ResultOfMethodCallIgnored

View File

@@ -155,10 +155,10 @@ class CallQualitySurveyManagerTest {
if (expectValid) {
assertDoesNotThrow(validateRequest);
} else {
final IllegalArgumentException illegalArgumentException =
assertThrows(IllegalArgumentException.class, validateRequest);
final CallQualityInvalidArgumentsException invalidArgumentsException =
assertThrows(CallQualityInvalidArgumentsException.class, validateRequest);
assertTrue(StringUtils.isNotBlank(illegalArgumentException.getMessage()));
assertTrue(StringUtils.isNotBlank(invalidArgumentsException.getMessage()));
}
}