Skip to content

Commit

Permalink
fix: Refactor error propagation
Browse files Browse the repository at this point in the history
Refactors error propagation to avoid logging and printing misleading
error messages.

Signed-off-by: Jonathan Woollett-Light <jcawl@amazon.co.uk>
  • Loading branch information
Jonathan Woollett-Light committed Oct 18, 2023
1 parent 2db88a4 commit b507103
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 7 deletions.
13 changes: 9 additions & 4 deletions src/firecracker/src/api_server_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ApiServerAdapter {
vm_resources: VmResources,
vmm: Arc<Mutex<Vmm>>,
event_manager: &mut EventManager,
) -> FcExitCode {
) -> Result<(), FcExitCode> {

Check warning on line 55 in src/firecracker/src/api_server_adapter.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/api_server_adapter.rs#L55

Added line #L55 was not covered by tests
let api_adapter = Arc::new(Mutex::new(Self {
api_event_fd,
from_api,
Expand All @@ -64,10 +64,14 @@ impl ApiServerAdapter {
event_manager
.run()
.expect("EventManager events driver fatal error");
if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
return exit_code;

match vmm.lock().unwrap().shutdown_exit_code() {
Some(FcExitCode::Ok) => break,
Some(exit_code) => return Err(exit_code),
None => continue,

Check warning on line 71 in src/firecracker/src/api_server_adapter.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/api_server_adapter.rs#L67-L71

Added lines #L67 - L71 were not covered by tests
}
}
Ok(())

Check warning on line 74 in src/firecracker/src/api_server_adapter.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/api_server_adapter.rs#L74

Added line #L74 was not covered by tests
}

fn handle_request(&mut self, req_action: VmmAction) {
Expand Down Expand Up @@ -245,7 +249,8 @@ pub(crate) fn run_with_api(
api_thread.join().expect("Api thread should join");

match result {
Ok(exit_code) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),
Ok(Ok(())) => Ok(()),
Ok(Err(exit_code)) => Err(ApiServerError::MicroVMStoppedWithoutError(exit_code)),

Check warning on line 253 in src/firecracker/src/api_server_adapter.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/api_server_adapter.rs#L252-L253

Added lines #L252 - L253 were not covered by tests
Err(exit_error) => Err(exit_error),
}
}
8 changes: 6 additions & 2 deletions src/firecracker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ fn main() -> ExitCode {
eprintln!("Error: {err:?}");
ExitCode::from(err)
} else {
info!("Firecracker exited successfully");

Check warning on line 101 in src/firecracker/src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/main.rs#L101

Added line #L101 was not covered by tests
ExitCode::SUCCESS
}
}
Expand Down Expand Up @@ -631,8 +632,11 @@ fn run_without_api(
.run()
.expect("Failed to start the event manager");

if let Some(exit_code) = vmm.lock().unwrap().shutdown_exit_code() {
return Err(RunWithoutApiError::Shutdown(exit_code));
match vmm.lock().unwrap().shutdown_exit_code() {
Some(FcExitCode::Ok) => break,
Some(exit_code) => return Err(RunWithoutApiError::Shutdown(exit_code)),
None => continue,

Check warning on line 638 in src/firecracker/src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/main.rs#L635-L638

Added lines #L635 - L638 were not covered by tests
}
}
Ok(())

Check warning on line 641 in src/firecracker/src/main.rs

View check run for this annotation

Codecov / codecov/patch

src/firecracker/src/main.rs#L641

Added line #L641 was not covered by tests
}
2 changes: 1 addition & 1 deletion tests/integration_tests/functional/test_cmd_line_start.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def test_config_start_no_api_exit(uvm_plain, vm_config_file):

# Check error log
test_microvm.check_log_message(
"RunWithoutApiError error: MicroVMStopped without an error: Ok"
"Firecracker exited successfully"
)


Expand Down

0 comments on commit b507103

Please sign in to comment.