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

Makes Tor task panics more user friendly #99

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

sr-gi
Copy link
Member

@sr-gi sr-gi commented Aug 24, 2022

Given we cannot address #57 atm, lets at least make it so if the Tor thread fails it does so in a more user friendly way.

@sr-gi sr-gi added the Seeking Code Review review me pls label Aug 24, 2022
@sr-gi sr-gi added this to the v.0.1.2 milestone Aug 24, 2022
@mariocynicys
Copy link
Collaborator

Does std::process::exit stop other spawned tokio threads?

@sr-gi
Copy link
Member Author

sr-gi commented Aug 24, 2022

Does std::process::exit stop other spawned tokio threads?

I've manually tested and looks like it does. std::process::exit terminates the current process, so I would assume so.

Copy link
Contributor

@booklearner booklearner left a comment

Choose a reason for hiding this comment

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

LGTM!

@sr-gi
Copy link
Member Author

sr-gi commented Aug 26, 2022

Any tACK? @booklearner @meryacine

Cannot make unit tests for this so manual testing would be appreviated

Copy link
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

tACK
image
test scenario: tor isn't running

@sr-gi sr-gi removed the Seeking Code Review review me pls label Aug 26, 2022
@sr-gi sr-gi merged commit 56bfe4c into talaia-labs:master Aug 26, 2022
@booklearner
Copy link
Contributor

ah, TIL tACK = test ACK - sorry for the premature approval 😅

@sr-gi
Copy link
Member Author

sr-gi commented Aug 26, 2022

@booklearner np np, we're all learning here 😛

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.

None yet

3 participants