Commit 85b2df2a authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Sync] Show the error on sync-internals when connection not available

This CL plumbs the network error code through to show it in the
chrome://sync-internals pages in case of NETWORK_CONNECTION_UNAVAILABLE.

This should allow for easier debugging for some user reported bugs.

Bug: 897049
Change-Id: Id0fc5455eb4d64b1a022898d8a898e7efa4a255c
Reviewed-on: https://chromium-review.googlesource.com/c/1346460
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarMisha Efimov <mef@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611113}
parent 1e66f47c
...@@ -10,4 +10,5 @@ include_rules = [ ...@@ -10,4 +10,5 @@ include_rules = [
"+crypto", "+crypto",
"+google/cacheinvalidation", "+google/cacheinvalidation",
"+third_party/zlib", # For UniquePosition compression "+third_party/zlib", # For UniquePosition compression
"+net/base",
] ]
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "components/sync/base/syncer_error.h" #include "components/sync/base/syncer_error.h"
#include "base/logging.h" #include "base/logging.h"
#include "net/base/net_errors.h"
namespace syncer { namespace syncer {
...@@ -14,7 +15,7 @@ namespace { ...@@ -14,7 +15,7 @@ namespace {
case SyncerError::x: \ case SyncerError::x: \
return #x; \ return #x; \
break; break;
const char* GetSyncerErrorString(SyncerError::Value value) { std::string GetSyncerErrorString(SyncerError::Value value) {
switch (value) { switch (value) {
ENUM_CASE(UNSET); ENUM_CASE(UNSET);
ENUM_CASE(CANNOT_DO_WORK); ENUM_CASE(CANNOT_DO_WORK);
...@@ -46,10 +47,12 @@ const char* GetSyncerErrorString(SyncerError::Value value) { ...@@ -46,10 +47,12 @@ const char* GetSyncerErrorString(SyncerError::Value value) {
} // namespace } // namespace
SyncerError::~SyncerError() = default; std::string SyncerError::ToString() const {
if (value_ != NETWORK_CONNECTION_UNAVAILABLE) {
const char* SyncerError::ToString() const {
return GetSyncerErrorString(value_); return GetSyncerErrorString(value_);
}
return GetSyncerErrorString(value_) + " (" +
net::ErrorToShortString(net_error_code_) + ")";
} }
bool SyncerError::IsActualError() const { bool SyncerError::IsActualError() const {
......
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef COMPONENTS_SYNC_BASE_SYNCER_ERROR_H_ #ifndef COMPONENTS_SYNC_BASE_SYNCER_ERROR_H_
#define COMPONENTS_SYNC_BASE_SYNCER_ERROR_H_ #define COMPONENTS_SYNC_BASE_SYNCER_ERROR_H_
#include <string>
namespace syncer { namespace syncer {
// This class describes all the possible results of a sync cycle. It should be // This class describes all the possible results of a sync cycle. It should be
...@@ -43,19 +45,30 @@ class SyncerError { ...@@ -43,19 +45,30 @@ class SyncerError {
SYNCER_OK SYNCER_OK
}; };
constexpr SyncerError() : value_(UNSET) {} constexpr SyncerError() : value_(UNSET), net_error_code_(0) {}
explicit constexpr SyncerError(Value value) : value_(value) {}
~SyncerError(); explicit constexpr SyncerError(Value value)
: value_(value), net_error_code_(0) {}
static constexpr SyncerError NetworkConnectionUnavailable(
int net_error_code) {
return SyncerError(NETWORK_CONNECTION_UNAVAILABLE, net_error_code);
}
Value value() const { return value_; } Value value() const { return value_; }
const char* ToString() const;
std::string ToString() const;
// Helper to check that |error| is set to something (not UNSET) and is not // Helper to check that |error| is set to something (not UNSET) and is not
// SYNCER_OK or SERVER_MORE_TO_DOWNLOAD. // SYNCER_OK or SERVER_MORE_TO_DOWNLOAD.
bool IsActualError() const; bool IsActualError() const;
private: private:
constexpr SyncerError(Value value, int net_error_code)
: value_(value), net_error_code_(net_error_code) {}
Value value_; Value value_;
int net_error_code_;
}; };
} // namespace syncer } // namespace syncer
......
...@@ -26,7 +26,7 @@ std::string ClearServerDataResponseEvent::GetType() const { ...@@ -26,7 +26,7 @@ std::string ClearServerDataResponseEvent::GetType() const {
} }
std::string ClearServerDataResponseEvent::GetDetails() const { std::string ClearServerDataResponseEvent::GetDetails() const {
return base::StringPrintf("Result: %s", result_.ToString()); return "Result: " + result_.ToString();
} }
std::unique_ptr<base::DictionaryValue> std::unique_ptr<base::DictionaryValue>
......
...@@ -26,7 +26,7 @@ std::string CommitResponseEvent::GetType() const { ...@@ -26,7 +26,7 @@ std::string CommitResponseEvent::GetType() const {
} }
std::string CommitResponseEvent::GetDetails() const { std::string CommitResponseEvent::GetDetails() const {
return base::StringPrintf("Result: %s", result_.ToString()); return "Result: " + result_.ToString();
} }
std::unique_ptr<base::DictionaryValue> CommitResponseEvent::GetProtoMessage( std::unique_ptr<base::DictionaryValue> CommitResponseEvent::GetProtoMessage(
......
...@@ -34,7 +34,7 @@ std::string GetUpdatesResponseEvent::GetDetails() const { ...@@ -34,7 +34,7 @@ std::string GetUpdatesResponseEvent::GetDetails() const {
return base::StringPrintf("Received %d update(s). Some updates remain.", return base::StringPrintf("Received %d update(s). Some updates remain.",
response_.get_updates().entries_size()); response_.get_updates().entries_size());
default: default:
return base::StringPrintf("Received error: %s", error_.ToString()); return "Received error: " + error_.ToString();
} }
} }
......
...@@ -201,6 +201,7 @@ bool ServerConnectionManager::PostBufferWithCachedAuth( ...@@ -201,6 +201,7 @@ bool ServerConnectionManager::PostBufferWithCachedAuth(
MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_)); MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_));
bool result = PostBufferToPath(params, path, auth_token()); bool result = PostBufferToPath(params, path, auth_token());
SetServerStatus(params->response.server_status); SetServerStatus(params->response.server_status);
net_error_code_ = params->response.net_error_code;
return result; return result;
} }
......
...@@ -54,6 +54,9 @@ struct HttpResponse { ...@@ -54,6 +54,9 @@ struct HttpResponse {
SERVER_CONNECTION_OK, SERVER_CONNECTION_OK,
}; };
// The network error code.
int net_error_code;
// The HTTP Status code. // The HTTP Status code.
int http_status_code; int http_status_code;
...@@ -163,6 +166,11 @@ class ServerConnectionManager { ...@@ -163,6 +166,11 @@ class ServerConnectionManager {
return server_status_; return server_status_;
} }
inline int net_error_code() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return net_error_code_;
}
const std::string client_id() const { return client_id_; } const std::string client_id() const { return client_id_; }
// Factory method to create an Connection object we can use for // Factory method to create an Connection object we can use for
...@@ -231,6 +239,11 @@ class ServerConnectionManager { ...@@ -231,6 +239,11 @@ class ServerConnectionManager {
HttpResponse::ServerConnectionCode server_status_; HttpResponse::ServerConnectionCode server_status_;
// Contains the network error code if there is an error when making the
// connection with the server in which case |server_status_| is set to
// HttpResponse::CONNECTION_UNAVAILABLE.
int net_error_code_;
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
CancelationSignal* const cancelation_signal_; CancelationSignal* const cancelation_signal_;
......
...@@ -70,6 +70,7 @@ bool SyncBridgedConnection::Init(const char* path, ...@@ -70,6 +70,7 @@ bool SyncBridgedConnection::Init(const char* path,
DCHECK_NE(net_error_code, net::OK); DCHECK_NE(net_error_code, net::OK);
DVLOG(1) << "Http POST failed, error returns: " << net_error_code; DVLOG(1) << "Http POST failed, error returns: " << net_error_code;
response->server_status = HttpResponse::CONNECTION_UNAVAILABLE; response->server_status = HttpResponse::CONNECTION_UNAVAILABLE;
response->net_error_code = net_error_code;
return false; return false;
} }
......
...@@ -83,10 +83,11 @@ void LogResponseProfilingData(const ClientToServerResponse& response) { ...@@ -83,10 +83,11 @@ void LogResponseProfilingData(const ClientToServerResponse& response) {
} }
SyncerError ServerConnectionErrorAsSyncerError( SyncerError ServerConnectionErrorAsSyncerError(
const HttpResponse::ServerConnectionCode server_status) { const HttpResponse::ServerConnectionCode server_status,
int net_error_code) {
switch (server_status) { switch (server_status) {
case HttpResponse::CONNECTION_UNAVAILABLE: case HttpResponse::CONNECTION_UNAVAILABLE:
return SyncerError(SyncerError::NETWORK_CONNECTION_UNAVAILABLE); return SyncerError::NetworkConnectionUnavailable(net_error_code);
case HttpResponse::IO_ERROR: case HttpResponse::IO_ERROR:
return SyncerError(SyncerError::NETWORK_IO_ERROR); return SyncerError(SyncerError::NETWORK_IO_ERROR);
case HttpResponse::SYNC_SERVER_ERROR: case HttpResponse::SYNC_SERVER_ERROR:
...@@ -390,7 +391,9 @@ SyncerError SyncerProtoUtil::PostClientToServerMessage( ...@@ -390,7 +391,9 @@ SyncerError SyncerProtoUtil::PostClientToServerMessage(
DCHECK_NE(server_status, HttpResponse::NONE); DCHECK_NE(server_status, HttpResponse::NONE);
DCHECK_NE(server_status, HttpResponse::SERVER_CONNECTION_OK); DCHECK_NE(server_status, HttpResponse::SERVER_CONNECTION_OK);
return ServerConnectionErrorAsSyncerError(server_status); return ServerConnectionErrorAsSyncerError(
server_status,
cycle->context()->connection_manager()->net_error_code());
} }
LogClientToServerResponse(*response); LogClientToServerResponse(*response);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment