Replace .unwrap() with proper error handling in run() by LEDazzio01 · Pull Request #689 · microsoft/litebox

LEDazzio01 · 2026-02-27T00:34:57Z

Summary

Replace panic-inducing .unwrap() calls in the run() function with proper error handling using anyhow::Context, so users get actionable error messages instead of panics when the environment is not set up correctly.

Fixes #650

Problem

Running litebox_runner_linux_userland echo yay on a stock Ubuntu installation causes a panic:

thread 'main' panicked at litebox_runner_linux_userland/src/lib.rs:118:48:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }

The function's own docstring acknowledges this:

"Can panic if any particulars of the environment are not set up as expected. If it does actually panic, then ping the authors of LiteBox."

Changes

Add anyhow::Context import for .with_context() support
Replace .unwrap() on std::path::absolute() (2 occurrences) with .with_context() providing the program name in the error
Replace .unwrap() on path.metadata() with .with_context() explaining which path was inaccessible
Replace .unwrap() on Mode::from_bits() with .ok_or_else() reporting the invalid mode bits
Update docstring from # Panics to # Errors section reflecting the new behavior

Scope

This PR intentionally focuses on host-OS-facing .unwrap() calls only — the ones that interact with the real filesystem and are likely to fail from environment issues (which is the scenario reported in #650). The remaining .unwrap() calls operate on the in-memory filesystem and are left untouched to keep this change focused and reviewable.

Before / After

Before: A raw panic with a cryptic stack trace

thread 'main' panicked at litebox_runner_linux_userland/src/lib.rs:118:48:
called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }

After: A clear, contextual error message

Error: Could not read metadata for '/path/to/program'. Ensure the path exists and is accessible.

Caused by:
    No such file or directory (os error 2)

Replace panic-inducing .unwrap() calls in the `run()` function with proper error handling using `anyhow::Context`, so users get actionable error messages instead of panics when the environment is not set up correctly. Changes: - Add `anyhow::Context` import - Replace `.unwrap()` on `std::path::absolute()` with `.with_context()` - Replace `.unwrap()` on `path.metadata()` with `.with_context()` - Replace `.unwrap()` on `Mode::from_bits()` with `.ok_or_else()` - Update docstring from "Panics" to "Errors" section Fixes #650

Copilot

Pull request overview

