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

Remove expect and unwrap from codebase #255

Open
chvck opened this issue Jul 27, 2023 · 2 comments
Open

Remove expect and unwrap from codebase #255

chvck opened this issue Jul 27, 2023 · 2 comments
Assignees

Comments

@chvck
Copy link
Contributor

chvck commented Jul 27, 2023

If we're going to fail something we should do it gracefully and at least attempt to give the user some information about why we've failed. We shouldn't just panic.

@chvck
Copy link
Contributor Author

chvck commented Aug 7, 2023

expect and unwrap in main.rs are probably ok, these happen at startup. unwrap when taking a lock also ok, this can only happen if the lock has been poisoned, i.e. a panic occurred in another thread which has holding the lock - this a) shouldn't be happening and b) is probably unrecoverable.

@Westwooo
Copy link
Contributor

Westwooo commented Apr 11, 2024

@chvck What about the numerous engine_state.ctrlc.as_ref().unwrap().clone(); ? Also there are a few places where we do serde_json::to_string(json).unwrap() and we know we are passing in valid json since it's a server response. And I'd say that Runtime::new().unwrap(); seems okay.

@Westwooo Westwooo self-assigned this Apr 15, 2024
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

No branches or pull requests

2 participants