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

Use std::panic::set_hook to exit serve thread if serving fails #1555

Merged
merged 1 commit into from
May 29, 2021

Conversation

joshrotenberg
Copy link
Contributor

mdbook serve starts the warp server in a thread and then continues on to watch for changes. However, if the thread panics, for example, because the address is already in use, mdbook doesn't exit.

This change will explicitly call std::process::exit(1) from the thread if warp::serve panics.

Fixes #1545.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm a bit surprised by this behavior in warp, it seems like it would be better to return a Result somewhere somehow.

@ehuss
Copy link
Contributor

ehuss commented May 29, 2021

Oh, I see this is in Draft status, are you intending to make more changes?

@joshrotenberg joshrotenberg marked this pull request as ready for review May 29, 2021 21:49
@joshrotenberg
Copy link
Contributor Author

Moved to ready. I was debating whether this was a good excuse to start some integration tests for the binary but that's probably better on its own.

@joshrotenberg
Copy link
Contributor Author

I'm a bit surprised by this behavior in warp, it seems like it would be better to return a Result somewhere somehow.

Yeah, same. I dug around the docs and in some example code to see how others might handle it. My guess is that the usual use case is that you run the server in the main thread, so a panic would result in an exit anyway.

@ehuss
Copy link
Contributor

ehuss commented May 29, 2021

Ah, sounds good, thanks!

@ehuss ehuss merged commit 8201b41 into rust-lang:master May 29, 2021
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.

mdbook should exit if serve can't serve
2 participants