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

feat(codegen): restore default log level, improve error messages #1211

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Expand Up @@ -98,7 +98,6 @@ tracing = { version = "0.1.37", default-features = false }
tracing-opentelemetry = "0.19.0"
tracing-subscriber = { version = "0.3.16", default-features = false, features = [
"registry",
"std",
jonaro00 marked this conversation as resolved.
Show resolved Hide resolved
] }
ttl_cache = "0.5.1"
utoipa = { version = "3.2.1", features = [ "uuid", "chrono" ] }
Expand Down
4 changes: 2 additions & 2 deletions cargo-shuttle/src/lib.rs
Expand Up @@ -1461,7 +1461,7 @@ impl Shuttle {

fn check_version(runtime_path: &Path) -> Result<()> {
let valid_version = semver::Version::from_str(VERSION)
.context("failed to convert runtime version to semver")?;
.context("failed to convert cargo-shuttle version to semver")?;

if !runtime_path.try_exists()? {
bail!("shuttle-runtime is not installed");
Expand All @@ -1484,7 +1484,7 @@ fn check_version(runtime_path: &Path) -> Result<()> {
.1
.trim(),
)
.context("failed to convert runtime version to semver")?;
.context("failed to convert user's runtime version to semver")?;

if semvers_are_compatible(&valid_version, &runtime_version) {
Ok(())
Expand Down
48 changes: 35 additions & 13 deletions codegen/src/shuttle_main/mod.rs
Expand Up @@ -15,19 +15,41 @@ pub(crate) fn r#impl(_attr: TokenStream, item: TokenStream) -> TokenStream {

let tracing_setup = if cfg!(feature = "setup-tracing") {
Some(quote! {
use shuttle_runtime::colored::*;
shuttle_runtime::colored::control::set_override(true);
shuttle_runtime::tracing_subscriber::fmt::init();
println!(
"{}\n{}\nTo disable tracing, remove the default features from {}:\n{}\n{}",
"Shuttle's default tracing subscriber is initialized!".yellow().bold(),
"=".repeat(52).yellow(),
"shuttle-runtime".italic(),
"shuttle-runtime = { version = \"*\", default-features = false }"
.white()
.italic(),
"=".repeat(52).yellow()
);
use shuttle_runtime::colored::{control, Colorize};
control::set_override(true); // always apply color

use shuttle_runtime::tracing_subscriber::prelude::*;
let level = if cfg!(debug_assertions) {
"debug,shuttle=trace,h2=info,tower=info,hyper=info"
} else {
"info,shuttle=trace"
};
shuttle_runtime::tracing_subscriber::registry()
.with(shuttle_runtime::tracing_subscriber::fmt::layer().without_time())
.with(
// let user override RUST_LOG in local run if they want to
shuttle_runtime::tracing_subscriber::EnvFilter::try_from_default_env()
// otherwise use our default
.or_else(|_| shuttle_runtime::tracing_subscriber::EnvFilter::try_new(level))
.unwrap()
)
.init();
eprintln!( // stderr to not interfere with runtime's --version output on stdout
"{}\n\
{}\n\
To disable the subscriber and use your own,\n\
remove the default features for {}:\n\
\n\
{}\n\
{}",
"=".repeat(63).yellow(),
"Shuttle's default tracing subscriber is initialized!".yellow().bold(),
"shuttle-runtime".italic(),
r#"shuttle-runtime = { version = "...", default-features = false }"#
.white()
.italic(),
"=".repeat(63).yellow()
);
})
} else {
None
Expand Down
14 changes: 9 additions & 5 deletions deployer/src/deployment/run.rs
Expand Up @@ -351,10 +351,12 @@ async fn load(
match response {
Ok(response) => {
let response = response.into_inner();

// Make sure to not log the entire response, the resources field is likely to contain
// secrets.
info!(success = %response.success, "loading response");
if response.success {
info!("Successfully loaded service");
}

let resources = response
.resources
.into_iter()
Expand All @@ -375,12 +377,12 @@ async fn load(
if response.success {
Ok(())
} else {
error!(error = %response.message, "failed to load service");
error!(error = %response.message, "Failed to load service");
Copy link
Contributor

@oddgrd oddgrd Sep 8, 2023

Choose a reason for hiding this comment

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

Nit: IMO we should be consistent with the casing of error messages (and potentially other traces too, but at least error messages), and lowercase is the norm (e.g. rust std uses lowercase without trailing punctuation). We could also discuss this internally and agree on a standard.

Err(Error::Load(response.message))
}
}
Err(error) => {
error!(%error, "failed to load service");
error!(%error, "Failed to load service");
Err(Error::Load(error.to_string()))
}
}
Expand Down Expand Up @@ -417,7 +419,9 @@ async fn run(

match response {
Ok(response) => {
info!(response = ?response.into_inner(), "start client response: ");
if response.into_inner().success {
info!("Runtime started successully");
}

// Wait for stop reason
let reason = stream.message().await.expect("message from tonic stream");
Expand Down
3 changes: 2 additions & 1 deletion runtime/Cargo.toml
Expand Up @@ -70,7 +70,8 @@ next = [
"shuttle-common/wasm",
]
setup-tracing = [
"tracing-subscriber",
"tracing-subscriber/default",
"tracing-subscriber/env-filter",
"colored",
"shuttle-codegen/setup-tracing",
]
15 changes: 8 additions & 7 deletions runtime/src/alpha/mod.rs
Expand Up @@ -56,10 +56,10 @@ pub async fn start(loader: impl Loader<ProvisionerFactory> + Send + 'static) {
let args = match Args::parse() {
Ok(args) => args,
Err(e) => {
println!("runtime received malformed or incorrect args, {e}");
eprintln!("Runtime received malformed or incorrect args, {e}");
let help_str = "[HINT]: Run shuttle with `cargo shuttle run`";
let wrapper_str = "-".repeat(help_str.len());
println!("{wrapper_str}\n{help_str}\n{wrapper_str}");
eprintln!("{wrapper_str}\n{help_str}\n{wrapper_str}");
return;
}
};
Expand Down Expand Up @@ -186,7 +186,7 @@ where
service_name,
deployment_id,
} = request.into_inner();
println!("loading alpha project at {path}");
println!("Loading alpha service at {path}");

let secrets = BTreeMap::from_iter(secrets.into_iter());

Expand Down Expand Up @@ -225,6 +225,7 @@ where

let loader = self.loader.lock().unwrap().deref_mut().take().unwrap();

// send to new thread to catch panics
let service =
match tokio::spawn(loader.load(factory, resource_tracker, deployment_id)).await {
Ok(res) => match res {
Expand Down Expand Up @@ -302,7 +303,7 @@ where
&self,
request: Request<StartRequest>,
) -> Result<Response<StartResponse>, Status> {
println!("alpha starting");
println!("Alpha runtime starting");
let service = self.service.lock().unwrap().deref_mut().take();
let service = service.unwrap();

Expand All @@ -311,7 +312,7 @@ where
.context("invalid socket address")
.map_err(|err| Status::invalid_argument(err.to_string()))?;

println!("starting on {service_address}");
println!("Starting on {service_address}");

let (kill_tx, kill_rx) = tokio::sync::oneshot::channel();
*self.kill_tx.lock().unwrap() = Some(kill_tx);
Expand Down Expand Up @@ -344,13 +345,13 @@ where
},
};

println!( "service panicked: {msg}");
println!("service panicked: {msg}");

let _ = stopped_tx
.send((StopReason::Crash, msg))
.map_err(|e| println!("{e}"));
} else {
println!( "service crashed: {error}");
println!("service crashed: {error}");
let _ = stopped_tx
.send((StopReason::Crash, error.to_string()))
.map_err(|e| println!("{e}"));
Expand Down
3 changes: 2 additions & 1 deletion service/src/builder.rs
Expand Up @@ -56,7 +56,8 @@ impl BuiltService {
}

fn extract_shuttle_toml_name(path: PathBuf) -> anyhow::Result<String> {
let shuttle_toml = read_to_string(path).context("Shuttle.toml not found")?;
let shuttle_toml =
read_to_string(path.as_path()).map_err(|_| anyhow!("{} not found", path.display()))?;

let toml: toml::Value =
toml::from_str(&shuttle_toml).context("failed to parse Shuttle.toml")?;
Expand Down