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

chore: Log an error when failing to resolve external ip address #11085

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: Log an error when failing to resolve external ip address #11085

wants to merge 2 commits into from
+14 −7

Conversation

garwahl
Copy link
Contributor

@garwahl garwahl commented Sep 21, 2024

Closes #11026

Similar to #10922, we should log an error when IP resolution fails rather than failing silently.

mattsse
mattsse requested changes Sep 21, 2024
View reviewed changes
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I don't find these useful as errors because this leaks to cli output, I'd be okay with debug

crates/net/nat/src/lib.rs Outdated Show resolved Hide resolved
crates/net/nat/src/lib.rs Outdated Show resolved Hide resolved
emhane
emhane requested changes Sep 21, 2024
View reviewed changes
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

pls fix all the log messages as suggested, and change to debug as suggested by matze

crates/net/nat/src/lib.rs Outdated Show resolved Hide resolved
@garwahl
Copy link
Contributor Author

garwahl commented Sep 22, 2024

Removed redundant logging and changed all to debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@emhane emhane Awaiting requested review from emhane emhane is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

crates/net: resolve_external_ip() fails silently, should error or panic
3 participants
@garwahl @mattsse @emhane