Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log Rewards errors by default and add a feature flag for verbose logging (uplift to 1.25.x) #8717

Merged
merged 1 commit into from
May 18, 2021
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
4 changes: 4 additions & 0 deletions chromium_src/chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ using ntp_background_images::features::kBraveNTPSuperReferralWallpaper;
flag_descriptions::kBravePermissionLifetimeName, \
flag_descriptions::kBravePermissionLifetimeDescription, kOsAll, \
FEATURE_VALUE_TYPE(permissions::features::kPermissionLifetime)}, \
{"brave-rewards-verbose-logging", \
flag_descriptions::kBraveRewardsVerboseLoggingName, \
flag_descriptions::kBraveRewardsVerboseLoggingDescription, kOsDesktop, \
FEATURE_VALUE_TYPE(brave_rewards::features::kVerboseLoggingFeature)}, \
{"brave-rewards-bitflyer", \
flag_descriptions::kBraveRewardsBitflyerName, \
flag_descriptions::kBraveRewardsBitflyerDescription, kOsDesktop, \
Expand Down
8 changes: 8 additions & 0 deletions chromium_src/chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ const char kBraveSyncDescription[] =
const char kBraveIpfsName[] = "Enable IPFS";
const char kBraveIpfsDescription[] =
"Enable native support of IPFS.";
const char kBraveRewardsVerboseLoggingName[] =
"Enable Brave Rewards verbose logging";
const char kBraveRewardsVerboseLoggingDescription[] =
"Enables detailed logging of Brave Rewards system events to a log file "
"stored on your device. Please note that this log file could include "
"information such as browsing history and credentials such as passwords "
"and access tokens depending on your activity. Please do not share it "
"unless asked to by Brave staff.";
const char kBraveRewardsBitflyerName[] = "Enable bitFlyer for Brave Rewards";
const char kBraveRewardsBitflyerDescription[] =
"Enables support for bitFlyer as an external wallet provider for Brave "
Expand Down
2 changes: 2 additions & 0 deletions chromium_src/chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ extern const char kBraveSyncName[];
extern const char kBraveSyncDescription[];
extern const char kBraveIpfsName[];
extern const char kBraveIpfsDescription[];
extern const char kBraveRewardsVerboseLoggingName[];
extern const char kBraveRewardsVerboseLoggingDescription[];
extern const char kBraveRewardsBitflyerName[];
extern const char kBraveRewardsBitflyerDescription[];
extern const char kNativeBraveWalletName[];
Expand Down
20 changes: 10 additions & 10 deletions components/brave_rewards/browser/rewards_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,9 @@ RewardsServiceImpl::RewardsServiceImpl(Profile* profile)
content::URLDataSource::Add(profile_,
std::make_unique<BraveRewardsSource>(profile_));
ready_ = std::make_unique<base::OneShotEvent>();

if (base::FeatureList::IsEnabled(features::kVerboseLoggingFeature))
persist_log_level_ = kDiagnosticLogMaxVerboseLevel;
}

RewardsServiceImpl::~RewardsServiceImpl() {
Expand Down Expand Up @@ -2263,15 +2266,12 @@ void RewardsServiceImpl::WriteDiagnosticLog(const std::string& file,
const int line,
const int verbose_level,
const std::string& message) {
if (ledger_for_testing_ || !should_persist_logs_) {
return;
}

if (resetting_rewards_) {
if (ledger_for_testing_ || resetting_rewards_) {
return;
}

if (verbose_level > kDiagnosticLogMaxVerboseLevel) {
if (verbose_level > kDiagnosticLogMaxVerboseLevel ||
verbose_level > persist_log_level_) {
return;
}

Expand Down Expand Up @@ -2414,13 +2414,13 @@ void RewardsServiceImpl::HandleFlags(const std::string& options) {
continue;
}

// The "persist-logs" command-line flag is deprecated and will be removed
// in a future version. Use --enable-features=BraveRewardsVerboseLogging
// instead.
if (name == "persist-logs") {
const std::string lower = base::ToLowerASCII(value);

if (lower == "true" || lower == "1") {
should_persist_logs_ = true;
} else {
should_persist_logs_ = false;
persist_log_level_ = kDiagnosticLogMaxVerboseLevel;
}
}

Expand Down
2 changes: 1 addition & 1 deletion components/brave_rewards/browser/rewards_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ class RewardsServiceImpl : public RewardsService,
bool reset_states_;
bool ledger_for_testing_ = false;
bool resetting_rewards_ = false;
bool should_persist_logs_ = false;
int persist_log_level_ = 0;

GetTestResponseCallback test_response_callback_;

Expand Down
3 changes: 3 additions & 0 deletions components/brave_rewards/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,8 @@ namespace features {
const base::Feature kBitflyerFeature{"BraveRewardsBitflyer",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kVerboseLoggingFeature{"BraveRewardsVerboseLogging",
base::FEATURE_DISABLED_BY_DEFAULT};

} // namespace features
} // namespace brave_rewards
1 change: 1 addition & 0 deletions components/brave_rewards/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace brave_rewards {
namespace features {

extern const base::Feature kBitflyerFeature;
extern const base::Feature kVerboseLoggingFeature;

} // namespace features
} // namespace brave_rewards
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ void AdRewards::OnGetPayments(const UrlResponse& url_response) {
}

if (!payments_->SetFromJson(url_response.body)) {
BLOG(0, "Failed to parse payment balance: " << url_response.body);
BLOG(0, "Failed to parse payment balance");
BLOG(6, "Payment balance response body: " << url_response.body);
OnFailedToReconcileAdRewards();
return;
}
Expand Down Expand Up @@ -324,7 +325,8 @@ void AdRewards::OnGetAdGrants(const UrlResponse& url_response) {
}

if (!ad_grants_->SetFromJson(url_response.body)) {
BLOG(0, "Failed to parse ad grants: " << url_response.body);
BLOG(0, "Failed to parse ad grants");
BLOG(6, "Ad grants response body: " << url_response.body);
OnFailedToReconcileAdRewards();
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type::Result GetParameters::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ type::Result GetBalance::CheckStatusCode(const int status_code) {
if (status_code == net::HTTP_UNAUTHORIZED ||
status_code == net::HTTP_NOT_FOUND ||
status_code == net::HTTP_FORBIDDEN) {
BLOG(0, "Invalid authorization HTTP status: " << status_code);
return type::Result::EXPIRED_TOKEN;
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ std::string PostOauth::GeneratePayload(const std::string& external_account_id,

type::Result PostOauth::CheckStatusCode(const int status_code) {
if (status_code == net::HTTP_UNAUTHORIZED) {
BLOG(0, "Unauthorized access");
return type::Result::EXPIRED_TOKEN;
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down Expand Up @@ -125,6 +127,7 @@ void PostOauth::Request(const std::string& external_account_id,
request->headers = RequestAuthorization();
request->content_type = "application/json";
request->method = type::UrlMethod::POST;
request->skip_log = true;
ledger_->LoadURL(std::move(request), url_callback);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ std::string PostTransaction::GeneratePayload(

type::Result PostTransaction::CheckStatusCode(const int status_code) {
if (status_code == net::HTTP_UNAUTHORIZED) {
BLOG(0, "Unauthorized access");
return type::Result::EXPIRED_TOKEN;
}

Expand All @@ -64,6 +65,7 @@ type::Result PostTransaction::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type::Result GetCredentials::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::RETRY;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type::Result PostCredentials::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type::Result PostOrder::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_CREATED) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ type::Result PostTransactionAnon::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_CREATED) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type::Result PostTransactionUphold::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_CREATED) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ type::Result PostVotes::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ type::Result GetPublisher::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type::Result GetAvailable::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type::Result GetCaptcha::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type::Result GetDrain::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type::Result GetRecoverWallet::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type::Result GetSignedCreds::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type::Result GetWalletBalance::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type::Result PostBatLoss::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type::Result PostCaptcha::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type::Result PostClaimBitflyer::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type::Result PostClaimBrave::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ type::Result PostClaimUphold::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type::Result PostClobberedClaims::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ type::Result PostCreds::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type::Result PostDevicecheck::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type::Result PostSafetynet::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type::Result PostSuggestions::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type::Result PostSuggestionsClaim::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type::Result PostWalletBrave::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_CREATED) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type::Result PutCaptcha::CheckStatusCode(const int status_code) {
}

if (status_code != net::HTTP_OK) {
BLOG(0, "Unexpected HTTP status: " << status_code);
return type::Result::LEDGER_ERROR;
}

Expand Down
Loading