Graceful errors instead of crashing (#1267)

This commit is contained in:
boatbomber
2026-06-01 20:02:39 -07:00
committed by GitHub
parent 85655ca84f
commit 1abf675949
17 changed files with 167 additions and 99 deletions

View File

@@ -200,7 +200,15 @@ impl JobThreadContext {
if let Some(instance) = tree.get_instance(id) {
if let Some(instigating_source) = &instance.metadata().instigating_source {
match instigating_source {
InstigatingSource::Path(path) => fs::remove_file(path).unwrap(),
InstigatingSource::Path(path) => {
if let Err(err) = fs::remove_file(path) {
log::error!(
"Failed to remove file {}: {}",
path.display(),
err
);
}
}
InstigatingSource::ProjectNode { .. } => {
log::warn!(
"Cannot remove instance {:?}, it's from a project file",
@@ -244,7 +252,13 @@ impl JobThreadContext {
match instigating_source {
InstigatingSource::Path(path) => {
if let Some(Variant::String(value)) = changed_value {
fs::write(path, value).unwrap();
if let Err(err) = fs::write(path, value) {
log::error!(
"Failed to write file {}: {}",
path.display(),
err
);
}
} else {
log::warn!("Cannot change Source to non-string value.");
}

View File

@@ -75,10 +75,10 @@ impl BuildCommand {
_ => unreachable!(),
};
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
log::trace!("Constructing in-memory filesystem");
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(self.watch);
let session = ServeSession::new(vfs, project_path)?;
@@ -87,11 +87,16 @@ impl BuildCommand {
write_model(&session, &output_path, output_kind)?;
if self.watch {
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?;
loop {
let receiver = session.message_queue().subscribe(cursor);
let (new_cursor, _patch_set) = rt.block_on(receiver).unwrap();
let (new_cursor, _patch_set) = match rt.block_on(receiver) {
Ok(message) => message,
// The message queue was dropped, so there is nothing left
// to watch. Stop watching gracefully.
Err(_) => break,
};
cursor = new_cursor;
write_model(&session, &output_path, output_kind)?;

View File

@@ -18,10 +18,10 @@ pub struct FmtProjectCommand {
impl FmtProjectCommand {
pub fn run(self) -> anyhow::Result<()> {
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(false);
let base_path = resolve_path(&self.project);
let base_path = resolve_path(&self.project)?;
let project = Project::load_fuzzy(&vfs, &base_path)?
.context("A project file is required to run 'rojo fmt-project'")?;

View File

@@ -9,7 +9,7 @@ use std::{
io::{self, Write},
};
use anyhow::{bail, format_err};
use anyhow::{bail, format_err, Context};
use clap::Parser;
use fs_err as fs;
use fs_err::OpenOptions;
@@ -42,9 +42,9 @@ pub struct InitCommand {
impl InitCommand {
pub fn run(self) -> anyhow::Result<()> {
let template = self.kind.template();
let template = self.kind.template()?;
let base_path = resolve_path(&self.path);
let base_path = resolve_path(&self.path)?;
fs::create_dir_all(&base_path)?;
let canonical = fs::canonicalize(&base_path)?;
@@ -128,7 +128,7 @@ pub enum InitKind {
}
impl InitKind {
fn template(&self) -> InMemoryFs {
fn template(&self) -> anyhow::Result<InMemoryFs> {
let template_path = match self {
Self::Place => "place",
Self::Model => "model",
@@ -136,20 +136,24 @@ impl InitKind {
};
let snapshot: VfsSnapshot = bincode::deserialize(TEMPLATE_BINCODE)
.expect("Rojo's templates were not properly packed into Rojo's binary");
.context("Rojo's templates were not properly packed into Rojo's binary. This is a bug in Rojo; please file an issue.")?;
if let VfsSnapshot::Dir { mut children } = snapshot {
if let Some(template) = children.remove(template_path) {
let mut fs = InMemoryFs::new();
fs.load_snapshot("", template)
.expect("loading a template in memory should never fail");
fs
} else {
panic!("template for project type {:?} is missing", self)
}
} else {
panic!("Rojo's templates were packed as a file instead of a directory")
}
let VfsSnapshot::Dir { mut children } = snapshot else {
bail!("Rojo's templates were packed as a file instead of a directory. This is a bug in Rojo; please file an issue.");
};
let template = children.remove(template_path).ok_or_else(|| {
format_err!(
"The template for project type {:?} is missing. This is a bug in Rojo; please file an issue.",
self
)
})?;
let mut fs = InMemoryFs::new();
fs.load_snapshot("", template)
.context("Failed to load Rojo's bundled template into memory")?;
Ok(fs)
}
}

View File

@@ -12,6 +12,7 @@ mod upload;
use std::{borrow::Cow, env, path::Path, str::FromStr};
use anyhow::Context;
use clap::Parser;
use thiserror::Error;
@@ -125,10 +126,14 @@ pub enum Subcommand {
Syncback(SyncbackCommand),
}
pub(super) fn resolve_path(path: &Path) -> Cow<'_, Path> {
pub(super) fn resolve_path(path: &Path) -> anyhow::Result<Cow<'_, Path>> {
if path.is_absolute() {
Cow::Borrowed(path)
Ok(Cow::Borrowed(path))
} else {
Cow::Owned(env::current_dir().unwrap().join(path))
let current_dir = env::current_dir().context(
"Could not determine the current working directory. \
It may have been deleted, or Rojo may not have permission to access it.",
)?;
Ok(Cow::Owned(current_dir.join(path)))
}
}

View File

@@ -35,9 +35,9 @@ pub struct ServeCommand {
impl ServeCommand {
pub fn run(self, global: GlobalOptions) -> anyhow::Result<()> {
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
let session = Arc::new(ServeSession::new(vfs, project_path)?);
@@ -53,8 +53,9 @@ impl ServeCommand {
let server = LiveServer::new(session);
let _ = show_start_message(ip, port, global.color.into());
server.start((ip, port).into());
server.start((ip, port).into(), || {
let _ = show_start_message(ip, port, global.color.into());
})?;
Ok(())
}

View File

@@ -5,6 +5,7 @@ use std::{
path::{self, Path, PathBuf},
};
use anyhow::Context;
use clap::Parser;
use fs_err::File;
use memofs::Vfs;
@@ -71,10 +72,10 @@ pub struct SourcemapCommand {
impl SourcemapCommand {
pub fn run(self) -> anyhow::Result<()> {
let project_path = fs_err::canonicalize(resolve_path(&self.project))?;
let project_path = fs_err::canonicalize(resolve_path(&self.project)?)?;
log::trace!("Constructing filesystem with StdBackend");
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(self.watch);
log::trace!("Setting up session for sourcemap generation");
@@ -100,11 +101,16 @@ impl SourcemapCommand {
if self.watch {
log::trace!("Setting up runtime for watch mode");
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for watch mode")?;
loop {
let receiver = session.message_queue().subscribe(cursor);
let (new_cursor, patch_set) = rt.block_on(receiver).unwrap();
let (new_cursor, patch_set) = match rt.block_on(receiver) {
Ok(message) => message,
// The message queue was dropped, so there is nothing left
// to watch. Stop watching gracefully.
Err(_) => break,
};
cursor = new_cursor;
if patch_set_affects_sourcemap(&session, &patch_set, filter) {

View File

@@ -58,8 +58,8 @@ pub struct SyncbackCommand {
impl SyncbackCommand {
pub fn run(&self, global: GlobalOptions) -> anyhow::Result<()> {
let path_old = resolve_path(&self.project);
let path_new = resolve_path(&self.input);
let path_old = resolve_path(&self.project)?;
let path_new = resolve_path(&self.input)?;
let input_kind = FileKind::from_path(&path_new).context(UNKNOWN_INPUT_KIND_ERR)?;
let dom_start_timer = Instant::now();
@@ -69,7 +69,7 @@ impl SyncbackCommand {
dom_start_timer.elapsed().as_secs_f32()
);
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
vfs.set_watch_enabled(false);
let project_start_timer = Instant::now();

View File

@@ -38,9 +38,9 @@ pub struct UploadCommand {
impl UploadCommand {
pub fn run(self) -> Result<(), anyhow::Error> {
let project_path = resolve_path(&self.project);
let project_path = resolve_path(&self.project)?;
let vfs = Vfs::new_default();
let vfs = Vfs::new_default()?;
let session = ServeSession::new(vfs, project_path)?;

View File

@@ -195,7 +195,7 @@ struct LocalizationEntry<'a> {
/// https://github.com/BurntSushi/rust-csv/issues/151
///
/// This function operates in one step in order to minimize data-copying.
fn convert_localization_csv(contents: &[u8]) -> Result<String, csv::Error> {
fn convert_localization_csv(contents: &[u8]) -> anyhow::Result<String> {
let mut reader = csv::Reader::from_reader(contents);
let headers = reader.headers()?.clone();
@@ -237,7 +237,7 @@ fn convert_localization_csv(contents: &[u8]) -> Result<String, csv::Error> {
}
let encoded =
serde_json::to_string(&entries).expect("Could not encode JSON for localization table");
serde_json::to_string(&entries).context("Could not encode JSON for localization table")?;
Ok(encoded)
}

View File

@@ -12,6 +12,7 @@ use std::convert::Infallible;
use std::net::SocketAddr;
use std::sync::Arc;
use anyhow::Context;
use hyper::{
server::Server,
service::{make_service_fn, service_fn},
@@ -30,7 +31,12 @@ impl LiveServer {
LiveServer { serve_session }
}
pub fn start(self, address: SocketAddr) {
/// Starts the server on the given address, blocking until it stops.
///
/// `on_listening` is invoked once the server has successfully bound to the
/// address, so callers can defer printing any "listening" message until
/// after binding can no longer fail (e.g. due to the port being in use).
pub fn start(self, address: SocketAddr, on_listening: impl FnOnce()) -> anyhow::Result<()> {
let serve_session = Arc::clone(&self.serve_session);
let make_service = make_service_fn(move |_conn| {
@@ -53,9 +59,25 @@ impl LiveServer {
}
});
let rt = Runtime::new().unwrap();
let rt = Runtime::new().context("Failed to start the async runtime for the web server")?;
let _guard = rt.enter();
let server = Server::bind(&address).serve(make_service);
rt.block_on(server).unwrap();
let server = Server::try_bind(&address)
.with_context(|| {
format!(
"Could not start the Rojo server on {address}.\n\
The address may already be in use or reserved. Another Rojo server might already \
be running, or another program may be using that port.\n\
You can pick a different port with the --port option."
)
})?
.serve(make_service);
// Binding succeeded, so it's now safe to tell the user we're listening.
on_listening();
rt.block_on(server)
.context("The Rojo web server encountered a fatal error")?;
Ok(())
}
}

View File

@@ -6,7 +6,7 @@
use std::{borrow::Cow, sync::Arc, time::Duration};
use hyper::{header, Body, Method, Request, Response, StatusCode};
use hyper::{Body, Method, Request, Response, StatusCode};
use rbx_dom_weak::types::{Ref, Variant};
use ritz::{html, Fragment, HtmlContent, HtmlSelfClosingTag};
@@ -16,7 +16,7 @@ use crate::{
web::{
assets,
interface::{ErrorResponse, SERVER_VERSION},
util::json,
util::{json, response},
},
};
@@ -45,17 +45,11 @@ impl UiService {
}
fn handle_logo(&self) -> Response<Body> {
Response::builder()
.header(header::CONTENT_TYPE, "image/png")
.body(Body::from(assets::logo()))
.unwrap()
response(StatusCode::OK, "image/png", assets::logo())
}
fn handle_icon(&self) -> Response<Body> {
Response::builder()
.header(header::CONTENT_TYPE, "image/png")
.body(Body::from(assets::icon()))
.unwrap()
response(StatusCode::OK, "image/png", assets::icon())
}
fn handle_home(&self) -> Response<Body> {
@@ -66,10 +60,11 @@ impl UiService {
</div>
});
Response::builder()
.header(header::CONTENT_TYPE, "text/html")
.body(Body::from(format!("<!DOCTYPE html>{}", page)))
.unwrap()
response(
StatusCode::OK,
"text/html",
format!("<!DOCTYPE html>{}", page),
)
}
fn handle_show_instances(&self) -> Response<Body> {
@@ -80,10 +75,11 @@ impl UiService {
{ Self::instance(&tree, root_id) }
});
Response::builder()
.header(header::CONTENT_TYPE, "text/html")
.body(Body::from(format!("<!DOCTYPE html>{}", page)))
.unwrap()
response(
StatusCode::OK,
"text/html",
format!("<!DOCTYPE html>{}", page),
)
}
fn instance(tree: &RojoTree, id: Ref) -> HtmlContent<'_> {

View File

@@ -1,6 +1,27 @@
use hyper::{header::CONTENT_TYPE, Body, Response, StatusCode};
use serde::{Deserialize, Serialize};
/// Builds an HTTP response, falling back to an empty `500` response (rather than
/// panicking) if the response could not be constructed. With constant headers
/// and a valid status code this never actually fails, but routing every
/// response through here means a malformed response can never crash the server.
pub fn response(
code: StatusCode,
content_type: &'static str,
body: impl Into<Body>,
) -> Response<Body> {
Response::builder()
.status(code)
.header(CONTENT_TYPE, content_type)
.body(body.into())
.unwrap_or_else(|err| {
log::error!("Failed to build HTTP response: {}", err);
let mut fallback = Response::new(Body::empty());
*fallback.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
fallback
})
}
pub fn msgpack_ok<T: Serialize>(value: T) -> Response<Body> {
msgpack(value, StatusCode::OK)
}
@@ -12,18 +33,14 @@ pub fn msgpack<T: Serialize>(value: T, code: StatusCode) -> Response<Body> {
.with_struct_map();
if let Err(err) = value.serialize(&mut serializer) {
return Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.header(CONTENT_TYPE, "text/plain")
.body(Body::from(err.to_string()))
.unwrap();
return response(
StatusCode::INTERNAL_SERVER_ERROR,
"text/plain",
err.to_string(),
);
};
Response::builder()
.status(code)
.header(CONTENT_TYPE, "application/msgpack")
.body(Body::from(serialized))
.unwrap()
response(code, "application/msgpack", serialized)
}
pub fn serialize_msgpack<T: Serialize>(value: T) -> anyhow::Result<Vec<u8>> {
@@ -49,17 +66,13 @@ pub fn json<T: Serialize>(value: T, code: StatusCode) -> Response<Body> {
let serialized = match serde_json::to_string(&value) {
Ok(v) => v,
Err(err) => {
return Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.header(CONTENT_TYPE, "text/plain")
.body(Body::from(err.to_string()))
.unwrap();
return response(
StatusCode::INTERNAL_SERVER_ERROR,
"text/plain",
err.to_string(),
);
}
};
Response::builder()
.status(code)
.header(CONTENT_TYPE, "application/json")
.body(Body::from(serialized))
.unwrap()
response(code, "application/json", serialized)
}