This PR aims to prevent panic-on-misconfiguration in litebox_runner_linux_userland::run() by replacing several host-OS-facing .unwrap() calls with contextual anyhow errors, improving the user experience when the environment or paths are not set up as expected (Fixes #650).

Changes:

Import anyhow::Context and add contextual error messages for failures when resolving the program’s absolute path and reading path metadata.
Replace .unwrap() on Mode::from_bits(...) with an error-producing path.
Update the run() docstring from a # Panics section to a # Errors section.

Copilot · 2026-02-27T00:39:08Z

litebox_runner_linux_userland/src/lib.rs

+                let mode = litebox::fs::Mode::from_bits(metadata.st_mode())
+                    .ok_or_else(|| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?;


Mode::from_bits(metadata.st_mode()) is very unlikely to ever return None here: Mode is defined with const _ = !0 (accepting all remaining bits), so this new ok_or_else error path appears unreachable. Either drop the check, or (if the intent is to validate/sanitize mode bits) mask/truncate to the permission/sticky/setuid bits before constructing Mode.

Suggested change

let mode = litebox::fs::Mode::from_bits(metadata.st_mode())

.ok_or_else(|| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?;

let mode = litebox::fs::Mode::from_bits_truncate(metadata.st_mode());

Copilot · 2026-02-27T00:39:08Z

litebox_runner_linux_userland/src/lib.rs

+        let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0]))
+            .with_context(|| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?;
        let ancestors: Vec<_> = prog.ancestors().collect();


Please add a regression test covering the reported scenario (missing/unresolvable program path) to ensure run() returns a contextual Err instead of panicking. This crate already has an integration test suite under litebox_runner_linux_userland/tests, and a test here would prevent #650 from regressing.

Copilot · 2026-02-27T00:39:08Z

litebox_runner_linux_userland/src/lib.rs

+/// # Errors
 ///
-/// Can panic if any particulars of the environment are not set up as expected. Ideally, would not
-/// panic. If it does actually panic, then ping the authors of LiteBox, and likely a better error
-/// message could be thrown instead.
+/// Returns an error if the program path cannot be resolved, directories along the path are
+/// inaccessible, or the initial filesystem cannot be set up.


The docstring now implies run() returns errors for environment/setup problems, but the function can still panic (e.g., remaining .unwrap()s and panic!() paths during filesystem setup / syscall rewriting). Either update the docs to mention remaining panic conditions, or convert those remaining panics into Result errors so the # Errors section is accurate.

Copilot · 2026-02-27T00:39:09Z

litebox_runner_linux_userland/src/lib.rs

+        let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0]))
+            .with_context(|| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?;


There is still a host-OS-facing std::path::absolute(...).unwrap() later in run() (around the "We need to get the file path before enabling seccomp" block). This means the runner can still panic in the same class of environment issues this PR is addressing, and it also contradicts the PR description claiming all relevant absolute() unwraps were replaced. Consider applying the same with_context + ? handling there too (or reusing the already-resolved prog).

- Use `from_bits_truncate` instead of `ok_or_else` for Mode (comment 1) - Add regression test for nonexistent program path (comment 2) - Update docstring to mention remaining panic conditions (comment 3) - Fix third missed `.unwrap()` on `std::path::absolute` (comment 4)

LEDazzio01 · 2026-02-28T01:08:25Z

Thanks for the thorough review, @copilot! All four comments have been addressed in commit f2f38c7:

from_bits_truncate — Switched to Mode::from_bits_truncate() as suggested, since Mode accepts all bits via const _ = !0.
Regression test — Added tests/error_handling.rs with a test that invokes the runner with a nonexistent path and asserts it returns an error (not a panic) with a contextual message. Just pushed a follow-up strengthening this test to also verify the error message content.
Docstring accuracy — Updated the docstring to include both # Errors (for the new error paths) and # Panics (for remaining internal panics from in-memory fs operations and syscall rewriting).
Missed .unwrap() — Applied .with_context() to the third std::path::absolute() call near the seccomp block as well.

The test now also asserts that stderr contains a contextual error message (e.g., "Could not resolve absolute path" or "Could not read metadata") rather than just checking for the absence of a panic.

LEDazzio01

All four review comments have been addressed in the latest commit:

Mode::from_bits → from_bits_truncate — Applied as suggested, since Mode accepts all bits via const _ = !0.
Regression test — Added tests/error_handling.rs with test_nonexistent_program_returns_error_not_panic() that verifies the runner returns a contextual error (not a panic) for nonexistent program paths.
Docstring accuracy — Updated to include both # Errors (for the new contextual errors) and # Panics (for remaining internal panics from in-memory filesystem operations).
Missed .unwrap() in seccomp block — Replaced with .with_context() + ? so it returns a proper error instead of panicking.

Copilot AI review requested due to automatic review settings February 27, 2026 00:34

Copilot started reviewing on behalf of LEDazzio01 February 27, 2026 00:35 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Strengthen regression test to verify contextual error message

7a4ea7a

The test now also asserts that stderr contains a contextual error message (e.g., "Could not resolve absolute path" or "Could not read metadata") rather than just checking for the absence of a panic.

LEDazzio01 commented Feb 28, 2026

View reviewed changes

LEDazzio01 closed this by deleting the head repository Mar 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace .unwrap() with proper error handling in run()#689

Replace .unwrap() with proper error handling in run()#689
LEDazzio01 wants to merge 3 commits intomicrosoft:mainfrom
LEDazzio01:fix/650-replace-unwrap-with-error-handling

LEDazzio01 commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

LEDazzio01 commented Feb 28, 2026

Uh oh!

LEDazzio01 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		let mode = litebox::fs::Mode::from_bits(metadata.st_mode())
		.ok_or_else(\|\| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?;

	let mode = litebox::fs::Mode::from_bits(metadata.st_mode())
	.ok_or_else(\|\| anyhow!("Invalid file mode bits {:#o} for '{}'", metadata.st_mode(), path.display()))?;
	let mode = litebox::fs::Mode::from_bits_truncate(metadata.st_mode());

		let prog = std::path::absolute(Path::new(&cli_args.program_and_arguments[0]))
		.with_context(\|\| format!("Could not resolve absolute path for program '{}'", &cli_args.program_and_arguments[0]))?;

Conversation

LEDazzio01 commented Feb 27, 2026

Summary

Problem

Changes

Scope

Before / After

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

LEDazzio01 commented Feb 28, 2026

Uh oh!

LEDazzio01 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants