diff --git a/CHANGELOG.md b/CHANGELOG.md index c08650a4..7b126607 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ Making a new release? Simply add the new header with the version and date undern * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217]) * Fixed the sync fallback scrambling sibling order; replacements are now re-parented ancestors-first and in their original child order. ([#1265]) * Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266]) +* Rojo now reports a clear error instead of panicking in several cases, including when the `serve` port is already in use, when a synced file is read-only or locked, when the filesystem watcher can't be created, and when the working directory is inaccessible. ([#1267]) [#1176]: https://github.com/rojo-rbx/rojo/pull/1176 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179 @@ -52,6 +53,7 @@ Making a new release? Simply add the new header with the version and date undern [#1217]: https://github.com/rojo-rbx/rojo/pull/1217 [#1265]: https://github.com/rojo-rbx/rojo/pull/1265 [#1266]: https://github.com/rojo-rbx/rojo/pull/1266 +[#1267]: https://github.com/rojo-rbx/rojo/pull/1267 ## [7.7.0-rc.1] (November 27th, 2025) diff --git a/crates/memofs/CHANGELOG.md b/crates/memofs/CHANGELOG.md index c35b5399..ec0b71f0 100644 --- a/crates/memofs/CHANGELOG.md +++ b/crates/memofs/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased Changes * Added `Vfs::canonicalize`. [#1201] +* **Breaking:** `StdBackend::new` and `Vfs::new_default` now return `io::Result`, so a failure to create the filesystem watcher is reported as an error instead of panicking. The `Default` implementation for `StdBackend` has been removed as a result. [#1267] + +[#1267]: https://github.com/rojo-rbx/rojo/pull/1267 ## 0.3.1 (2025-11-27) * Added `Vfs::exists`. [#1169] diff --git a/crates/memofs/src/lib.rs b/crates/memofs/src/lib.rs index 41d21100..11acf810 100644 --- a/crates/memofs/src/lib.rs +++ b/crates/memofs/src/lib.rs @@ -255,8 +255,11 @@ pub struct Vfs { impl Vfs { /// Creates a new `Vfs` with the default backend, `StdBackend`. - pub fn new_default() -> Self { - Self::new(StdBackend::new()) + /// + /// Returns an error if the filesystem watcher could not be initialized, + /// which can happen in restricted or sandboxed environments. + pub fn new_default() -> io::Result { + Ok(Self::new(StdBackend::new()?)) } /// Creates a new `Vfs` with the given backend. @@ -639,7 +642,7 @@ mod test { let file_path = dir.path().join("file.txt"); fs_err::write(&file_path, contents.to_string()).unwrap(); - let vfs = Vfs::new(StdBackend::new()); + let vfs = Vfs::new(StdBackend::new().unwrap()); let canonicalized = vfs.canonicalize(&file_path).unwrap(); assert_eq!(canonicalized, file_path.canonicalize().unwrap()); assert_eq!( @@ -653,7 +656,7 @@ mod test { let dir = tempfile::tempdir().unwrap(); let file_path = dir.path().join("test"); - let vfs = Vfs::new(StdBackend::new()); + let vfs = Vfs::new(StdBackend::new().unwrap()); let err = vfs.canonicalize(&file_path).unwrap_err(); assert_eq!(err.kind(), io::ErrorKind::NotFound); } diff --git a/crates/memofs/src/std_backend.rs b/crates/memofs/src/std_backend.rs index a25fa39d..d684bde5 100644 --- a/crates/memofs/src/std_backend.rs +++ b/crates/memofs/src/std_backend.rs @@ -17,9 +17,9 @@ pub struct StdBackend { } impl StdBackend { - pub fn new() -> StdBackend { + pub fn new() -> io::Result { let (notify_tx, notify_rx) = mpsc::channel(); - let watcher = watcher(notify_tx, Duration::from_millis(50)).unwrap(); + let watcher = watcher(notify_tx, Duration::from_millis(50)).map_err(io::Error::other)?; let (tx, rx) = crossbeam_channel::unbounded(); @@ -46,11 +46,11 @@ impl StdBackend { Result::<(), crossbeam_channel::SendError>::Ok(()) }); - Self { + Ok(Self { watcher, watcher_receiver: rx, watches: HashSet::new(), - } + }) } } @@ -134,9 +134,3 @@ impl VfsBackend for StdBackend { self.watcher.unwatch(path).map_err(io::Error::other) } } - -impl Default for StdBackend { - fn default() -> Self { - Self::new() - } -} diff --git a/src/change_processor.rs b/src/change_processor.rs index 6c5c8224..c1f245f2 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -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."); } diff --git a/src/cli/build.rs b/src/cli/build.rs index c2ddebb3..82e81387 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -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)?; diff --git a/src/cli/fmt_project.rs b/src/cli/fmt_project.rs index 1c165974..e1c7adaa 100644 --- a/src/cli/fmt_project.rs +++ b/src/cli/fmt_project.rs @@ -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'")?; diff --git a/src/cli/init.rs b/src/cli/init.rs index eb323310..434c2e34 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -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 { 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) } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 7d3d87b1..caef95aa 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -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> { 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))) } } diff --git a/src/cli/serve.rs b/src/cli/serve.rs index f89e4387..e33c3fc5 100644 --- a/src/cli/serve.rs +++ b/src/cli/serve.rs @@ -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(()) } diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index 94369e7c..10fff590 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -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) { diff --git a/src/cli/syncback.rs b/src/cli/syncback.rs index 282d55c8..7bb274fb 100644 --- a/src/cli/syncback.rs +++ b/src/cli/syncback.rs @@ -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(); diff --git a/src/cli/upload.rs b/src/cli/upload.rs index 3c3610fd..bc9d427e 100644 --- a/src/cli/upload.rs +++ b/src/cli/upload.rs @@ -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)?; diff --git a/src/snapshot_middleware/csv.rs b/src/snapshot_middleware/csv.rs index 18fa1424..b4a86efc 100644 --- a/src/snapshot_middleware/csv.rs +++ b/src/snapshot_middleware/csv.rs @@ -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 { +fn convert_localization_csv(contents: &[u8]) -> anyhow::Result { let mut reader = csv::Reader::from_reader(contents); let headers = reader.headers()?.clone(); @@ -237,7 +237,7 @@ fn convert_localization_csv(contents: &[u8]) -> Result { } 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) } diff --git a/src/web/mod.rs b/src/web/mod.rs index d6df4b15..8e33d9c5 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -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(()) } } diff --git a/src/web/ui.rs b/src/web/ui.rs index 4d63eaad..f6361b4f 100644 --- a/src/web/ui.rs +++ b/src/web/ui.rs @@ -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 { - 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 { - 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 { @@ -66,10 +60,11 @@ impl UiService { }); - Response::builder() - .header(header::CONTENT_TYPE, "text/html") - .body(Body::from(format!("{}", page))) - .unwrap() + response( + StatusCode::OK, + "text/html", + format!("{}", page), + ) } fn handle_show_instances(&self) -> Response { @@ -80,10 +75,11 @@ impl UiService { { Self::instance(&tree, root_id) } }); - Response::builder() - .header(header::CONTENT_TYPE, "text/html") - .body(Body::from(format!("{}", page))) - .unwrap() + response( + StatusCode::OK, + "text/html", + format!("{}", page), + ) } fn instance(tree: &RojoTree, id: Ref) -> HtmlContent<'_> { diff --git a/src/web/util.rs b/src/web/util.rs index 1c1cbb50..bd1f6b81 100644 --- a/src/web/util.rs +++ b/src/web/util.rs @@ -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, +) -> Response { + 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(value: T) -> Response { msgpack(value, StatusCode::OK) } @@ -12,18 +33,14 @@ pub fn msgpack(value: T, code: StatusCode) -> Response { .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(value: T) -> anyhow::Result> { @@ -49,17 +66,13 @@ pub fn json(value: T, code: StatusCode) -> Response { 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) }