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

Ignore connections that fail parsing instead of panicking on BSD #288

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Conversation

cyqsimon
Copy link
Collaborator

@cyqsimon cyqsimon commented Sep 20, 2023

Will fix #217.

What I'm doing not is not ideal - I think there should be some kind of warning when parsing fails, but I'm not sure what's the most appropriate way to raise this warning. But at least it's better than panicking.

MacOS & BSD users can you please test this patch? Pay particular attention to whether there are missing connections. Thanks.

Further improvements very much welcomed.

@cyqsimon
Copy link
Collaborator Author

Rebased

@sigmaSd
Copy link
Collaborator

sigmaSd commented Sep 21, 2023

I think it would be great if we have a log file where can write such warnings.

@cyqsimon
Copy link
Collaborator Author

I think it would be great if we have a log file where can write such warnings.

Sounds reasonable. I'll find some time this week to add the logging infrastructure.

@cyqsimon
Copy link
Collaborator Author

Rebased.

@cyqsimon
Copy link
Collaborator Author

Rebased.

@cyqsimon
Copy link
Collaborator Author

For the moment this is the only bit of code that logs anything. I will look to add more logging incrementally.

@cyqsimon cyqsimon merged commit 1c996c3 into main Oct 11, 2023
10 checks passed
@cyqsimon cyqsimon deleted the lsof branch October 11, 2023 11:46
@sigmaSd
Copy link
Collaborator

sigmaSd commented Oct 11, 2023

The logging is currently optional , I think it should always happen

pub log_to: Option<PathBuf>,

@sigmaSd
Copy link
Collaborator

sigmaSd commented Oct 11, 2023

Actually nvm I don't feel that strong about it

@cyqsimon
Copy link
Collaborator Author

I made it optional because I feel like it's only really useful as a mechanism to help us debug users' issues. Not to mention that we would have to deal with log rotation (on different platforms no less, yuck). So this is good enough for now. We can always change it in the future if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to start on MacOS Big Sur
2 participants