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

Retry peers if connection refused #8192

Merged
merged 1 commit into from
Mar 10, 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
8 changes: 6 additions & 2 deletions components/ipfs/ipfs_navigation_throttle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,15 @@ void IpfsNavigationThrottle::OnIpfsLaunched(bool result) {
if (!resume_pending_)
return;

resume_pending_ = false;
if (!result) {
resume_pending_ = false;
ShowInterstitial();
} else {
Resume();
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&IpfsNavigationThrottle::GetConnectedPeers,
weak_ptr_factory_.GetWeakPtr()),
CalculatePeersRetryTime());
}
}

Expand Down
4 changes: 4 additions & 0 deletions components/ipfs/ipfs_navigation_throttle.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,14 @@ class IpfsNavigationThrottle : public content::NavigationThrottle {
DeferUntilIpfsProcessLaunched);
FRIEND_TEST_ALL_PREFIXES(IpfsNavigationThrottleUnitTest,
DeferUntilPeersFetched);
FRIEND_TEST_ALL_PREFIXES(IpfsNavigationThrottleUnitTest, SequentialRequests);
FRIEND_TEST_ALL_PREFIXES(IpfsNavigationThrottleUnitTest,
DeferMultipleUntilIpfsProcessLaunched);

void ShowInterstitial();
content::NavigationThrottle::ThrottleCheckResult
ShowIPFSOnboardingInterstitial();

