Skip to content

Commit

Permalink
Log Rewards errors by default and add a feature flag for verbose logging
Browse files Browse the repository at this point in the history
  • Loading branch information
zenparsing committed Apr 28, 2021
1 parent 040d67a commit 6136bed
Show file tree
Hide file tree
Showing 50 changed files with 87 additions and 14 deletions.
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_DISABLED_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
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type::Result PutDevicecheck::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

0 comments on commit 6136bed

Please sign in to comment.