0

Return grpc::Status in GrpcServer::Start API.

This change allows the callers to control what to do if gRPC server
fails to start, as opposed to crashing the whole process.

Bug: b:297925861
Test: UTs
Change-Id: Ie3ae247a493648adab5a37f3e598002ac4ccd3ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4824199
Reviewed-by: Mohamed Omar <mohamedaomar@google.com>
Commit-Queue: Vigen Issahhanjan <vigeni@google.com>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1190917}
This commit is contained in:
Vigen Issahhanjan
2023-08-31 19:22:21 +00:00
committed by Chromium LUCI CQ
parent f3ee4f773f
commit 37bcd47817
9 changed files with 95 additions and 19 deletions

@ -51,6 +51,7 @@ source_set("unit_tests") {
sources = [
"grpc_server_streaming_test.cc",
"grpc_server_test.cc",
"grpc_status_or_test.cc",
"grpc_unary_test.cc",
]

@ -63,7 +63,7 @@ GrpcServer::~GrpcServer() {
DCHECK(!server_) << "gRPC server must be explicitly stopped";
}
void GrpcServer::Start(const std::string& endpoint) {
grpc::Status GrpcServer::Start(const std::string& endpoint) {
DCHECK(!server_) << "Server is already running";
DCHECK(server_reactor_tracker_) << "Server was alreadys shutdown";
@ -71,8 +71,11 @@ void GrpcServer::Start(const std::string& endpoint) {
.AddListeningPort(endpoint, grpc::InsecureServerCredentials())
.RegisterCallbackGenericService(this)
.BuildAndStart();
CHECK(server_) << "Failed to start server at " << endpoint;
if (!server_) {
return grpc::Status(grpc::StatusCode::INTERNAL,
"can't start gRPC server on " + endpoint);
}
return grpc::Status::OK;
}
void GrpcServer::Stop() {

@ -87,7 +87,7 @@ class GrpcServer : public grpc::CallbackGenericService {
}
// Starts the gRPC server.
void Start(const std::string& endpoint);
ABSL_MUST_USE_RESULT grpc::Status Start(const std::string& endpoint);
// Stops the gRPC server synchronously. May block indefinitely if there's a
// non-finished pending reactor created by the gRPC framework.

@ -81,7 +81,7 @@ TEST_F(GrpcServerStreamingTest, ServerStreamingCallSucceeds) {
GrpcServer server;
server.SetHandler<ServerStreamingServiceHandler::StreamingCall>(
std::move(call_handler));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
ServerStreamingServiceStub stub(endpoint_);
auto call = stub.CreateCall<ServerStreamingServiceStub::StreamingCall>();
@ -114,7 +114,7 @@ TEST_F(GrpcServerStreamingTest, ServerStreamingCallFailsRightAway) {
reactor->Write(
grpc::Status(grpc::StatusCode::NOT_FOUND, "not found"));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
ServerStreamingServiceStub stub(endpoint_);
auto call = stub.CreateCall<ServerStreamingServiceStub::StreamingCall>();
@ -144,7 +144,7 @@ TEST_F(GrpcServerStreamingTest, ServerStreamingCallCancelledIfServerIsStopped) {
base::BindLambdaForTesting(
[&]() { server_stopped_event.Signal(); }));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
ServerStreamingServiceStub stub(endpoint_);
auto call = stub.CreateCall<ServerStreamingServiceStub::StreamingCall>();
@ -196,7 +196,7 @@ TEST_F(GrpcServerStreamingTest, DISABLED_ServerStreamingCallIsCancelledByClient)
GrpcServer server;
server.SetHandler<ServerStreamingServiceHandler::StreamingCall>(
std::move(call_handler));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
size_t response_count = 0;
base::WaitableEvent response_received_event{

@ -0,0 +1,54 @@
// Copyright 2021 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromecast/cast_core/grpc/grpc_server.h"
#include "base/test/bind.h"
#include "base/uuid.h"
#include "chromecast/cast_core/grpc/status_matchers.h"
#include "chromecast/cast_core/grpc/test_service.castcore.pb.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace cast {
namespace utils {
namespace {
using ::cast::test::StatusIs;
class GrpcServerTest : public ::testing::Test {
protected:
GrpcServerTest() = default;
const std::string endpoint_ =
"unix-abstract:cast-uds-" +
base::Uuid::GenerateRandomV4().AsLowercaseString();
};
TEST_F(GrpcServerTest, FailedStartReturnsError) {
GrpcServer server1;
server1.SetHandler<SimpleServiceHandler::SimpleCall>(
base::BindLambdaForTesting(
[](TestRequest, SimpleServiceHandler::SimpleCall::Reactor* reactor) {
reactor->Write(TestResponse());
}));
ASSERT_THAT(server1.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
// Verify server has started.
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
auto response_or = std::move(call).Invoke();
CU_ASSERT_OK(response_or);
// Verify that 2nd server on the same endpoint cannot be created.
GrpcServer server2;
server2.SetHandler<SimpleServiceHandler::SimpleCall>(
base::BindLambdaForTesting(
[](TestRequest, SimpleServiceHandler::SimpleCall::Reactor*) {}));
EXPECT_THAT(server2.Start(endpoint_), StatusIs(grpc::StatusCode::INTERNAL));
server1.Stop();
}
} // namespace
} // namespace utils
} // namespace cast

@ -62,7 +62,7 @@ TEST_F(GrpcUnaryTest, SyncUnaryCallSucceeds) {
response.set_bar("test_bar");
reactor->Write(std::move(response));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -84,7 +84,7 @@ TEST_F(GrpcUnaryTest, SyncUnaryCallReturnsErrorStatus) {
reactor->Write(
grpc::Status(grpc::StatusCode::NOT_FOUND, "Not found"));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -108,7 +108,7 @@ TEST_F(GrpcUnaryTest, SyncUnaryCallCancelledIfServerIsStopped) {
base::BindLambdaForTesting(
[&]() { server_stopped_event.Signal(); }));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -131,7 +131,7 @@ TEST_F(GrpcUnaryTest, AsyncUnaryCallSucceeds) {
response.set_bar("test_bar");
reactor->Write(std::move(response));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -158,7 +158,7 @@ TEST_F(GrpcUnaryTest, AsyncUnaryCallReturnsErrorStatus) {
reactor->Write(
grpc::Status(grpc::StatusCode::NOT_FOUND, "Not Found"));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -187,7 +187,7 @@ TEST_F(GrpcUnaryTest, AsyncUnaryCallCancelledIfServerIsStopped) {
base::BindLambdaForTesting(
[&]() { server_stopped_event.Signal(); }));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();
@ -215,7 +215,7 @@ TEST_F(GrpcUnaryTest, SyncUnaryCallSucceedsExtra) {
response.set_bar("test_bar");
reactor->Write(std::move(response));
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceExtraStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceExtraStub::SimpleCall>();
@ -245,7 +245,7 @@ TEST_F(GrpcUnaryTest, DISABLED_AsyncUnaryCallCancelledByClient) {
EXPECT_EQ(request.foo(), "test_foo");
request_received_event.Signal();
}));
server.Start(endpoint_);
ASSERT_THAT(server.Start(endpoint_), StatusIs(grpc::StatusCode::OK));
SimpleServiceStub stub(endpoint_);
auto call = stub.CreateCall<SimpleServiceStub::SimpleCall>();

@ -189,8 +189,17 @@ void RuntimeApplicationServiceImpl::Load(
task_runner_,
base::BindRepeating(&RuntimeApplicationServiceImpl::HandlePostMessage,
weak_factory_.GetWeakPtr())));
grpc_server_->Start(
auto status = grpc_server_->Start(
request.runtime_application_service_info().grpc_endpoint());
if (!status.ok()) {
LOG(ERROR) << "Failed to start runtime application server: status="
<< status.error_message();
std::move(callback).Run(cast_receiver::Status(
cast_receiver::StatusCode::kInternal, status.error_message()));
return;
}
LOG(INFO) << "Runtime application server started: endpoint="
<< request.runtime_application_service_info().grpc_endpoint();

@ -89,7 +89,12 @@ cast_receiver::Status RuntimeServiceImpl::Start() {
base::BindRepeating(
&RuntimeServiceImpl::HandleStopMetricsRecorder,
weak_factory_.GetWeakPtr())));
grpc_server_->Start(runtime_service_endpoint_);
auto status = grpc_server_->Start(runtime_service_endpoint_);
// Browser runtime must crash if the runtime service failed to start to avoid
// the process to dangle without any proper connection to the Cast Core.
CHECK(status.ok()) << "Failed to start runtime service: status="
<< status.error_message();
LOG(INFO) << "Runtime service started";
return cast_receiver::OkStatus();

@ -268,7 +268,11 @@ void FakeDMServer::StartGrpcServer() {
base::SingleThreadTaskRunner::GetCurrentDefault(),
base::BindRepeating(&FakeDMServer::HandleWaitRemoteCommandResult,
weak_ptr_factory_.GetWeakPtr())));
grpc_server_->Start(grpc_unix_socket_uri_);
auto status = grpc_server_->Start(grpc_unix_socket_uri_);
// Browser runtime must crash if the runtime service failed to start to avoid
// the process to dangle without any proper connection to the Cast Core.
CHECK(status.ok()) << "Failed to start DM gRPC server: status="
<< status.error_message();
}
void FakeDMServer::HandleSendRemoteCommand(