Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,14 @@ def _com_google_absl():
name = "abseil_algorithm",
actual = "@com_google_absl//absl/algorithm:algorithm",
)
native.bind(
name = "abseil_variant",
actual = "@com_google_absl//absl/types:variant",
)
native.bind(
name = "abseil_status",
actual = "@com_google_absl//absl/status",
)

def _com_google_protobuf():
_repository_impl("rules_python")
Expand Down
7 changes: 6 additions & 1 deletion ci/run_clang_tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ function exclude_chromium_url() {
grep -v source/common/chromium_url/
}

# Exclude files in third_party which are temporary forks from other OSS projects.
function exclude_third_party() {
grep -v third_party/
}

function filter_excludes() {
exclude_testdata | exclude_chromium_url | exclude_win32_impl
exclude_testdata | exclude_chromium_url | exclude_win32_impl | exclude_third_party
}


Expand Down
8 changes: 8 additions & 0 deletions source/common/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,11 @@ envoy_cc_library(
"//source/common/buffer:buffer_lib",
],
)

envoy_cc_library(
name = "statusor_lib",
hdrs = ["statusor.h"],
deps = [
"//third_party/statusor:statusor_lib",
],
)
42 changes: 42 additions & 0 deletions source/common/common/statusor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#pragma once

#include "third_party/statusor/statusor.h"

/**
* Facility for returning either a valid value or an error in a form of Envoy::Status.
*
* IMPORTANT: StatusOr default constructor must not be used as it does not fit into any
* Envoy's use cases. The error extracting functions in the common/common/status.h will
* RELEASE_ASSERT on default initialized StatusOr.
*
* To return an error StatusOr object an error creating function from common/common/status.h must be
* used.
* TODO(yanavlasov): add clang-tidy or lint check to enforce this.
*
* Usage example:
*
* Envoy::StatusOr<int> Foo() {
* ...
* if (codec_error) {
* return CodecProtocolError("Invalid protocol");
* }
* return 123456;
* }
*
* void Bar() {
* auto status_or = Foo();
* if (status_or.ok()) {
* int result = status_or.value();
* ...
* } else {
* ASSERT(IsCodecProtocolError(status_or.status()));
* ENVOY_LOG(debug, "Codec error encountered: {}", status_or.status().message());
* }
* }
*/

namespace Envoy {

using absl::StatusOr; // NOLINT(misc-unused-using-decls)

} // namespace Envoy
12 changes: 12 additions & 0 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -412,3 +412,15 @@ envoy_cc_library(
"@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",
],
)

envoy_cc_library(
name = "status_lib",
srcs = ["status.cc"],
hdrs = ["status.h"],
external_deps = [
"abseil_status",
],
deps = [
"//include/envoy/http:codes_interface",
],
)
123 changes: 123 additions & 0 deletions source/common/http/status.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
#include "common/http/status.h"

#include "absl/strings/str_cat.h"

