Skip to content

Commit

Permalink
server: cleaning up some exception throwing (envoyproxy#32750)
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <alyssar@google.com>
  • Loading branch information
alyssawilk authored Mar 8, 2024
1 parent 3a1408b commit 62a0673
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 18 deletions.
33 changes: 18 additions & 15 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,14 @@ bool canBeRegisteredAsInlineHeader(const Http::LowerCaseString& header_name) {
return true;
}

void registerCustomInlineHeadersFromBootstrap(
const envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
absl::Status
registerCustomInlineHeadersFromBootstrap(const envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
for (const auto& inline_header : bootstrap.inline_headers()) {
const Http::LowerCaseString lower_case_name(inline_header.inline_header_name());
if (!canBeRegisteredAsInlineHeader(lower_case_name)) {
throwEnvoyExceptionOrPanic(fmt::format("Header {} cannot be registered as an inline header.",
inline_header.inline_header_name()));
return absl::InvalidArgumentError(
fmt::format("Header {} cannot be registered as an inline header.",
inline_header.inline_header_name()));
}
switch (inline_header.inline_header_type()) {
case envoy::config::bootstrap::v3::CustomInlineHeader::REQUEST_HEADER:
Expand All @@ -342,6 +343,7 @@ void registerCustomInlineHeadersFromBootstrap(
PANIC("not implemented");
}
}
return absl::OkStatus();
}

} // namespace
Expand Down Expand Up @@ -407,7 +409,7 @@ void InstanceBase::initialize(Network::Address::InstanceConstSharedPtr local_add
}
restarter_.initialize(*dispatcher_, *this);
drain_manager_ = component_factory.createDrainManager(*this);
initializeOrThrow(std::move(local_address), component_factory);
THROW_IF_NOT_OK(initializeOrThrow(std::move(local_address), component_factory));
}
END_TRY
MULTI_CATCH(
Expand All @@ -426,8 +428,8 @@ void InstanceBase::initialize(Network::Address::InstanceConstSharedPtr local_add
});
}

void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr local_address,
ComponentFactory& component_factory) {
absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr local_address,
ComponentFactory& component_factory) {
ENVOY_LOG(info, "initializing epoch {} (base id={}, hot restart version={})",
options_.restartEpoch(), restarter_.baseId(), restarter_.version());

Expand All @@ -442,9 +444,9 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
bootstrap_config_update_time_ = time_source_.systemTime();

if (bootstrap_.has_application_log_config()) {
THROW_IF_NOT_OK(
RETURN_IF_NOT_OK(
Utility::assertExclusiveLogFormatMethod(options_, bootstrap_.application_log_config()));
THROW_IF_NOT_OK(Utility::maybeSetApplicationLogFormat(bootstrap_.application_log_config()));
RETURN_IF_NOT_OK(Utility::maybeSetApplicationLogFormat(bootstrap_.application_log_config()));
}

#ifdef ENVOY_PERFETTO
Expand All @@ -468,7 +470,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
tracing_session_ = perfetto::Tracing::NewTrace();
tracing_fd_ = open(pftrace_path.c_str(), O_RDWR | O_CREAT | O_TRUNC, 0600);
if (tracing_fd_ == -1) {
throwEnvoyExceptionOrPanic(
return absl::InvalidArgumentError(
fmt::format("unable to open tracing file {}: {}", pftrace_path, errorDetails(errno)));
}
// Configure the tracing session.
Expand All @@ -485,7 +487,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
}

// Register Custom O(1) headers from bootstrap.
registerCustomInlineHeadersFromBootstrap(bootstrap_);
RETURN_IF_NOT_OK(registerCustomInlineHeadersFromBootstrap(bootstrap_));

ENVOY_LOG(info, "HTTP header map info:");
for (const auto& info : Http::HeaderMapImplUtility::getAllHeaderMapImplInfo()) {
Expand Down Expand Up @@ -537,7 +539,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
version_int = bootstrap_.stats_server_version_override().value();
} else {
if (!StringUtil::atoull(VersionInfo::revision().substr(0, 6).c_str(), version_int, 16)) {
throwEnvoyExceptionOrPanic("compiled GIT SHA is invalid. Invalid build.");
return absl::InvalidArgumentError("compiled GIT SHA is invalid. Invalid build.");
}
}
server_stats_->version_.set(version_int);
Expand Down Expand Up @@ -587,7 +589,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo

absl::Status creation_status;
Configuration::InitialImpl initial_config(bootstrap_, creation_status);
THROW_IF_NOT_OK_REF(creation_status);
RETURN_IF_NOT_OK(creation_status);

// Learn original_start_time_ if our parent is still around to inform us of it.
const auto parent_admin_shutdown_response = restarter_.sendParentAdminShutdownRequest();
Expand Down Expand Up @@ -717,7 +719,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
admin_->startHttpListener(initial_config.admin().accessLogs(), initial_config.admin().address(),
initial_config.admin().socketOptions());
#else
throwEnvoyExceptionOrPanic("Admin address configured but admin support compiled out");
return absl::InvalidArgumentError("Admin address configured but admin support compiled out");
#endif
} else {
ENVOY_LOG(info, "No admin address given, so no admin HTTP server started.");
Expand All @@ -742,7 +744,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
// thread local data per above. See MainImpl::initialize() for why ConfigImpl
// is constructed as part of the InstanceBase and then populated once
// cluster_manager_factory_ is available.
THROW_IF_NOT_OK(config_.initialize(bootstrap_, *this, *cluster_manager_factory_));
RETURN_IF_NOT_OK(config_.initialize(bootstrap_, *this, *cluster_manager_factory_));

// Instruct the listener manager to create the LDS provider if needed. This must be done later
// because various items do not yet exist when the listener manager is created.
Expand Down Expand Up @@ -785,6 +787,7 @@ void InstanceBase::initializeOrThrow(Network::Address::InstanceConstSharedPtr lo
// started and before our own run() loop runs.
main_thread_guard_dog_ = maybeCreateGuardDog("main_thread");
worker_guard_dog_ = maybeCreateGuardDog("workers");
return absl::OkStatus();
}

void InstanceBase::onClusterManagerPrimaryInitializationComplete() {
Expand Down
6 changes: 3 additions & 3 deletions source/server/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,10 @@ class InstanceBase : Logger::Loggable<Logger::Id::main>,
ProtobufTypes::MessagePtr dumpBootstrapConfig();
void flushStatsInternal();
void updateServerStats();
// This does most of the work of initialization, but can throw errors caught
// This does most of the work of initialization, but can throw or return errors caught
// by initialize().
void initializeOrThrow(Network::Address::InstanceConstSharedPtr local_address,
ComponentFactory& component_factory);
absl::Status initializeOrThrow(Network::Address::InstanceConstSharedPtr local_address,
ComponentFactory& component_factory);
void loadServerFlags(const absl::optional<std::string>& flags_path);
void startWorkers();
void terminate();
Expand Down

0 comments on commit 62a0673

Please sign in to comment.