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

Catch more exceptions #1548

Merged
merged 5 commits into from Feb 13, 2020
Merged

Catch more exceptions #1548

merged 5 commits into from Feb 13, 2020

Conversation

@carver
Copy link
Contributor

carver commented Feb 13, 2020

What was wrong?

Clean up random uncaught exceptions.

How was it fixed?

Mostly just catching and logging them.

To-Do

  • Clean up commit history
  • Add release notes for #1545

Cute Animal Picture

put a cute animal picture link inside the parentheses

carver added 4 commits Feb 12, 2020
If a peer connection is lost while disconnecting with them for a
MalformedMessage, make sure to handle the PeerConnectionLost exception.
@carver carver requested a review from gsalgado Feb 13, 2020
if remote is None:
raise ValidationError("Cannot create handshake with None remote")
elif remote.address is None:
raise ValidationError("Cannot create handshake with remote address=None")

This comment has been minimized.

Copy link
@carver

carver Feb 13, 2020

Author Contributor

@gsalgado Is this a reasonable constraint?

I had an issue later on in the handshake where remote.address was None and raised an uncaught exception. I wanted to find out earlier when this (seemingly) problematic remote was created.

This comment has been minimized.

Copy link
@gsalgado

gsalgado Feb 13, 2020

Contributor

The ENR spec does not require an IP address to be included, and I've seen some nodes send us ENRs without that, so yeah, they end up with None as their .address. Maybe I should change the should_skip() function used when looking up connection candidates to skip nodes with no address? Still, I think it'd make sense to guard against that here

This comment has been minimized.

Copy link
@carver

carver Feb 13, 2020

Author Contributor

Yeah, seems like the ENR is unusable for connecting if it doesn't have an IP address. So 👍 for updating should_skip()

@carver carver merged commit 62df297 into ethereum:master Feb 13, 2020
44 checks passed
44 checks passed
ci/circleci: docker-trinity-beacon-image-build-test Your tests passed on CircleCI!
Details
ci/circleci: docker-trinity-image-build-test Your tests passed on CircleCI!
Details
ci/circleci: py36-docs Your tests passed on CircleCI!
Details
ci/circleci: py36-eth1-components Your tests passed on CircleCI!
Details
ci/circleci: py36-eth1-core Your tests passed on CircleCI!
Details
ci/circleci: py36-eth1-monitor-trio Your tests passed on CircleCI!
Details
ci/circleci: py36-eth2-core Your tests passed on CircleCI!
Details
ci/circleci: py36-eth2-fixtures Your tests passed on CircleCI!
Details
ci/circleci: py36-eth2-integration Your tests passed on CircleCI!
Details
ci/circleci: py36-eth2-utils Your tests passed on CircleCI!
Details
ci/circleci: py36-integration Your tests passed on CircleCI!
Details
ci/circleci: py36-lightchain_integration Your tests passed on CircleCI!
Details
ci/circleci: py36-lint Your tests passed on CircleCI!
Details
ci/circleci: py36-lint-eth2 Your tests passed on CircleCI!
Details
ci/circleci: py36-long_run_integration Your tests passed on CircleCI!
Details
ci/circleci: py36-p2p Your tests passed on CircleCI!
Details
ci/circleci: py36-p2p-trio Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-blockchain Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-byzantium Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-constantinople Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-frontier Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-homestead Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-petersburg Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-spurious_dragon Your tests passed on CircleCI!
Details
ci/circleci: py36-rpc-state-tangerine_whistle Your tests passed on CircleCI!
Details
ci/circleci: py36-wheel-cli Your tests passed on CircleCI!
Details
ci/circleci: py37-eth1-components Your tests passed on CircleCI!
Details
ci/circleci: py37-eth1-core Your tests passed on CircleCI!
Details
ci/circleci: py37-eth1-monitor-trio Your tests passed on CircleCI!
Details
ci/circleci: py37-eth2-components Your tests passed on CircleCI!
Details
ci/circleci: py37-eth2-core Your tests passed on CircleCI!
Details
ci/circleci: py37-eth2-fixtures Your tests passed on CircleCI!
Details
ci/circleci: py37-eth2-integration Your tests passed on CircleCI!
Details
ci/circleci: py37-eth2-utils Your tests passed on CircleCI!
Details
ci/circleci: py37-libp2p Your tests passed on CircleCI!
Details
ci/circleci: py37-lint Your tests passed on CircleCI!
Details
ci/circleci: py37-lint-eth2 Your tests passed on CircleCI!
Details
ci/circleci: py37-p2p Your tests passed on CircleCI!
Details
ci/circleci: py37-p2p-trio Your tests passed on CircleCI!
Details
ci/circleci: py37-rpc-state-quadratic Your tests passed on CircleCI!
Details
ci/circleci: py37-rpc-state-sstore Your tests passed on CircleCI!
Details
ci/circleci: py37-rpc-state-zero_knowledge Your tests passed on CircleCI!
Details
ci/circleci: py37-wheel-cli Your tests passed on CircleCI!
Details
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
@carver carver deleted the carver:catch-more-exceptions branch Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.