namespace Envoy {
namespace Http {

namespace {

constexpr absl::string_view EnvoyPayloadUrl = "Envoy";

absl::string_view statusCodeToString(StatusCode code) {
switch (code) {
case StatusCode::Ok:
return absl::OkStatus().ToString();
case StatusCode::CodecProtocolError:
return "CodecProtocolError";
case StatusCode::BufferFloodError:
return "BufferFloodError";
case StatusCode::PrematureResponseError:
return "PrematureResponseError";
case StatusCode::CodecClientError:
return "CodecClientError";
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're getting compilation issues on Windows as this helper function does not return a value in all cases, we're going to make a PR to fix this and add a RELEASE_ASSERT in the default case (and change all all the below ASSERTs to RELEASE_ASSERTs), if an exception is more appropriate, please let us know

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation of getStatusCode mentions a RELEASE_ASSERT but is using ASSERT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disregard the part about ASSERT -> RELEASE_ASSERT as I see the comments now asking to do the opposite

}

struct EnvoyStatusPayload {
EnvoyStatusPayload(StatusCode status_code) : status_code_(status_code) {}
const StatusCode status_code_;
};

struct PrematureResponsePayload : public EnvoyStatusPayload {
PrematureResponsePayload(Http::Code http_code)
: EnvoyStatusPayload(StatusCode::PrematureResponseError), http_code_(http_code) {}
const Http::Code http_code_;
};

template <typename T> void storePayload(absl::Status& status, const T& payload) {
status.SetPayload(
EnvoyPayloadUrl,
absl::Cord(absl::string_view(reinterpret_cast<const char*>(&payload), sizeof(payload))));
}

template <typename T = EnvoyStatusPayload> const T* getPayload(const absl::Status& status) {
auto payload = status.GetPayload(EnvoyPayloadUrl);
ASSERT(payload.has_value(), "Must have payload");
auto data = payload.value().Flatten();
ASSERT(data.length() >= sizeof(T), "Invalid payload length");
return reinterpret_cast<const T*>(data.data());
}

} // namespace

std::string toString(const Status& status) {
if (status.ok()) {
return status.ToString();
}
std::string text;
auto status_code = getStatusCode(status);
if (status_code != StatusCode::PrematureResponseError) {
absl::StrAppend(&text, statusCodeToString(status_code), ": ", status.message());
} else {
auto http_code = getPrematureResponseHttpCode(status);
absl::StrAppend(&text, "PrematureResponseError: HTTP code: ", http_code, ": ",
status.message());
}
return text;
}

Status codecProtocolError(absl::string_view message) {
absl::Status status(absl::StatusCode::kInternal, message);
storePayload(status, EnvoyStatusPayload(StatusCode::CodecProtocolError));
return status;
}

Status bufferFloodError(absl::string_view message) {
absl::Status status(absl::StatusCode::kInternal, message);
storePayload(status, EnvoyStatusPayload(StatusCode::BufferFloodError));
return status;
}

Status prematureResponseError(absl::string_view message, Http::Code http_code) {
absl::Status status(absl::StatusCode::kInternal, message);
storePayload(status, PrematureResponsePayload(http_code));
return status;
}

Status codecClientError(absl::string_view message) {
absl::Status status(absl::StatusCode::kInternal, message);
storePayload(status, EnvoyStatusPayload(StatusCode::CodecClientError));
return status;
}

// Methods for checking and extracting error information
StatusCode getStatusCode(const Status& status) {
return status.ok() ? StatusCode::Ok : getPayload(status)->status_code_;
}

bool isCodecProtocolError(const Status& status) {
return getStatusCode(status) == StatusCode::CodecProtocolError;
}

bool isBufferFloodError(const Status& status) {
return getStatusCode(status) == StatusCode::BufferFloodError;
}

bool isPrematureResponseError(const Status& status) {
return getStatusCode(status) == StatusCode::PrematureResponseError;
}

Http::Code getPrematureResponseHttpCode(const Status& status) {
const auto* payload = getPayload<PrematureResponsePayload>(status);
ASSERT(payload->status_code_ == StatusCode::PrematureResponseError,
"Must be PrematureResponseError");
return payload->http_code_;
}

bool isCodecClientError(const Status& status) {
return getStatusCode(status) == StatusCode::CodecClientError;
}

} // namespace Http
} // namespace Envoy
113 changes: 113 additions & 0 deletions source/common/http/status.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
#pragma once

#include <atomic>
#include <string>

#include "envoy/http/codes.h"

#include "absl/status/status.h"
#include "absl/strings/string_view.h"

/**
* Facility for returning rich error information.
* This facility is to be used in place of exceptions, in components where
* exceptions safety is not guaranteed (i.e. codecs).
*
* Envoy::Status is an alias of absl::Status.
* IMPORTANT: `absl::Status` constructor `absl::Status::code()` and absl::Status::ToString()`
* methods must not be used as they will not return correct error information. Instead the error
* value creating and corresponding error checking functions defined below must be used.
* TODO(yanavlasov): add clang-tidy or lint check to enforce this.
*
* Usage example:
*
* Envoy::Status Foo() {
* ...
* if (codec_error) {
* return CodecProtocolError("Invalid protocol");
* }
* return Envoy::OkStatus();
* }
*
* void Bar() {
* auto status = Foo();
* if (status.ok()) {
* ...
* } else {
* ASSERT(IsCodecProtocolError(status));
* ENVOY_LOG(debug, "Codec error encountered: {}", status.message());
* }
* }
*/

namespace Envoy {
namespace Http {

/**
* Status codes for representing classes of Envoy errors.
*/
enum class StatusCode : int {
Ok = 0,

/**
* Indicates a non-recoverable protocol error that should result in connection termination.
*/
CodecProtocolError = 1,

/**
* Indicates detection of outbound frame queue flood.
*/
BufferFloodError = 2,

/**
* Indicates a response is received on a connection that did not send a request. In practice
* this can only happen on HTTP/1.1 connections.
*/
PrematureResponseError = 3,

/**
* Indicates a client (local) side error which should not happen.
*/
CodecClientError = 4
};

using Status = absl::Status;

inline Status okStatus() { return absl::OkStatus(); }

/**
* Returns the combination of the error code name, message and any additional error attributes.
*/
std::string toString(const Status& status);

/**
* Functions for creating error values. The error code of the returned status object matches the
* name of the function.
*/
Status codecProtocolError(absl::string_view message);
Status bufferFloodError(absl::string_view message);
Status prematureResponseError(absl::string_view message, Http::Code http_code);
Status codecClientError(absl::string_view message);

/**
* Returns Envoy::StatusCode of the given status object.
* If the status object does not contain valid Envoy::Status value the function will RELEASE_ASSERT.
*/
StatusCode getStatusCode(const Status& status);

/**
* Returns true if the given status matches error code implied by the name of the function.
*/
ABSL_MUST_USE_RESULT bool isCodecProtocolError(const Status& status);
ABSL_MUST_USE_RESULT bool isBufferFloodError(const Status& status);
ABSL_MUST_USE_RESULT bool isPrematureResponseError(const Status& status);
ABSL_MUST_USE_RESULT bool isCodecClientError(const Status& status);

/**
* Returns Http::Code value of the PrematureResponseError status.
* IsPrematureResponseError(status) must be true which is checked by RELEASE_ASSERT.
*/
Http::Code getPrematureResponseHttpCode(const Status& status);

} // namespace Http
} // namespace Envoy
9 changes: 9 additions & 0 deletions test/common/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,12 @@ envoy_cc_test(
"//source/common/common:version_lib",
],
)

envoy_cc_test(
name = "statusor_test",
srcs = ["statusor_test.cc"],
deps = [
"//source/common/common:statusor_lib",
"//source/common/http:status_lib",
],
)
19 changes: 19 additions & 0 deletions test/common/common/statusor_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include "common/common/statusor.h"
#include "common/http/status.h"

#include "gmock/gmock.h"
#include "gtest/gtest.h"

namespace Envoy {

TEST(StatusOr, Initialization) {
StatusOr<int> statusor(
Http::prematureResponseError("foobar", Http::Code::ProxyAuthenticationRequired));
EXPECT_FALSE(statusor.ok());
EXPECT_TRUE(Http::isPrematureResponseError(statusor.status()));
EXPECT_EQ("foobar", statusor.status().message());
EXPECT_EQ(Http::Code::ProxyAuthenticationRequired,
Http::getPrematureResponseHttpCode(statusor.status()));
}

} // namespace Envoy
8 changes: 8 additions & 0 deletions test/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -384,3 +384,11 @@ envoy_cc_test(
"//test/test_common:utility_lib",
],
)

envoy_cc_test(
name = "status_test",
srcs = ["status_test.cc"],
deps = [
"//source/common/http:status_lib",
],
)
Loading