Skip to content

Commit

Permalink
ROVER-311 Stop rover dev if the Router binary crashes (#2382)
Browse files Browse the repository at this point in the history
At the moment (in Rover 0.26.3) if the Router fails to startup because
of bad configuration `rover dev` doesn't respond to that event. It will
sit there and still attempt to update the hot reloadable config, despite
the fact that this won't have an impact as the binary is not monitoring
the config once it has crashed.

This moves us to a much better situation where the error is printed to
the user, and then `rover dev` shuts down, so they can retry it once
they've fixed the config in question.

This also includes a change to re-instate some logging that was
previously removed.
  • Loading branch information
jonathanrainer authored Feb 4, 2025
1 parent 5810499 commit 9fd86aa
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 24 deletions.
21 changes: 21 additions & 0 deletions src/command/dev/do_dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use semver::Version;
use tower::ServiceExt;

use super::version_upgrade_message::VersionUpgradeMessage;
use crate::command::dev::router::binary::RunRouterBinaryError;
use crate::command::dev::router::config::{RouterAddress, RouterHost, RouterPort};
use crate::command::dev::router::hot_reload::HotReloadConfigOverrides;
use crate::command::dev::router::run::RunRouter;
Expand Down Expand Up @@ -278,6 +279,26 @@ impl Dev {
eprintln!("{}", router_log);
}
}
Err(RunRouterBinaryError::BinaryExited(res)) => {
match res {
Ok(status) => {
match status.code() {
None => {
eprintln!("Router process terminal by signal");
}
Some(code) => {
eprintln!("Router process exited with status code: {code}");
}
}

}
Err(err) => {
tracing::error!("Router process exited without status code. Error: {err}")
}
}
eprintln!("\nRouter binary exited, stopping `rover dev` processes...");
break;
}
Err(err) => {
tracing::error!("{:?}", err);
}
Expand Down
95 changes: 71 additions & 24 deletions src/command/dev/router/binary.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
use std::collections::HashMap;
use std::fmt;
use std::process::Stdio;
use std::fmt::Formatter;
use std::net::{AddrParseError, SocketAddr};
use std::process::{ExitStatus, Stdio};
use std::{fmt, io};

use buildstructor::Builder;
use camino::Utf8PathBuf;
use futures::TryFutureExt;
use houston::Credential;
use rover_std::Style;
use regex::Regex;
use rover_std::{infoln, Style};
use semver::Version;
use tap::TapFallible;
use tokio::io::{AsyncBufReadExt, BufReader};
Expand All @@ -16,6 +19,7 @@ use tower::{Service, ServiceExt};

use super::config::remote::RemoteRouterConfig;
use super::hot_reload::HotReloadError;
use crate::command::dev::router::config::{RouterAddress, RouterHost, RouterPort};
use crate::subtask::SubtaskHandleUnit;
use crate::utils::effect::exec::{ExecCommandConfig, ExecCommandOutput};
use crate::RoverError;
Expand All @@ -39,11 +43,34 @@ fn should_select_log_message(log_message: &str) -> bool {
.is_empty()
}

fn produce_special_message(raw_message: &str) {
let starting_message_regex = Regex::new(r"^.*\s+.*://(.*:[0-9]+).*\s+.*").unwrap();

let contents = match starting_message_regex.captures(raw_message) {
None => raw_message.to_string(),
Some(captures) => {
let socket_address: Option<Result<SocketAddr, AddrParseError>> =
captures.get(1).map(|m| m.as_str().parse());
match socket_address {
Some(Ok(socket_addr)) => {
let router_address = RouterAddress::new(
Some(RouterHost::CliOption(socket_addr.ip())),
Some(RouterPort::CliOption(socket_addr.port())),
)
.pretty_string();
format!("Your supergraph is running! head to {router_address} to query your supergraph")
}
_ => raw_message.to_string(),
}
}
};
infoln!("{}", contents)
}

impl fmt::Display for RouterLog {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let warn_prefix = Style::WarningPrefix.paint("WARN:");
let error_prefix = Style::ErrorPrefix.paint("ERROR:");
let info_prefix = Style::InfoPrefix.paint("INFO:");
let unknown_prefix = Style::ErrorPrefix.paint("UNKNOWN:");
match self {
Self::Stdout(stdout) => {
Expand All @@ -60,12 +87,10 @@ impl fmt::Display for RouterLog {
.unwrap_or(stdout);

match level {
"INFO" => {
if should_select_log_message(message) {
write!(f, "{} {}", info_prefix, &message)?
}
tracing::info!(%message)
"INFO" if should_select_log_message(message) => {
produce_special_message(message);
}
"INFO" => tracing::info!(%message),
"DEBUG" => tracing::debug!(%message),
"TRACE" => tracing::trace!(%message),
"WARN" => write!(f, "{} {}", warn_prefix, &message)?,
Expand Down Expand Up @@ -112,6 +137,8 @@ pub enum RunRouterBinaryError {
},
#[error("Failed to expand config: {}.", .err)]
Expansion { err: RoverError },
#[error("Router Binary exited")]
BinaryExited(io::Result<ExitStatus>),
}

impl From<HotReloadError> for RunRouterBinaryError {
Expand Down Expand Up @@ -261,24 +288,27 @@ where
}
match child.stderr.take() {
Some(stderr) => {
tokio::task::spawn(async move {
let mut lines = BufReader::new(stderr).lines();
while let Ok(Some(line)) =
lines.next_line().await.tap_err(|err| {
tracing::error!(
"Error reading from router stderr: {:?}",
err
)
})
{
let _ = sender
.send(Ok(RouterLog::Stderr(line)))
.tap_err(|err| {
tokio::task::spawn({
let sender = sender.clone();
async move {
let mut lines = BufReader::new(stderr).lines();
while let Ok(Some(line)) =
lines.next_line().await.tap_err(|err| {
tracing::error!(
"Error reading from router stderr: {:?}",
err
)
})
{
let _ = sender
.send(Ok(RouterLog::Stderr(line)))
.tap_err(|err| {
tracing::error!(
"Failed to send router stderr message. {:?}",
err
)
});
});
}
}
});
}
Expand All @@ -291,6 +321,23 @@ where
});
}
}
// Spawn a task that just sits listening to the Router binary, and if it
// exits, fire an error to say so, such that we can stop Rover Dev
// running if this happens.
tokio::spawn({
async move {
let res = child.wait().await;
let _ = sender
.send(Err(RunRouterBinaryError::BinaryExited(res)))
.tap_err(|err| {
tracing::error!(
"Failed to send router stderr message. {:?}",
err
)
});
}
})
.await
})
.await;
}
Expand Down

0 comments on commit 9fd86aa

Please sign in to comment.