void LoadPublicGatewayURL();
void GetConnectedPeers();
void OnGetConnectedPeers(bool success, const std::vector<std::string>& peers);
Expand Down
29 changes: 25 additions & 4 deletions components/ipfs/ipfs_navigation_throttle_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,15 @@ TEST_F(IpfsNavigationThrottleUnitTest, DeferMultipleUntilIpfsProcessLaunched) {

service->SetAllowIpfsLaunchForTest(true);
service->RunLaunchDaemonCallbackForTest(true);
EXPECT_FALSE(was_navigation_resumed1);
EXPECT_FALSE(was_navigation_resumed2);
EXPECT_FALSE(was_navigation_resumed3);
auto peers = std::vector<std::string>{
"/ip4/101.101.101.101/tcp/4001/p2p/"
"QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"};
throttle1->OnGetConnectedPeers(true, peers);
throttle2->OnGetConnectedPeers(true, peers);
throttle3->OnGetConnectedPeers(true, peers);
EXPECT_TRUE(was_navigation_resumed1);
EXPECT_TRUE(was_navigation_resumed2);
EXPECT_TRUE(was_navigation_resumed3);
Expand All @@ -187,11 +196,23 @@ TEST_F(IpfsNavigationThrottleUnitTest, SequentialRequests) {

service->SetAllowIpfsLaunchForTest(true);
service->RunLaunchDaemonCallbackForTest(true);
throttle1->OnIpfsLaunched(true);
EXPECT_FALSE(was_navigation_resumed1);
EXPECT_FALSE(was_navigation_resumed2);

auto peers = std::vector<std::string>();
throttle1->OnGetConnectedPeers(true, peers);
EXPECT_FALSE(was_navigation_resumed1);
EXPECT_FALSE(was_navigation_resumed2);

peers = std::vector<std::string>{
"/ip4/101.101.101.101/tcp/4001/p2p/"
"QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ"};
throttle1->OnGetConnectedPeers(true, peers);
EXPECT_TRUE(was_navigation_resumed1);
EXPECT_FALSE(was_navigation_resumed2);
throttle2->OnGetConnectedPeers(true, peers);
EXPECT_TRUE(was_navigation_resumed2);
ASSERT_FALSE(service->WasConnectedPeersCalledForTest());

auto throttle3 = CreateDeferredNavigation(service, base::RepeatingClosure());

ASSERT_TRUE(service->WasConnectedPeersCalledForTest());
Expand Down Expand Up @@ -264,7 +285,7 @@ TEST_F(IpfsNavigationThrottleUnitTest, DeferUntilIpfsProcessLaunched) {
<< GetIPFSURL();
service->SetAllowIpfsLaunchForTest(true);
service->RunLaunchDaemonCallbackForTest(true);
EXPECT_TRUE(was_navigation_resumed);
EXPECT_FALSE(was_navigation_resumed);

was_navigation_resumed = false;
EXPECT_EQ(NavigationThrottle::DEFER, throttle->WillStartRequest().action())
Expand All @@ -280,7 +301,7 @@ TEST_F(IpfsNavigationThrottleUnitTest, DeferUntilIpfsProcessLaunched) {
<< GetIPNSURL();
service->SetAllowIpfsLaunchForTest(true);
service->RunLaunchDaemonCallbackForTest(true);
EXPECT_TRUE(was_navigation_resumed);
EXPECT_FALSE(was_navigation_resumed);

was_navigation_resumed = false;
EXPECT_EQ(NavigationThrottle::DEFER, throttle->WillStartRequest().action())
Expand Down
39 changes: 37 additions & 2 deletions components/ipfs/ipfs_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/command_line.h"
#include "base/files/file_util.h"
#include "base/json/json_reader.h"
#include "base/rand_util.h"
#include "base/strings/strcat.h"
#include "base/strings/string_util.h"
#include "base/task/post_task.h"
Expand Down Expand Up @@ -38,6 +39,12 @@

namespace {

// Used to retry request if we got zero peers from ipfs service
// Actual value will be generated randomly in range
// (kMinimalPeersRetryIntervalMs, kPeersRetryRate*kMinimalPeersRetryIntervalMs)
const int kMinimalPeersRetryIntervalMs = 350;
const int kPeersRetryRate = 3;

net::NetworkTrafficAnnotationTag GetNetworkTrafficAnnotationTag() {
return net::DefineNetworkTrafficAnnotation("ipfs_service", R"(
semantics {
Expand Down Expand Up @@ -265,7 +272,8 @@ std::unique_ptr<network::SimpleURLLoader> IpfsService::CreateURLLoader(
return url_loader;
}

void IpfsService::GetConnectedPeers(GetConnectedPeersCallback callback) {
void IpfsService::GetConnectedPeers(GetConnectedPeersCallback callback,
int retries) {
if (!IsDaemonLaunched()) {
if (callback)
std::move(callback).Run(false, std::vector<std::string>{});
Expand All @@ -285,19 +293,38 @@ void IpfsService::GetConnectedPeers(GetConnectedPeersCallback callback) {
iter->get()->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
url_loader_factory_.get(),
base::BindOnce(&IpfsService::OnGetConnectedPeers, base::Unretained(this),
std::move(iter), std::move(callback)));
std::move(iter), std::move(callback), retries));
}

base::TimeDelta IpfsService::CalculatePeersRetryTime() {
if (zero_peer_time_for_test_)
return base::TimeDelta();
return base::TimeDelta::FromMilliseconds(
base::RandInt(kMinimalPeersRetryIntervalMs,
kPeersRetryRate * kMinimalPeersRetryIntervalMs));
}

void IpfsService::OnGetConnectedPeers(
SimpleURLLoaderList::iterator iter,
GetConnectedPeersCallback callback,
int retry_number,
std::unique_ptr<std::string> response_body) {
auto* url_loader = iter->get();
int error_code = url_loader->NetError();
int response_code = -1;
if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers)
response_code = url_loader->ResponseInfo()->headers->response_code();
url_loaders_.erase(iter);
last_peers_retry_value_for_test_ = retry_number;
if (error_code == net::ERR_CONNECTION_REFUSED && retry_number) {
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&IpfsService::GetConnectedPeers,
weak_factory_.GetWeakPtr(), std::move(callback),
retry_number - 1),
CalculatePeersRetryTime());
return;
}

std::vector<std::string> peers;
bool success = (error_code == net::OK && response_code == net::HTTP_OK);
Expand Down Expand Up @@ -433,6 +460,14 @@ void IpfsService::RegisterIpfsClientUpdater() {
}
}

int IpfsService::GetLastPeersRetryForTest() const {
return last_peers_retry_value_for_test_;
}

void IpfsService::SetZeroPeersDeltaForTest(bool value) {
zero_peer_time_for_test_ = value;
}

void IpfsService::SetAllowIpfsLaunchForTest(bool launched) {
allow_ipfs_launch_for_test_ = launched;
}
Expand Down
16 changes: 14 additions & 2 deletions components/ipfs/ipfs_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ class IpfsService : public KeyedService,
using ShutdownDaemonCallback = base::OnceCallback<void(bool)>;
using GetConfigCallback = base::OnceCallback<void(bool, const std::string&)>;

// Retry after some time If local node responded with error.
// The connected peers are often called immediately after startup
// and node initialization may take some time.
static constexpr int kPeersDefaultRetries = 5;

void AddObserver(IpfsServiceObserver* observer);
void RemoveObserver(IpfsServiceObserver* observer);

Expand All @@ -88,7 +93,9 @@ class IpfsService : public KeyedService,
void Shutdown() override;

void RestartDaemon();
void GetConnectedPeers(GetConnectedPeersCallback callback);

void GetConnectedPeers(GetConnectedPeersCallback callback,
int retries = kPeersDefaultRetries);
void GetAddressesConfig(GetAddressesConfigCallback callback);
void LaunchDaemon(LaunchDaemonCallback callback);
void ShutdownDaemon(ShutdownDaemonCallback callback);
Expand All @@ -103,6 +110,8 @@ class IpfsService : public KeyedService,
bool WasConnectedPeersCalledForTest() const;
void SetGetConnectedPeersCalledForTest(bool value);
void RunLaunchDaemonCallbackForTest(bool result);
int GetLastPeersRetryForTest() const;
void SetZeroPeersDeltaForTest(bool value);

protected:
void OnConfigLoaded(GetConfigCallback, const std::pair<bool, std::string>&);
Expand All @@ -122,11 +131,12 @@ class IpfsService : public KeyedService,
void NotifyDaemonLaunchCallbacks(bool result);
// Launches the ipfs service in an utility process.
void LaunchIfNotRunning(const base::FilePath& executable_path);

base::TimeDelta CalculatePeersRetryTime();
std::unique_ptr<network::SimpleURLLoader> CreateURLLoader(const GURL& gurl);

void OnGetConnectedPeers(SimpleURLLoaderList::iterator iter,
GetConnectedPeersCallback,
int retries,
std::unique_ptr<std::string> response_body);
void OnGetAddressesConfig(SimpleURLLoaderList::iterator iter,
GetAddressesConfigCallback callback,
Expand Down Expand Up @@ -159,6 +169,8 @@ class IpfsService : public KeyedService,
bool allow_ipfs_launch_for_test_ = false;
bool skip_get_connected_peers_callback_for_test_ = false;
bool connected_peers_function_called_ = false;
int last_peers_retry_value_for_test_ = -1;
bool zero_peer_time_for_test_ = false;

GURL server_endpoint_;

Expand Down
28 changes: 28 additions & 0 deletions components/ipfs/ipfs_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/http_request.h"
#include "net/test/embedded_test_server/http_response.h"
#include "net/test/url_request/url_request_failed_job.h"

namespace ipfs {

Expand Down Expand Up @@ -55,6 +56,10 @@ class IpfsServiceBrowserTest : public InProcessBrowserTest {
ipfs_service_->SetServerEndpointForTest(test_server_->base_url());
}

void ShutDownTestServer() {
ASSERT_TRUE(test_server_->ShutdownAndWaitUntilComplete());
}

GURL GetURL(const std::string& host, const std::string& path) {
return test_server_->GetURL(host, path);
}
Expand Down Expand Up @@ -305,6 +310,16 @@ class IpfsServiceBrowserTest : public InProcessBrowserTest {
EXPECT_EQ(peers, std::vector<std::string>{});
}

void OnGetConnectedPeersAfterRetry(bool success,
const std::vector<std::string>& peers) {
if (wait_for_request_) {
wait_for_request_->Quit();
}
EXPECT_FALSE(success);
EXPECT_EQ(peers, std::vector<std::string>{});
EXPECT_EQ(ipfs_service()->GetLastPeersRetryForTest(), 0);
}

void OnGetAddressesConfigSuccess(bool success,
const AddressesConfig& config) {
if (wait_for_request_) {
Expand Down Expand Up @@ -418,6 +433,19 @@ IN_PROC_BROWSER_TEST_F(IpfsServiceBrowserTest, GetConnectedPeersServerError) {
WaitForRequest();
}

IN_PROC_BROWSER_TEST_F(IpfsServiceBrowserTest, GetConnectedPeersRetry) {
ResetTestServer(
base::BindRepeating(&IpfsServiceBrowserTest::HandleRequestServerError,
base::Unretained(this)));
ipfs_service()->SetZeroPeersDeltaForTest(true);
ShutDownTestServer();
ipfs_service()->GetConnectedPeers(
base::BindOnce(&IpfsServiceBrowserTest::OnGetConnectedPeersAfterRetry,
base::Unretained(this)),
4);
WaitForRequest();
}

IN_PROC_BROWSER_TEST_F(IpfsServiceBrowserTest, GetAddressesConfig) {
ResetTestServer(
base::BindRepeating(&IpfsServiceBrowserTest::HandleGetAddressesConfig,
Expand Down