Skip to content

Commit

Permalink
mobile: Add respones codes to the error details
Browse files Browse the repository at this point in the history
The HTTP response code is not reported in the Cronvoy exceptions, and
the error_code, which maps from some small subset of response codes to
envoy_error_code_t, is not used in the onError functions. As a first
step, we want to append the HTTP response code and the Envoy error code
to the error details.

A subsequent PR will add more support for mapping Http::Code to
envoy_error_code_t (or getting rid of envoy_error_code_t all together).

Signed-off-by: Ali Beyad <abeyad@google.com>
  • Loading branch information
abeyad committed May 9, 2024
1 parent 02c9e15 commit 701700b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
15 changes: 14 additions & 1 deletion mobile/library/common/http/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include "source/common/http/headers.h"
#include "source/common/http/utility.h"

#include "absl/strings/str_cat.h"
#include "absl/strings/str_join.h"
#include "library/common/bridge/utility.h"
#include "library/common/data/utility.h"
#include "library/common/http/header_utility.h"
Expand Down Expand Up @@ -437,16 +439,27 @@ void Client::DirectStreamCallbacks::latchError() {
}
const auto& info = request_decoder->streamInfo();

std::vector<std::string> error_msg_details;
if (info.responseCode().has_value()) {
error_->error_code = Bridge::Utility::errorCodeFromLocalStatus(
static_cast<Http::Code>(info.responseCode().value()));
error_msg_details.push_back(absl::StrCat("RC: ", info.responseCode().value()));
} else if (StreamInfo::isStreamIdleTimeout(info)) {
error_->error_code = ENVOY_REQUEST_TIMEOUT;
} else {
error_->error_code = ENVOY_STREAM_RESET;
}

error_->message = info.responseCodeDetails().value_or("");
error_msg_details.push_back(absl::StrCat("EC: ", error_->error_code));
if (info.responseCodeDetails().has_value()) {
error_msg_details.push_back(info.responseCodeDetails().value());
}
// The format of the error message propogated to callbacks is:
// {RESPONSE_CODE}|{ERROR_CODE}|{DETAILS}
// Where RESPONSE_CODE is the HTTP response code from StreamInfo::responseCode().
// ERROR_CODE is of the envoy_error_code_t enum type, and gets mapped from RESPONSE_CODE.
// DETAILS is the contents of StreamInfo::responseCodeDetails().
error_->message = absl::StrJoin(std::move(error_msg_details), "|");
error_->attempt_count = info.attemptCount().value_or(0);
}

Expand Down
6 changes: 4 additions & 2 deletions mobile/test/common/http/client_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

using testing::_;
using testing::AnyNumber;
using testing::ContainsRegex;
using testing::NiceMock;
using testing::Return;
using testing::ReturnPointee;
Expand Down Expand Up @@ -444,6 +445,7 @@ TEST_P(ClientTest, EnvoyLocalError) {
stream_callbacks.on_error_ = [&](EnvoyError error, envoy_stream_intel,
envoy_final_stream_intel) -> void {
EXPECT_EQ(error.error_code, ENVOY_CONNECTION_FAILURE);
EXPECT_THAT(error.message, ContainsRegex("503|2|failed miserably"));
EXPECT_EQ(error.attempt_count, 123);
callbacks_called.on_error_calls_++;
};
Expand All @@ -463,7 +465,7 @@ TEST_P(ClientTest, EnvoyLocalError) {
EXPECT_CALL(dispatcher_, popTrackedObject(_));
EXPECT_CALL(dispatcher_, deferredDelete_(_));
stream_info_.setResponseCode(503);
stream_info_.setResponseCodeDetails("nope");
stream_info_.setResponseCodeDetails("failed miserably");
stream_info_.setAttemptCount(123);
response_encoder_->getStream().resetStream(Http::StreamResetReason::LocalConnectionFailure);
ASSERT_EQ(callbacks_called.on_headers_calls_, 0);
Expand Down Expand Up @@ -518,7 +520,7 @@ TEST_P(ClientTest, RemoteResetAfterStreamStart) {
stream_callbacks.on_error_ = [&](EnvoyError error, envoy_stream_intel,
envoy_final_stream_intel) -> void {
EXPECT_EQ(error.error_code, ENVOY_STREAM_RESET);
EXPECT_EQ(error.message.length(), 0);
EXPECT_THAT(error.message, ContainsRegex("1"));
EXPECT_EQ(error.attempt_count, 0);
callbacks_called.on_error_calls_++;
};
Expand Down

0 comments on commit 701700b

Please sign in to comment.