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

feature: handle process errors and shutdowns more elegantly #29

Merged
merged 22 commits into from
Apr 9, 2024

Conversation

steezeburger
Copy link
Member

It was possible for the tui to completely bork the shell if there was a failure in one of the binaries at a specific point in app startup.

This PR adds context and better stdout/stderr handling. We're no longer panicking, but using context's cancel functionality. We're also using a buffer for the process output, so that no output gets lost

closes #27

@steezeburger steezeburger changed the title feature: handle binary errors and shutdowns more elegantly feature: handle process errors and shutdowns more elegantly Apr 2, 2024
Copy link
Contributor

@sambukowski sambukowski left a comment

Choose a reason for hiding this comment

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

Overall, looks good! We just need to fix the line count increment bug because of the new ticker.

@steezeburger steezeburger merged commit c846d92 into main Apr 9, 2024
1 check passed
@steezeburger steezeburger deleted the feature/process-runner-context branch April 9, 2024 17:12
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.

Use context to correctly manage running processes and shutdown all on any singular failure
2 participants