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.

Sign up for GitHub

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

Jump to bottom

Don't throw exception when connecting to all neighbors fails #933

Merged
merged 2 commits into from Jul 23, 2020
Merged

Don't throw exception when connecting to all neighbors fails #933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
be95952
Don't throw exception when connecting to all neighbors fails
earlbread Jul 23, 2020
2495685
Update CHANGES.md
earlbread Jul 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
View file
Open in desktop
Expand Up @@ -98,6 +98,8 @@ To be released.
[[#459], [#919]]
- `Swarm<T>` became to promote the most difficult chain as a canonical chain
instead of the longest chain. [[#459], [#919]]
- `Swarm<T>.BootstrapAsync()` method became not to throw `TimeoutException` when
it fails to connect to all neighbors. [[#933]]

### Bug fixes

Expand Down Expand Up @@ -151,6 +153,7 @@ To be released.
[#927]: https://github.com/planetarium/libplanet/pull/927
[#930]: https://github.com/planetarium/libplanet/pull/930
[#932]: https://github.com/planetarium/libplanet/pull/932
[#933]: https://github.com/planetarium/libplanet/pull/933
[sleep mode]: https://en.wikipedia.org/wiki/Sleep_mode


Expand Down
29 changes: 0 additions & 29 deletions Libplanet/Net/Protocols/KademliaProtocol.cs
View file
Open in desktop
Expand Up @@ -127,15 +127,6 @@ private readonly DifferentAppProtocolVersionEncountered
}
catch (Exception e)
{
if (findPeerTasks.All(task =>
task.IsFaulted &&
!(task.Exception is null) &&
task.Exception.InnerExceptions.All(ex => ex is TimeoutException)))
{
throw new TimeoutException(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we log inner exceptions in other places? If not it might better to log them here. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the inner exceptions are logged here. https://github.com/planetarium/libplanet/pull/933/files#diff-ae527d5afcdf41da4a57b594cab3cfefL655

$"Timeout exception occurred during {nameof(BootstrapAsync)}().");
}

var msg = $"An unexpected exception occurred during {nameof(BootstrapAsync)}()." +
" {Exception}";
_logger.Error(e, msg, e);
Expand Down Expand Up @@ -640,16 +631,6 @@ private void ReceivePing(Ping ping)
}
catch (Exception e)
{
if (awaitables.All(task =>
task.IsFaulted &&
!(task.Exception is null) &&
task.Exception.InnerExceptions.All(ex => ex is TimeoutException)))
{
throw new TimeoutException(
$"All neighbors found do not respond in {_requestTimeout}."
);
}

_logger.Error(
e,
"Some responses from neighbors found unexpectedly terminated: {Exception}",
Expand Down Expand Up @@ -695,16 +676,6 @@ private void ReceivePing(Ping ping)
}
catch (Exception e)
{
if (findPeerTasks.All(task =>
task.IsFaulted &&
!(task.Exception is null) &&
task.Exception.InnerExceptions.All(ex => ex is TimeoutException)))
{
throw new TimeoutException(
"All FindPeer tasks caused timeout during " +
$"{nameof(ProcessFoundAsync)}().");
}

_logger.Error(
e,
"Some FindPeer tasks were unexpectedly terminated: {Exception}",
Expand Down