From 376dc971344e4627519f10f7b42b45da518d4834 Mon Sep 17 00:00:00 2001 From: mofeng-git Date: Sun, 26 Jul 2026 22:59:27 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=20MSD=20ISO/FLASH=20?= =?UTF-8?q?=E6=8C=82=E8=BD=BD=E8=AF=86=E5=88=AB=E9=94=99=E8=AF=AF=EF=BC=9B?= =?UTF-8?q?=E5=AE=8C=E5=96=84=E9=94=99=E8=AF=AF=E6=8F=90=E7=A4=BA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/auth/middleware.rs | 1 + src/error.rs | 366 ++++++++++++++++++++ src/events/types.rs | 3 + src/msd/controller.rs | 174 ++++++---- src/msd/image.rs | 170 ++++----- src/msd/mod.rs | 1 + src/msd/types.rs | 2 +- src/msd/ventoy_drive.rs | 153 ++++---- src/otg/msd.rs | 348 +++++++++++++++++-- src/otg/service.rs | 23 +- src/redfish/auth.rs | 5 +- src/redfish/routes/virtual_media.rs | 94 ++++- src/redfish/schema.rs | 18 + src/web/error.rs | 116 ++++++- src/web/handlers/msd_api.rs | 300 +++++++++------- web/src/api/index.ts | 83 ++--- web/src/api/request.ts | 137 ++++++-- web/src/components/ActionBar.vue | 4 +- web/src/components/MsdDialog.vue | 115 ++++-- web/src/components/PasteModal.vue | 6 +- web/src/components/ui/textarea/Textarea.vue | 10 + web/src/i18n/en-US.ts | 41 +++ web/src/i18n/zh-CN.ts | 41 +++ web/src/views/ConsoleView.vue | 14 +- 24 files changed, 1703 insertions(+), 522 deletions(-) diff --git a/src/auth/middleware.rs b/src/auth/middleware.rs index 625b67be..40d75cdf 100644 --- a/src/auth/middleware.rs +++ b/src/auth/middleware.rs @@ -71,6 +71,7 @@ pub async fn auth_middleware( fn unauthorized_response(message: &str) -> Response { let body = ErrorResponse { success: false, + code: None, message: message.to_string(), }; (StatusCode::UNAUTHORIZED, Json(body)).into_response() diff --git a/src/error.rs b/src/error.rs index 390f3511..43d77fca 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,236 @@ +use serde::Serialize; +use std::fmt; use thiserror::Error; +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "SCREAMING_SNAKE_CASE")] +pub enum MsdErrorCode { + MsdUnavailable, + MsdOperationInProgress, + MsdOperationFailed, + MsdInvalidRequest, + MsdResourceNotFound, + MsdResourceAlreadyExists, + MsdMediaSlotsFull, + MsdMediaAlreadyMounted, + MsdMediaInUse, + MsdImageTooLarge, + MsdInvalidUrl, + MsdRemoteDownloadFailed, + MsdDownloadIncomplete, + MsdDriveNotInitialized, + MsdDriveConnected, + MsdDriveFilesystemUnsupported, + MsdDriveSizeInvalid, + MsdStorageSpaceUnavailable, + MsdStorageFull, + MsdStorageReadOnly, + MsdStoragePermissionDenied, + MsdMediumRemovalPrevented, + MsdDisconnectFailed, +} + +impl MsdErrorCode { + pub const ALL: [Self; 23] = [ + Self::MsdUnavailable, + Self::MsdOperationInProgress, + Self::MsdOperationFailed, + Self::MsdInvalidRequest, + Self::MsdResourceNotFound, + Self::MsdResourceAlreadyExists, + Self::MsdMediaSlotsFull, + Self::MsdMediaAlreadyMounted, + Self::MsdMediaInUse, + Self::MsdImageTooLarge, + Self::MsdInvalidUrl, + Self::MsdRemoteDownloadFailed, + Self::MsdDownloadIncomplete, + Self::MsdDriveNotInitialized, + Self::MsdDriveConnected, + Self::MsdDriveFilesystemUnsupported, + Self::MsdDriveSizeInvalid, + Self::MsdStorageSpaceUnavailable, + Self::MsdStorageFull, + Self::MsdStorageReadOnly, + Self::MsdStoragePermissionDenied, + Self::MsdMediumRemovalPrevented, + Self::MsdDisconnectFailed, + ]; + + pub const fn as_str(self) -> &'static str { + match self { + Self::MsdUnavailable => "MSD_UNAVAILABLE", + Self::MsdOperationInProgress => "MSD_OPERATION_IN_PROGRESS", + Self::MsdOperationFailed => "MSD_OPERATION_FAILED", + Self::MsdInvalidRequest => "MSD_INVALID_REQUEST", + Self::MsdResourceNotFound => "MSD_RESOURCE_NOT_FOUND", + Self::MsdResourceAlreadyExists => "MSD_RESOURCE_ALREADY_EXISTS", + Self::MsdMediaSlotsFull => "MSD_MEDIA_SLOTS_FULL", + Self::MsdMediaAlreadyMounted => "MSD_MEDIA_ALREADY_MOUNTED", + Self::MsdMediaInUse => "MSD_MEDIA_IN_USE", + Self::MsdImageTooLarge => "MSD_IMAGE_TOO_LARGE", + Self::MsdInvalidUrl => "MSD_INVALID_URL", + Self::MsdRemoteDownloadFailed => "MSD_REMOTE_DOWNLOAD_FAILED", + Self::MsdDownloadIncomplete => "MSD_DOWNLOAD_INCOMPLETE", + Self::MsdDriveNotInitialized => "MSD_DRIVE_NOT_INITIALIZED", + Self::MsdDriveConnected => "MSD_DRIVE_CONNECTED", + Self::MsdDriveFilesystemUnsupported => "MSD_DRIVE_FILESYSTEM_UNSUPPORTED", + Self::MsdDriveSizeInvalid => "MSD_DRIVE_SIZE_INVALID", + Self::MsdStorageSpaceUnavailable => "MSD_STORAGE_SPACE_UNAVAILABLE", + Self::MsdStorageFull => "MSD_STORAGE_FULL", + Self::MsdStorageReadOnly => "MSD_STORAGE_READ_ONLY", + Self::MsdStoragePermissionDenied => "MSD_STORAGE_PERMISSION_DENIED", + Self::MsdMediumRemovalPrevented => "MSD_MEDIUM_REMOVAL_PREVENTED", + Self::MsdDisconnectFailed => "MSD_DISCONNECT_FAILED", + } + } + + pub const fn message(self) -> &'static str { + match self { + Self::MsdUnavailable => "Virtual media service is unavailable.", + Self::MsdOperationInProgress => "Another virtual media operation is in progress.", + Self::MsdOperationFailed => "The virtual media operation failed.", + Self::MsdInvalidRequest => "The virtual media request is invalid.", + Self::MsdResourceNotFound => "The requested virtual media resource was not found.", + Self::MsdResourceAlreadyExists => "The virtual media resource already exists.", + Self::MsdMediaSlotsFull => "All virtual media slots are in use.", + Self::MsdMediaAlreadyMounted => "The virtual medium is already mounted.", + Self::MsdMediaInUse => "The virtual medium is currently in use.", + Self::MsdImageTooLarge => "The virtual media image is too large.", + Self::MsdInvalidUrl => "The download URL is invalid.", + Self::MsdRemoteDownloadFailed => "The remote image download failed.", + Self::MsdDownloadIncomplete => "The remote image download was incomplete.", + Self::MsdDriveNotInitialized => "The virtual drive is not initialized.", + Self::MsdDriveConnected => "The virtual drive is connected to the controlled computer.", + Self::MsdDriveFilesystemUnsupported => "The virtual drive filesystem is unsupported.", + Self::MsdDriveSizeInvalid => "The virtual drive size is invalid.", + Self::MsdStorageSpaceUnavailable => { + "Available virtual media storage space could not be determined." + } + Self::MsdStorageFull => "Virtual media storage does not have enough free space.", + Self::MsdStorageReadOnly => "Virtual media storage is read-only.", + Self::MsdStoragePermissionDenied => { + "Permission to access virtual media storage was denied." + } + Self::MsdMediumRemovalPrevented => { + "The controlled computer prevented removal of the virtual medium." + } + Self::MsdDisconnectFailed => "The virtual medium could not be disconnected.", + } + } + + pub const fn redfish_key(self) -> &'static str { + match self { + Self::MsdUnavailable => "MsdUnavailable", + Self::MsdOperationInProgress => "MsdOperationInProgress", + Self::MsdOperationFailed => "MsdOperationFailed", + Self::MsdInvalidRequest => "MsdInvalidRequest", + Self::MsdResourceNotFound => "MsdResourceNotFound", + Self::MsdResourceAlreadyExists => "MsdResourceAlreadyExists", + Self::MsdMediaSlotsFull => "MsdMediaSlotsFull", + Self::MsdMediaAlreadyMounted => "MsdMediaAlreadyMounted", + Self::MsdMediaInUse => "MsdMediaInUse", + Self::MsdImageTooLarge => "MsdImageTooLarge", + Self::MsdInvalidUrl => "MsdInvalidUrl", + Self::MsdRemoteDownloadFailed => "MsdRemoteDownloadFailed", + Self::MsdDownloadIncomplete => "MsdDownloadIncomplete", + Self::MsdDriveNotInitialized => "MsdDriveNotInitialized", + Self::MsdDriveConnected => "MsdDriveConnected", + Self::MsdDriveFilesystemUnsupported => "MsdDriveFilesystemUnsupported", + Self::MsdDriveSizeInvalid => "MsdDriveSizeInvalid", + Self::MsdStorageSpaceUnavailable => "MsdStorageSpaceUnavailable", + Self::MsdStorageFull => "MsdStorageFull", + Self::MsdStorageReadOnly => "MsdStorageReadOnly", + Self::MsdStoragePermissionDenied => "MsdStoragePermissionDenied", + Self::MsdMediumRemovalPrevented => "MsdMediumRemovalPrevented", + Self::MsdDisconnectFailed => "MsdDisconnectFailed", + } + } + + pub const fn severity(self) -> &'static str { + match self { + Self::MsdUnavailable | Self::MsdOperationFailed | Self::MsdDisconnectFailed => { + "Critical" + } + _ => "Warning", + } + } + + pub const fn resolution(self) -> &'static str { + match self { + Self::MsdUnavailable => "Enable or restore the virtual media service, then retry.", + Self::MsdOperationInProgress => { + "Wait for the current virtual media operation to finish, then retry." + } + Self::MsdResourceNotFound | Self::MsdDriveNotInitialized => { + "Verify that the requested virtual media resource exists, then retry." + } + Self::MsdResourceAlreadyExists => { + "Use a different resource name or remove the existing resource, then retry." + } + Self::MsdMediaSlotsFull => "Eject an inserted virtual medium, then retry.", + Self::MsdMediaAlreadyMounted => { + "Eject the existing virtual medium before mounting it again." + } + Self::MsdMediaInUse | Self::MsdDriveConnected | Self::MsdMediumRemovalPrevented => { + "Eject or unmount the virtual medium on the controlled computer, then retry." + } + Self::MsdImageTooLarge | Self::MsdDriveSizeInvalid => { + "Use a supported image or virtual drive size, then retry." + } + Self::MsdInvalidUrl | Self::MsdInvalidRequest => "Correct the request and retry.", + Self::MsdRemoteDownloadFailed | Self::MsdDownloadIncomplete => { + "Verify the remote server and network connection, then retry." + } + Self::MsdDriveFilesystemUnsupported => { + "Reinitialize the virtual drive with a supported filesystem, then retry." + } + Self::MsdStorageSpaceUnavailable => { + "Verify that virtual media storage is available, then retry." + } + Self::MsdStorageFull => { + "Free space in virtual media storage or select a smaller image, then retry." + } + Self::MsdStorageReadOnly => "Make virtual media storage writable, then retry.", + Self::MsdStoragePermissionDenied => { + "Correct virtual media storage permissions, then retry." + } + Self::MsdOperationFailed | Self::MsdDisconnectFailed => { + "Retry the operation. If the problem persists, check the One-KVM system logs." + } + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MsdError { + code: MsdErrorCode, +} + +impl MsdError { + pub const fn new(code: MsdErrorCode) -> Self { + Self { code } + } + + pub const fn code(self) -> MsdErrorCode { + self.code + } +} + +impl fmt::Display for MsdError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(self.code.message()) + } +} + +impl std::error::Error for MsdError {} + +impl From for AppError { + fn from(code: MsdErrorCode) -> Self { + Self::Msd(MsdError::new(code)) + } +} + #[derive(Error, Debug)] pub enum AppError { #[error("Authentication failed: {0}")] @@ -26,6 +257,9 @@ pub enum AppError { #[error("Internal error: {0}")] Internal(String), + #[error(transparent)] + Msd(#[from] MsdError), + #[error("Configuration error: {0}")] Config(String), @@ -66,3 +300,135 @@ impl From for AppError { AppError::Persistence(err.to_string()) } } + +#[cfg(test)] +mod tests { + use super::MsdErrorCode::*; + + #[test] + fn msd_codes_and_messages_are_stable() { + let cases = [ + ( + MsdUnavailable, + "MSD_UNAVAILABLE", + "Virtual media service is unavailable.", + ), + ( + MsdOperationInProgress, + "MSD_OPERATION_IN_PROGRESS", + "Another virtual media operation is in progress.", + ), + ( + MsdOperationFailed, + "MSD_OPERATION_FAILED", + "The virtual media operation failed.", + ), + ( + MsdInvalidRequest, + "MSD_INVALID_REQUEST", + "The virtual media request is invalid.", + ), + ( + MsdResourceNotFound, + "MSD_RESOURCE_NOT_FOUND", + "The requested virtual media resource was not found.", + ), + ( + MsdResourceAlreadyExists, + "MSD_RESOURCE_ALREADY_EXISTS", + "The virtual media resource already exists.", + ), + ( + MsdMediaSlotsFull, + "MSD_MEDIA_SLOTS_FULL", + "All virtual media slots are in use.", + ), + ( + MsdMediaAlreadyMounted, + "MSD_MEDIA_ALREADY_MOUNTED", + "The virtual medium is already mounted.", + ), + ( + MsdMediaInUse, + "MSD_MEDIA_IN_USE", + "The virtual medium is currently in use.", + ), + ( + MsdImageTooLarge, + "MSD_IMAGE_TOO_LARGE", + "The virtual media image is too large.", + ), + ( + MsdInvalidUrl, + "MSD_INVALID_URL", + "The download URL is invalid.", + ), + ( + MsdRemoteDownloadFailed, + "MSD_REMOTE_DOWNLOAD_FAILED", + "The remote image download failed.", + ), + ( + MsdDownloadIncomplete, + "MSD_DOWNLOAD_INCOMPLETE", + "The remote image download was incomplete.", + ), + ( + MsdDriveNotInitialized, + "MSD_DRIVE_NOT_INITIALIZED", + "The virtual drive is not initialized.", + ), + ( + MsdDriveConnected, + "MSD_DRIVE_CONNECTED", + "The virtual drive is connected to the controlled computer.", + ), + ( + MsdDriveFilesystemUnsupported, + "MSD_DRIVE_FILESYSTEM_UNSUPPORTED", + "The virtual drive filesystem is unsupported.", + ), + ( + MsdDriveSizeInvalid, + "MSD_DRIVE_SIZE_INVALID", + "The virtual drive size is invalid.", + ), + ( + MsdStorageSpaceUnavailable, + "MSD_STORAGE_SPACE_UNAVAILABLE", + "Available virtual media storage space could not be determined.", + ), + ( + MsdStorageFull, + "MSD_STORAGE_FULL", + "Virtual media storage does not have enough free space.", + ), + ( + MsdStorageReadOnly, + "MSD_STORAGE_READ_ONLY", + "Virtual media storage is read-only.", + ), + ( + MsdStoragePermissionDenied, + "MSD_STORAGE_PERMISSION_DENIED", + "Permission to access virtual media storage was denied.", + ), + ( + MsdMediumRemovalPrevented, + "MSD_MEDIUM_REMOVAL_PREVENTED", + "The controlled computer prevented removal of the virtual medium.", + ), + ( + MsdDisconnectFailed, + "MSD_DISCONNECT_FAILED", + "The virtual medium could not be disconnected.", + ), + ]; + + assert_eq!(cases.len(), super::MsdErrorCode::ALL.len()); + for (code, expected_code, expected_message) in cases { + assert_eq!(code.as_str(), expected_code); + assert_eq!(code.message(), expected_message); + } + } +} diff --git a/src/events/types.rs b/src/events/types.rs index 2cf37551..0edcd47e 100644 --- a/src/events/types.rs +++ b/src/events/types.rs @@ -204,6 +204,8 @@ pub enum SystemEvent { total_bytes: Option, progress_pct: Option, status: String, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, }, #[serde(rename = "system.device_info")] @@ -372,6 +374,7 @@ mod tests { total_bytes: None, progress_pct: None, status: String::new(), + error_code: None, }, SystemEvent::DeviceInfo { video: VideoDeviceInfo { diff --git a/src/msd/controller.rs b/src/msd/controller.rs index bfbfcf58..6c7fff2b 100644 --- a/src/msd/controller.rs +++ b/src/msd/controller.rs @@ -11,7 +11,7 @@ use super::types::{ DiskMode, DownloadProgress, DownloadStatus, DriveInfo, ImageInfo, MountedMedia, MountedMediaKind, MsdState, }; -use crate::error::{AppError, Result}; +use crate::error::{AppError, MsdErrorCode, Result}; use crate::otg::{MsdFunction, MsdLunConfig, OtgService}; pub struct MsdController { @@ -70,9 +70,11 @@ impl MsdController { } info!("Fetching MSD function from OtgService"); - let msd_func = self.otg_service.msd_function().await.ok_or_else(|| { - AppError::Internal("MSD function is not active in OtgService".to_string()) - })?; + let msd_func = self + .otg_service + .msd_function() + .await + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable))?; *self.msd_function.write().await = Some(msd_func); @@ -148,7 +150,7 @@ impl MsdController { read_only: bool, requested_lun: Option, ) -> Result<()> { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let mut state = self.state.write().await; let previous_state = state.clone(); @@ -159,7 +161,7 @@ impl MsdController { self.monitor .report_error(&error_msg, "image_not_found") .await; - return Err(AppError::Internal(error_msg)); + return Err(MsdErrorCode::MsdResourceNotFound.into()); } if state @@ -167,7 +169,7 @@ impl MsdController { .iter() .any(|media| media.kind == MountedMediaKind::Image && media.id == image.id) { - return Err(AppError::BadRequest("Image is already mounted".to_string())); + return Err(MsdErrorCode::MsdMediaAlreadyMounted.into()); } let lun = Self::select_lun(&state, requested_lun)?; @@ -195,19 +197,17 @@ impl MsdController { } pub async fn mount_drive(&self) -> Result<()> { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let mut state = self.state.write().await; let previous_state = state.clone(); self.assert_available(&state).await?; if !self.drive_path.exists() { - let err = - AppError::Internal("Virtual drive not initialized. Call init first.".to_string()); self.monitor .report_error("Virtual drive not initialized", "drive_not_found") .await; - return Err(err); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let drive_info = state.drive_info.clone().or_else(|| { @@ -230,15 +230,13 @@ impl MsdController { .iter() .any(|media| media.kind == MountedMediaKind::Drive) { - return Err(AppError::BadRequest( - "Virtual drive is already mounted".to_string(), - )); + return Err(MsdErrorCode::MsdMediaAlreadyMounted.into()); } - let drive_info = drive_info - .ok_or_else(|| AppError::Internal("Virtual drive info is unavailable".to_string()))?; + let drive_info = + drive_info.ok_or_else(|| AppError::from(MsdErrorCode::MsdDriveNotInitialized))?; let lun = Self::lowest_free_lun(&state) - .ok_or_else(|| AppError::BadRequest("Media slots are full".to_string()))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdMediaSlotsFull))?; let media = MountedMedia::drive(lun, &drive_info); if let Err(e) = self.configure_media(&media).await { @@ -265,7 +263,7 @@ impl MsdController { self.monitor .report_error("MSD not available", "not_available") .await; - return Err(AppError::Internal("MSD not available".to_string())); + return Err(MsdErrorCode::MsdUnavailable.into()); } Ok(()) } @@ -286,20 +284,14 @@ impl MsdController { fn select_lun(state: &MsdState, requested_lun: Option) -> Result { let Some(lun) = requested_lun else { return Self::lowest_free_lun(state) - .ok_or_else(|| AppError::BadRequest("Media slots are full".to_string())); + .ok_or_else(|| AppError::from(MsdErrorCode::MsdMediaSlotsFull)); }; if lun >= state.disk_mode.capacity() { - return Err(AppError::BadRequest(format!( - "Media slot {} is outside the current disk mode capacity", - lun + 1 - ))); + return Err(MsdErrorCode::MsdInvalidRequest.into()); } if state.mounted_media.iter().any(|media| media.lun == lun) { - return Err(AppError::BadRequest(format!( - "Media slot {} is already occupied", - lun + 1 - ))); + return Err(MsdErrorCode::MsdMediaSlotsFull.into()); } Ok(lun) } @@ -310,7 +302,7 @@ impl MsdController { } pub async fn set_disk_mode(&self, disk_mode: DiskMode) -> Result { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let previous_state = { let mut state = self.state.write().await; self.assert_available(&state).await?; @@ -327,9 +319,10 @@ impl MsdController { self.otg_service .set_msd_lun_capacity(disk_mode.capacity()) .await?; - self.otg_service.msd_function().await.ok_or_else(|| { - AppError::Internal("MSD function missing after OTG rebuild".to_string()) - }) + self.otg_service + .msd_function() + .await + .ok_or_else(|| AppError::from(MsdErrorCode::MsdOperationFailed)) } .await; @@ -349,7 +342,7 @@ impl MsdController { .report_error(&error_msg, "disk_mode_rollback_failed") .await; self.mark_device_info_dirty().await; - return Err(AppError::Internal(error_msg)); + return Err(MsdErrorCode::MsdOperationFailed.into()); } let mut state = self.state.write().await; @@ -360,7 +353,7 @@ impl MsdController { .report_error(&error_msg, "disk_mode_switch_failed") .await; self.mark_device_info_dirty().await; - return Err(AppError::Internal(error_msg)); + return Err(MsdErrorCode::MsdOperationFailed.into()); } }; *self.msd_function.write().await = Some(msd_function); @@ -397,7 +390,7 @@ impl MsdController { where F: Fn(&MountedMedia) -> bool, { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let mut state = self.state.write().await; let Some(index) = state.mounted_media.iter().position(predicate) else { @@ -419,25 +412,22 @@ impl MsdController { } async fn configure_media(&self, media: &MountedMedia) -> Result<()> { - let gadget_path = self.active_gadget_path().await?; - let msd_hold = self.msd_function.read().await; - let Some(ref msd) = *msd_hold else { - self.monitor - .report_error("MSD function not initialized", "not_initialized") - .await; - return Err(AppError::Internal( - "MSD function not initialized".to_string(), - )); - }; - if let Err(e) = msd - .configure_lun_async(&gadget_path, media.lun, &Self::media_config(media)) + if let Err(e) = self + .otg_service + .configure_msd_lun(media.lun, &Self::media_config(media)) .await { let error_msg = format!("Failed to configure LUN {}: {}", media.lun, e); self.monitor .report_error(&error_msg, "configfs_error") .await; - return Err(e); + return Err(match e { + AppError::Msd(error) => AppError::Msd(error), + error => { + warn!(%error, "Unclassified MSD media configuration failure"); + MsdErrorCode::MsdOperationFailed.into() + } + }); } Ok(()) } @@ -447,7 +437,7 @@ impl MsdController { let msd_hold = self.msd_function.read().await; let msd = msd_hold .as_ref() - .ok_or_else(|| AppError::Internal("MSD function not initialized".to_string()))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable))?; msd.disconnect_lun_async(&gadget_path, lun).await } @@ -455,9 +445,11 @@ impl MsdController { self.otg_service .set_msd_lun_capacity(previous_state.disk_mode.capacity()) .await?; - let msd_function = self.otg_service.msd_function().await.ok_or_else(|| { - AppError::Internal("MSD function missing after OTG rollback".to_string()) - })?; + let msd_function = self + .otg_service + .msd_function() + .await + .ok_or_else(|| AppError::from(MsdErrorCode::MsdOperationFailed))?; *self.msd_function.write().await = Some(msd_function); for media in &previous_state.mounted_media { self.configure_media(media).await?; @@ -473,7 +465,7 @@ impl MsdController { } pub async fn disconnect(&self) -> Result<()> { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let mut state = self.state.write().await; if state.mounted_media.is_empty() { @@ -488,10 +480,13 @@ impl MsdController { for prior in &disconnected { if let Err(restore_error) = self.configure_media(prior).await { state.available = false; - return Err(AppError::Internal(format!( - "Failed to disconnect LUN {}: {error}; restore failed: {restore_error}", - media.lun - ))); + warn!( + lun = media.lun, + disconnect_error = %error, + %restore_error, + "Failed to restore MSD media after disconnect failure" + ); + return Err(MsdErrorCode::MsdDisconnectFailed.into()); } } return Err(error); @@ -520,16 +515,14 @@ impl MsdController { } pub async fn delete_image(&self, image_id: &str) -> Result<()> { - let _op_guard = self.operation_lock.write().await; + let _op_guard = self.try_operation()?; let state = self.state.read().await; if state .mounted_media .iter() .any(|media| media.kind == MountedMediaKind::Image && media.id == image_id) { - return Err(AppError::BadRequest( - "Cannot delete image while it is mounted".to_string(), - )); + return Err(MsdErrorCode::MsdMediaInUse.into()); } ImageManager::new(self.images_path.clone()).delete(image_id) @@ -540,6 +533,12 @@ impl MsdController { url: String, filename: Option, ) -> Result { + let parsed_url = + reqwest::Url::parse(&url).map_err(|_| AppError::from(MsdErrorCode::MsdInvalidUrl))?; + if !matches!(parsed_url.scheme(), "http" | "https") { + return Err(MsdErrorCode::MsdInvalidUrl.into()); + } + let download_id = uuid::Uuid::new_v4().to_string(); let cancel_token = CancellationToken::new(); @@ -560,7 +559,7 @@ impl MsdController { total_bytes: None, progress_pct: None, status: DownloadStatus::Started, - error: None, + error_code: None, }; self.publish_event(crate::events::SystemEvent::MsdDownloadProgress { @@ -571,6 +570,7 @@ impl MsdController { total_bytes: None, progress_pct: None, status: "started".to_string(), + error_code: None, }) .await; @@ -600,6 +600,7 @@ impl MsdController { total_bytes: total, progress_pct, status: "in_progress".to_string(), + error_code: None, }); } }; @@ -624,11 +625,16 @@ impl MsdController { total_bytes: Some(image_info.size), progress_pct: Some(100.0), status: "completed".to_string(), + error_code: None, }); } } Err(e) => { - warn!("Download failed: {}", e); + warn!(error = %e, "MSD image download failed"); + let code = match e { + AppError::Msd(error) => error.code(), + _ => MsdErrorCode::MsdOperationFailed, + }; if let Some(ref bus) = events { bus.publish(crate::events::SystemEvent::MsdDownloadProgress { download_id: download_id_clone, @@ -637,7 +643,8 @@ impl MsdController { bytes_downloaded: 0, total_bytes: None, progress_pct: None, - status: format!("failed: {}", e), + status: "failed".to_string(), + error_code: Some(code.as_str().to_string()), }); } } @@ -655,10 +662,7 @@ impl MsdController { info!("Download cancelled: {}", download_id); Ok(()) } else { - Err(AppError::NotFound(format!( - "Download not found: {}", - download_id - ))) + Err(MsdErrorCode::MsdResourceNotFound.into()) } } @@ -666,7 +670,13 @@ impl MsdController { self.otg_service .gadget_path() .await - .ok_or_else(|| AppError::Internal("OTG gadget path is not available".to_string())) + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable)) + } + + fn try_operation(&self) -> Result> { + self.operation_lock + .try_write() + .map_err(|_| MsdErrorCode::MsdOperationInProgress.into()) } pub async fn shutdown(&self) -> Result<()> { @@ -712,6 +722,18 @@ mod tests { assert!(controller.drive_path.ends_with("ventoy.img")); } + #[tokio::test] + async fn concurrent_operations_have_a_stable_error_code() { + let temp_dir = TempDir::new().unwrap(); + let controller = MsdController::new(Arc::new(OtgService::new()), temp_dir.path()); + let _guard = controller.operation_lock.write().await; + + assert!(matches!( + controller.try_operation().unwrap_err(), + AppError::Msd(error) if error.code() == MsdErrorCode::MsdOperationInProgress + )); + } + #[tokio::test] async fn test_state_default() { let temp_dir = TempDir::new().unwrap(); @@ -783,14 +805,14 @@ mod tests { .push(MountedMedia::image(3, &image, false, true)); assert_eq!(MsdController::select_lun(&state, Some(5)).unwrap(), 5); - assert!(MsdController::select_lun(&state, Some(3)) - .unwrap_err() - .to_string() - .contains("already occupied")); - assert!(MsdController::select_lun(&state, Some(8)) - .unwrap_err() - .to_string() - .contains("outside")); + assert!(matches!( + MsdController::select_lun(&state, Some(3)).unwrap_err(), + AppError::Msd(error) if error.code() == MsdErrorCode::MsdMediaSlotsFull + )); + assert!(matches!( + MsdController::select_lun(&state, Some(8)).unwrap_err(), + AppError::Msd(error) if error.code() == MsdErrorCode::MsdInvalidRequest + )); } #[test] diff --git a/src/msd/image.rs b/src/msd/image.rs index 630f03d9..37314707 100644 --- a/src/msd/image.rs +++ b/src/msd/image.rs @@ -6,10 +6,10 @@ use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; use time::OffsetDateTime; use tokio::io::AsyncWriteExt; -use tracing::info; +use tracing::{info, warn}; use super::types::ImageInfo; -use crate::error::{AppError, Result}; +use crate::error::{AppError, MsdErrorCode, Result}; const MAX_IMAGE_SIZE: u64 = 32 * 1024 * 1024 * 1024; @@ -28,7 +28,7 @@ impl ImageManager { pub fn ensure_dir(&self) -> Result<()> { fs::create_dir_all(&self.images_path) - .map_err(|e| AppError::Internal(format!("Failed to create images directory: {}", e)))?; + .map_err(|error| storage_io_error("create images directory", error))?; Ok(()) } @@ -38,11 +38,9 @@ impl ImageManager { let mut images = Vec::new(); for entry in fs::read_dir(&self.images_path) - .map_err(|e| AppError::Internal(format!("Failed to read images directory: {}", e)))? + .map_err(|error| storage_io_error("read images directory", error))? { - let entry = entry.map_err(|e| { - AppError::Internal(format!("Failed to read directory entry: {}", e)) - })?; + let entry = entry.map_err(|error| storage_io_error("read image entry", error))?; let path = entry.path(); if path.is_file() { @@ -88,13 +86,13 @@ impl ImageManager { return Ok(image); } } - Err(AppError::NotFound(format!("Image not found: {}", id))) + Err(MsdErrorCode::MsdResourceNotFound.into()) } pub fn get_by_name(&self, name: &str) -> Result { let path = self.images_path.join(name); self.get_image_info(&path) - .ok_or_else(|| AppError::NotFound(format!("Image not found: {}", name))) + .ok_or_else(|| MsdErrorCode::MsdResourceNotFound.into()) } #[cfg(test)] @@ -103,30 +101,24 @@ impl ImageManager { let name = sanitize_filename(name); if name.is_empty() { - return Err(AppError::Internal("Invalid filename".to_string())); + return Err(MsdErrorCode::MsdInvalidRequest.into()); } if data.len() as u64 > MAX_IMAGE_SIZE { - return Err(AppError::Internal(format!( - "Image too large. Maximum size: {} GB", - MAX_IMAGE_SIZE / 1024 / 1024 / 1024 - ))); + return Err(MsdErrorCode::MsdImageTooLarge.into()); } let path = self.images_path.join(&name); if path.exists() { - return Err(AppError::Internal(format!( - "Image already exists: {}", - name - ))); + return Err(MsdErrorCode::MsdResourceAlreadyExists.into()); } - let mut file = fs::File::create(&path) - .map_err(|e| AppError::Internal(format!("Failed to create image file: {}", e)))?; + let mut file = + fs::File::create(&path).map_err(|error| storage_io_error("create image", error))?; - file.write_all(data).map_err(|e| { + file.write_all(data).map_err(|error| { let _ = fs::remove_file(&path); - AppError::Internal(format!("Failed to write image data: {}", e)) + storage_io_error("write image", error) })?; info!("Created image: {} ({} bytes)", name, data.len()); @@ -143,7 +135,7 @@ impl ImageManager { let name = sanitize_filename(name); if name.is_empty() { - return Err(AppError::Internal("Invalid filename".to_string())); + return Err(MsdErrorCode::MsdInvalidRequest.into()); } let temp_name = format!(".upload_{}", uuid::Uuid::new_v4()); @@ -151,48 +143,41 @@ impl ImageManager { let final_path = self.images_path.join(&name); if final_path.exists() { - return Err(AppError::Internal(format!( - "Image already exists: {}", - name - ))); + return Err(MsdErrorCode::MsdResourceAlreadyExists.into()); } let mut file = tokio::fs::File::create(&temp_path) .await - .map_err(|e| AppError::Internal(format!("Failed to create temp file: {}", e)))?; + .map_err(|error| storage_io_error("create image upload", error))?; let mut bytes_written: u64 = 0; - while let Some(chunk) = field - .chunk() - .await - .map_err(|e| AppError::Internal(format!("Failed to read upload chunk: {}", e)))? - { + while let Some(chunk) = field.chunk().await.map_err(|error| { + warn!(%error, "Failed to read MSD image upload chunk"); + AppError::from(MsdErrorCode::MsdOperationFailed) + })? { bytes_written += chunk.len() as u64; if bytes_written > MAX_IMAGE_SIZE { drop(file); let _ = tokio::fs::remove_file(&temp_path).await; - return Err(AppError::Internal(format!( - "Image too large. Maximum size: {} GB", - MAX_IMAGE_SIZE / 1024 / 1024 / 1024 - ))); + return Err(MsdErrorCode::MsdImageTooLarge.into()); } file.write_all(&chunk) .await - .map_err(|e| AppError::Internal(format!("Failed to write chunk: {}", e)))?; + .map_err(|error| storage_io_error("write image upload", error))?; } file.flush() .await - .map_err(|e| AppError::Internal(format!("Failed to flush file: {}", e)))?; + .map_err(|error| storage_io_error("flush image upload", error))?; drop(file); tokio::fs::rename(&temp_path, &final_path) .await - .map_err(|e| { + .map_err(|error| { let _ = std::fs::remove_file(&temp_path); - AppError::Internal(format!("Failed to rename temp file: {}", e)) + storage_io_error("commit image upload", error) })?; info!( @@ -206,8 +191,7 @@ impl ImageManager { pub fn delete(&self, id: &str) -> Result<()> { let image = self.get(id)?; - fs::remove_file(&image.path) - .map_err(|e| AppError::Internal(format!("Failed to delete image: {}", e)))?; + fs::remove_file(&image.path).map_err(|error| storage_io_error("delete image", error))?; info!("Deleted image: {}", image.name); Ok(()) @@ -224,8 +208,11 @@ impl ImageManager { { self.ensure_dir()?; - let parsed_url = reqwest::Url::parse(url) - .map_err(|e| AppError::BadRequest(format!("Invalid URL: {}", e)))?; + let parsed_url = + reqwest::Url::parse(url).map_err(|_| AppError::from(MsdErrorCode::MsdInvalidUrl))?; + if !matches!(parsed_url.scheme(), "http" | "https") { + return Err(MsdErrorCode::MsdInvalidUrl.into()); + } info!("Starting download from: {}", url); @@ -233,19 +220,17 @@ impl ImageManager { .timeout(std::time::Duration::from_secs(3600)) .connect_timeout(std::time::Duration::from_secs(30)) .build() - .map_err(|e| AppError::Internal(format!("Failed to create HTTP client: {}", e)))?; + .map_err(|error| remote_download_error("create HTTP client", error))?; let head_response = client .head(url) .send() .await - .map_err(|e| AppError::Internal(format!("Failed to connect: {}", e)))?; + .map_err(|error| remote_download_error("send HEAD request", error))?; if !head_response.status().is_success() { - return Err(AppError::Internal(format!( - "Server returned error: {}", - head_response.status() - ))); + warn!(status = %head_response.status(), "MSD image HEAD request failed"); + return Err(MsdErrorCode::MsdRemoteDownloadFailed.into()); } let total_size = head_response @@ -256,11 +241,7 @@ impl ImageManager { if let Some(size) = total_size { if size > MAX_IMAGE_SIZE { - return Err(AppError::BadRequest(format!( - "File too large: {} bytes (max {} GB)", - size, - MAX_IMAGE_SIZE / 1024 / 1024 / 1024 - ))); + return Err(MsdErrorCode::MsdImageTooLarge.into()); } } @@ -284,17 +265,12 @@ impl ImageManager { }; if final_filename.is_empty() { - return Err(AppError::BadRequest( - "Could not determine filename".to_string(), - )); + return Err(MsdErrorCode::MsdInvalidRequest.into()); } let final_path = self.images_path.join(&final_filename); if final_path.exists() { - return Err(AppError::BadRequest(format!( - "Image already exists: {}", - final_filename - ))); + return Err(MsdErrorCode::MsdResourceAlreadyExists.into()); } let temp_filename = format!(".download_{}", uuid::Uuid::new_v4()); @@ -304,13 +280,11 @@ impl ImageManager { .get(url) .send() .await - .map_err(|e| AppError::Internal(format!("Download failed: {}", e)))?; + .map_err(|error| remote_download_error("send GET request", error))?; if !response.status().is_success() { - return Err(AppError::Internal(format!( - "Download failed: HTTP {}", - response.status() - ))); + warn!(status = %response.status(), "MSD image GET request failed"); + return Err(MsdErrorCode::MsdRemoteDownloadFailed.into()); } let content_length = response @@ -322,7 +296,7 @@ impl ImageManager { let mut file = tokio::fs::File::create(&temp_path) .await - .map_err(|e| AppError::Internal(format!("Failed to create temp file: {}", e)))?; + .map_err(|error| storage_io_error("create image download", error))?; let mut stream = response.bytes_stream(); let mut downloaded: u64 = 0; @@ -334,11 +308,11 @@ impl ImageManager { while let Some(chunk_result) = stream.next().await { let chunk = - chunk_result.map_err(|e| AppError::Internal(format!("Download error: {}", e)))?; + chunk_result.map_err(|error| remote_download_error("read response body", error))?; - file.write_all(&chunk).await.map_err(|e| { + file.write_all(&chunk).await.map_err(|error| { let _ = std::fs::remove_file(&temp_path); - AppError::Internal(format!("Failed to write data: {}", e)) + storage_io_error("write image download", error) })?; downloaded += chunk.len() as u64; @@ -360,29 +334,29 @@ impl ImageManager { file.flush() .await - .map_err(|e| AppError::Internal(format!("Failed to flush file: {}", e)))?; + .map_err(|error| storage_io_error("flush image download", error))?; drop(file); let metadata = tokio::fs::metadata(&temp_path) .await - .map_err(|e| AppError::Internal(format!("Failed to read file metadata: {}", e)))?; + .map_err(|error| storage_io_error("read downloaded image metadata", error))?; if let Some(expected) = content_length { if metadata.len() != expected { let _ = tokio::fs::remove_file(&temp_path).await; - return Err(AppError::Internal(format!( - "Download incomplete: got {} bytes, expected {}", - metadata.len(), - expected - ))); + warn!( + actual = metadata.len(), + expected, "MSD image download was incomplete" + ); + return Err(MsdErrorCode::MsdDownloadIncomplete.into()); } } tokio::fs::rename(&temp_path, &final_path) .await - .map_err(|e| { + .map_err(|error| { let _ = std::fs::remove_file(&temp_path); - AppError::Internal(format!("Failed to move file: {}", e)) + storage_io_error("commit image download", error) })?; info!( @@ -395,6 +369,26 @@ impl ImageManager { } } +fn storage_io_error(operation: &'static str, error: std::io::Error) -> AppError { + warn!(operation, %error, "MSD storage operation failed"); + #[cfg(unix)] + let code = match error.raw_os_error() { + Some(libc::EFBIG) => MsdErrorCode::MsdImageTooLarge, + Some(libc::ENOSPC) => MsdErrorCode::MsdStorageFull, + Some(libc::EROFS) => MsdErrorCode::MsdStorageReadOnly, + Some(libc::EACCES | libc::EPERM) => MsdErrorCode::MsdStoragePermissionDenied, + _ => MsdErrorCode::MsdOperationFailed, + }; + #[cfg(not(unix))] + let code = MsdErrorCode::MsdOperationFailed; + code.into() +} + +fn remote_download_error(operation: &'static str, error: reqwest::Error) -> AppError { + warn!(operation, %error, "MSD remote download failed"); + MsdErrorCode::MsdRemoteDownloadFailed.into() +} + fn stable_image_id_from_filename(name: &str) -> String { let mut hash: u64 = 0; for (i, byte) in name.bytes().enumerate() { @@ -490,4 +484,18 @@ mod tests { assert!(manager.list().unwrap().is_empty()); } + + #[test] + fn classifies_storage_io_errors() { + for (errno, expected) in [ + (libc::EFBIG, MsdErrorCode::MsdImageTooLarge), + (libc::ENOSPC, MsdErrorCode::MsdStorageFull), + (libc::EROFS, MsdErrorCode::MsdStorageReadOnly), + (libc::EACCES, MsdErrorCode::MsdStoragePermissionDenied), + (libc::EPERM, MsdErrorCode::MsdStoragePermissionDenied), + ] { + let error = storage_io_error("test", std::io::Error::from_raw_os_error(errno)); + assert!(matches!(error, AppError::Msd(error) if error.code() == expected)); + } + } } diff --git a/src/msd/mod.rs b/src/msd/mod.rs index fc21f851..35fbf71a 100644 --- a/src/msd/mod.rs +++ b/src/msd/mod.rs @@ -14,4 +14,5 @@ pub use types::{ }; pub use ventoy_drive::{VentoyDrive, MIN_DRIVE_SIZE_MB}; +pub use crate::error::{MsdError, MsdErrorCode}; pub use crate::otg::{MsdFunction, MsdLunConfig}; diff --git a/src/msd/types.rs b/src/msd/types.rs index e7d6be80..f8a001e8 100644 --- a/src/msd/types.rs +++ b/src/msd/types.rs @@ -235,7 +235,7 @@ pub struct DownloadProgress { pub total_bytes: Option, pub progress_pct: Option, pub status: DownloadStatus, - pub error: Option, + pub error_code: Option, } #[cfg(test)] diff --git a/src/msd/ventoy_drive.rs b/src/msd/ventoy_drive.rs index 0a9dee2a..edeb4faf 100644 --- a/src/msd/ventoy_drive.rs +++ b/src/msd/ventoy_drive.rs @@ -1,12 +1,12 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use tokio::sync::RwLock; -use tracing::info; +use tracing::{info, warn}; use ventoy_img::{FileInfo as VentoyFileInfo, VentoyError, VentoyImage}; use super::types::{DriveFile, DriveInfo}; -use crate::error::{AppError, Result}; +use crate::error::{AppError, MsdErrorCode, Result}; const STREAM_CHUNK_SIZE: usize = 64 * 1024; @@ -44,10 +44,7 @@ impl VentoyDrive { pub async fn init(&self, size_mb: u32) -> Result { if size_mb < MIN_DRIVE_SIZE_MB { - return Err(AppError::BadRequest(format!( - "Drive size must be at least {} MB", - MIN_DRIVE_SIZE_MB - ))); + return Err(MsdErrorCode::MsdDriveSizeInvalid.into()); } let size_str = format!("{}M", size_mb); let path = self.path.clone(); @@ -59,7 +56,7 @@ impl VentoyDrive { VentoyImage::create(&path, &size_str, DEFAULT_LABEL).map_err(drive_init_error)?; let metadata = std::fs::metadata(&path) - .map_err(|e| AppError::Internal(format!("Failed to read drive metadata: {}", e)))?; + .map_err(|error| drive_io_error("read initialized drive metadata", error))?; Ok::(DriveInfo { size: metadata.len(), @@ -70,7 +67,7 @@ impl VentoyDrive { }) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))??; + .map_err(|error| task_error("initialize virtual drive", error))??; info!("Ventoy drive created successfully"); Ok(info) @@ -78,7 +75,7 @@ impl VentoyDrive { pub async fn info(&self) -> Result { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -86,7 +83,7 @@ impl VentoyDrive { tokio::task::spawn_blocking(move || { let metadata = std::fs::metadata(&path) - .map_err(|e| AppError::Internal(format!("Failed to read drive metadata: {}", e)))?; + .map_err(|error| drive_io_error("read drive metadata", error))?; let image = VentoyImage::open(&path).map_err(ventoy_to_app_error)?; @@ -110,12 +107,12 @@ impl VentoyDrive { }) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))? + .map_err(|error| task_error("read virtual drive info", error))? } pub async fn list_files(&self, dir_path: &str) -> Result> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -138,7 +135,7 @@ impl VentoyDrive { .collect()) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))? + .map_err(|error| task_error("list virtual drive files", error))? } pub async fn write_file_from_multipart_field( @@ -147,7 +144,7 @@ impl VentoyDrive { mut field: axum::extract::multipart::Field<'_>, ) -> Result { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let temp_dir = self.path.parent().unwrap_or(Path::new("/tmp")); @@ -156,24 +153,23 @@ impl VentoyDrive { let mut temp_file = tokio::fs::File::create(&temp_path) .await - .map_err(|e| AppError::Internal(format!("Failed to create temp file: {}", e)))?; + .map_err(|error| drive_io_error("create virtual drive upload", error))?; let mut bytes_written: u64 = 0; - while let Some(chunk) = field - .chunk() - .await - .map_err(|e| AppError::Internal(format!("Failed to read upload chunk: {}", e)))? - { + while let Some(chunk) = field.chunk().await.map_err(|error| { + warn!(%error, "Failed to read virtual drive upload chunk"); + AppError::from(MsdErrorCode::MsdOperationFailed) + })? { bytes_written += chunk.len() as u64; tokio::io::AsyncWriteExt::write_all(&mut temp_file, &chunk) .await - .map_err(|e| AppError::Internal(format!("Failed to write chunk: {}", e)))?; + .map_err(|error| drive_io_error("write virtual drive upload", error))?; } tokio::io::AsyncWriteExt::flush(&mut temp_file) .await - .map_err(|e| AppError::Internal(format!("Failed to flush temp file: {}", e)))?; + .map_err(|error| drive_io_error("flush virtual drive upload", error))?; drop(temp_file); let path = self.path.clone(); @@ -191,7 +187,7 @@ impl VentoyDrive { Ok::<(), AppError>(()) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))?; + .map_err(|error| task_error("write virtual drive file", error))?; let _ = tokio::fs::remove_file(&temp_path).await; @@ -202,7 +198,7 @@ impl VentoyDrive { #[cfg(test)] pub async fn read_file(&self, file_path: &str) -> Result> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -215,12 +211,12 @@ impl VentoyDrive { image.read_file(&file_path).map_err(ventoy_to_app_error) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))? + .map_err(|error| task_error("read virtual drive file", error))? } pub async fn get_file_info(&self, file_path: &str) -> Result> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -234,7 +230,7 @@ impl VentoyDrive { .map_err(ventoy_to_app_error) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))??; + .map_err(|error| task_error("read virtual drive file information", error))??; Ok(info.map(|f| DriveFile { name: f.name, @@ -253,19 +249,16 @@ impl VentoyDrive { tokio::sync::mpsc::Receiver>, )> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let file_info = self .get_file_info(file_path) .await? - .ok_or_else(|| AppError::NotFound(format!("File not found: {}", file_path)))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdResourceNotFound))?; if file_info.is_dir { - return Err(AppError::BadRequest(format!( - "'{}' is a directory", - file_path - ))); + return Err(MsdErrorCode::MsdInvalidRequest.into()); } let file_size = file_info.size; @@ -300,7 +293,7 @@ impl VentoyDrive { pub async fn mkdir(&self, dir_path: &str) -> Result<()> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -315,12 +308,12 @@ impl VentoyDrive { .map_err(ventoy_to_app_error) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))? + .map_err(|error| task_error("create virtual drive directory", error))? } pub async fn delete(&self, path_to_delete: &str) -> Result<()> { if !self.exists() { - return Err(AppError::Internal("Drive not initialized".to_string())); + return Err(MsdErrorCode::MsdDriveNotInitialized.into()); } let path = self.path.clone(); @@ -335,22 +328,23 @@ impl VentoyDrive { .map_err(ventoy_to_app_error) }) .await - .map_err(|e| AppError::Internal(format!("Task join error: {}", e)))? + .map_err(|error| task_error("delete virtual drive resource", error))? } } fn ventoy_to_app_error(err: VentoyError) -> AppError { + warn!(%err, "Virtual drive filesystem operation failed"); match err { - VentoyError::Io(e) => AppError::Io(e), - VentoyError::InvalidSize(s) => AppError::BadRequest(format!("Invalid size: {}", s)), - VentoyError::SizeParseError(s) => AppError::BadRequest(format!("Size parse error: {}", s)), - VentoyError::FilesystemError(s) => AppError::Internal(format!("Filesystem error: {}", s)), - VentoyError::ImageError(s) => AppError::Internal(format!("Image error: {}", s)), - VentoyError::FileNotFound(s) => AppError::NotFound(format!("File not found: {}", s)), - VentoyError::ResourceNotFound(s) => { - AppError::Internal(format!("Resource not found: {}", s)) + VentoyError::Io(error) => drive_io_error("access virtual drive", error), + VentoyError::InvalidSize(_) | VentoyError::SizeParseError(_) => { + MsdErrorCode::MsdDriveSizeInvalid.into() + } + VentoyError::FilesystemError(_) + | VentoyError::ImageError(_) + | VentoyError::PartitionError(_) => MsdErrorCode::MsdDriveFilesystemUnsupported.into(), + VentoyError::FileNotFound(_) | VentoyError::ResourceNotFound(_) => { + MsdErrorCode::MsdResourceNotFound.into() } - VentoyError::PartitionError(s) => AppError::Internal(format!("Partition error: {}", s)), } } @@ -361,21 +355,35 @@ fn drive_init_error(err: VentoyError) -> AppError { #[cfg(unix)] match error.raw_os_error() { - Some(libc::EFBIG) => AppError::BadRequest( - "MSD directory filesystem does not support a virtual drive file of this size".into(), - ), - Some(libc::ENOSPC) => AppError::BadRequest( - "MSD directory does not have enough free space for the virtual drive".into(), - ), - Some(libc::EROFS) => AppError::BadRequest("MSD directory filesystem is read-only".into()), - Some(libc::EACCES | libc::EPERM) => AppError::BadRequest( - "One-KVM does not have permission to write to the MSD directory".into(), - ), - _ => AppError::Io(error), + Some(libc::EFBIG) => MsdErrorCode::MsdDriveSizeInvalid.into(), + Some(libc::ENOSPC) => MsdErrorCode::MsdStorageFull.into(), + Some(libc::EROFS) => MsdErrorCode::MsdStorageReadOnly.into(), + Some(libc::EACCES | libc::EPERM) => MsdErrorCode::MsdStoragePermissionDenied.into(), + _ => drive_io_error("initialize virtual drive", error), } #[cfg(not(unix))] - AppError::Io(error) + drive_io_error("initialize virtual drive", error) +} + +fn drive_io_error(operation: &'static str, error: std::io::Error) -> AppError { + warn!(operation, %error, "Virtual drive storage operation failed"); + #[cfg(unix)] + let code = match error.raw_os_error() { + Some(libc::EFBIG) => MsdErrorCode::MsdImageTooLarge, + Some(libc::ENOSPC) => MsdErrorCode::MsdStorageFull, + Some(libc::EROFS) => MsdErrorCode::MsdStorageReadOnly, + Some(libc::EACCES | libc::EPERM) => MsdErrorCode::MsdStoragePermissionDenied, + _ => MsdErrorCode::MsdOperationFailed, + }; + #[cfg(not(unix))] + let code = MsdErrorCode::MsdOperationFailed; + code.into() +} + +fn task_error(operation: &'static str, error: tokio::task::JoinError) -> AppError { + warn!(operation, %error, "Virtual drive task failed"); + MsdErrorCode::MsdOperationFailed.into() } fn ventoy_file_to_drive_file(info: VentoyFileInfo, parent_path: &str) -> DriveFile { @@ -470,16 +478,35 @@ mod tests { #[test] fn classifies_drive_creation_io_errors() { for (errno, expected) in [ - (libc::EFBIG, "does not support"), - (libc::ENOSPC, "enough free space"), - (libc::EROFS, "read-only"), - (libc::EACCES, "permission"), + (libc::EFBIG, MsdErrorCode::MsdDriveSizeInvalid), + (libc::ENOSPC, MsdErrorCode::MsdStorageFull), + (libc::EROFS, MsdErrorCode::MsdStorageReadOnly), + (libc::EACCES, MsdErrorCode::MsdStoragePermissionDenied), + (libc::EPERM, MsdErrorCode::MsdStoragePermissionDenied), ] { let error = drive_init_error(VentoyError::Io(std::io::Error::from_raw_os_error(errno))); - assert!(matches!(error, AppError::BadRequest(message) if message.contains(expected))); + assert!(matches!(error, AppError::Msd(error) if error.code() == expected)); } } + #[test] + fn classifies_ventoy_filesystem_and_resource_errors() { + for error in [ + VentoyError::FilesystemError("details".into()), + VentoyError::ImageError("details".into()), + VentoyError::PartitionError("details".into()), + ] { + assert!(matches!( + ventoy_to_app_error(error), + AppError::Msd(error) if error.code() == MsdErrorCode::MsdDriveFilesystemUnsupported + )); + } + assert!(matches!( + ventoy_to_app_error(VentoyError::FileNotFound("details".into())), + AppError::Msd(error) if error.code() == MsdErrorCode::MsdResourceNotFound + )); + } + fn init_ventoy_resources() -> bool { static INIT: OnceLock = OnceLock::new(); *INIT.get_or_init(|| { diff --git a/src/otg/msd.rs b/src/otg/msd.rs index a1256f50..2dbc7f5d 100644 --- a/src/otg/msd.rs +++ b/src/otg/msd.rs @@ -1,10 +1,13 @@ -use std::fs; +use std::fs::{self, OpenOptions}; +use std::io::Write; use std::path::{Path, PathBuf}; use tracing::{debug, info, warn}; use super::configfs::{create_dir, create_symlink, remove_dir, remove_file, write_file}; use super::function::GadgetFunction; -use crate::error::{AppError, Result}; +use crate::error::{AppError, MsdErrorCode, Result}; + +const MEDIA_TYPE_REBIND_DELAY_MS: u64 = 300; #[derive(Debug, Clone)] pub struct MsdLunConfig { @@ -150,6 +153,71 @@ impl MsdFunction { ))); } + let current_cdrom = fs::read_to_string(lun_path.join("cdrom")) + .unwrap_or_default() + .trim() + .to_string(); + let rebind_required = Self::media_type_rebind_required(¤t_cdrom, config); + let udc_path = gadget_path.join("UDC"); + let bound_udc = if rebind_required && udc_path.exists() { + fs::read_to_string(&udc_path) + .map_err(|error| { + AppError::Internal(format!( + "Failed to read bound UDC before changing LUN {lun} media type: {error}" + )) + })? + .trim() + .to_string() + } else { + String::new() + }; + + if !bound_udc.is_empty() { + info!( + "LUN {} media type is changing; temporarily unbinding UDC {}", + lun, bound_udc + ); + write_file(&udc_path, "")?; + std::thread::sleep(std::time::Duration::from_millis(MEDIA_TYPE_REBIND_DELAY_MS)); + } + + let configure_result = self.configure_lun_attributes(&lun_path, lun, config); + let rebind_result = if bound_udc.is_empty() { + Ok(()) + } else { + let result = write_file(&udc_path, &bound_udc); + if result.is_ok() { + std::thread::sleep(std::time::Duration::from_millis(MEDIA_TYPE_REBIND_DELAY_MS)); + info!( + "Rebound UDC {} after changing LUN {} media type", + bound_udc, lun + ); + } + result + }; + + match (configure_result, rebind_result) { + (Ok(()), Ok(())) => Ok(()), + (Err(configure_error), Ok(())) => Err(configure_error), + (Ok(()), Err(rebind_error)) => Err(AppError::Internal(format!( + "Configured LUN {lun}, but failed to rebind UDC {bound_udc}: {rebind_error}" + ))), + (Err(configure_error), Err(rebind_error)) => Err(AppError::Internal(format!( + "Failed to configure LUN {lun}: {configure_error}; also failed to rebind UDC {bound_udc}: {rebind_error}" + ))), + } + } + + fn media_type_rebind_required(current_cdrom: &str, config: &MsdLunConfig) -> bool { + current_cdrom != if config.cdrom { "1" } else { "0" } + } + + fn configure_lun_attributes( + &self, + lun_path: &Path, + lun: u8, + config: &MsdLunConfig, + ) -> Result<()> { let read_attr = |attr: &str| -> String { fs::read_to_string(lun_path.join(attr)) .unwrap_or_default() @@ -161,7 +229,6 @@ impl MsdFunction { let current_ro = read_attr("ro"); let current_removable = read_attr("removable"); let current_nofua = read_attr("nofua"); - let new_cdrom = if config.cdrom { "1" } else { "0" }; let new_ro = if config.ro { "1" } else { "0" }; let new_removable = if config.removable { "1" } else { "0" }; @@ -170,15 +237,20 @@ impl MsdFunction { let forced_eject_path = lun_path.join("forced_eject"); if forced_eject_path.exists() { debug!("Using forced_eject to clear LUN {}", lun); - let _ = write_file(&forced_eject_path, "1"); + if let Err(error) = write_file(&forced_eject_path, "1") { + warn!( + "LUN {} forced_eject failed while changing media: {}; clearing file instead", + lun, error + ); + write_file(&lun_path.join("file"), "")?; + } } else { - let _ = write_file(&lun_path.join("file"), ""); + write_file(&lun_path.join("file"), "")?; } std::thread::sleep(std::time::Duration::from_millis(50)); - let cdrom_changed = current_cdrom != new_cdrom; - if cdrom_changed { + if current_cdrom != new_cdrom { debug!( "Updating LUN {} cdrom: {} -> {}", lun, current_cdrom, new_cdrom @@ -204,11 +276,6 @@ impl MsdFunction { write_file(&lun_path.join("nofua"), new_nofua)?; } - if cdrom_changed { - debug!("CDROM mode changed, brief yield for USB host"); - std::thread::sleep(std::time::Duration::from_millis(50)); - } - if config.file.exists() { let file_path = config.file.to_string_lossy(); let mut last_error = None; @@ -225,10 +292,9 @@ impl MsdFunction { ); return Ok(()); } - Err(e) => { - let is_busy = e.to_string().contains("Device or resource busy") - || e.to_string().contains("os error 16"); - + Err(error) => { + let is_busy = error.to_string().contains("Device or resource busy") + || error.to_string().contains("os error 16"); if is_busy && attempt < 4 { warn!( "LUN {} file write busy, retrying (attempt {}/5)", @@ -236,17 +302,16 @@ impl MsdFunction { attempt + 1 ); std::thread::sleep(std::time::Duration::from_millis(50 << attempt)); - last_error = Some(e); + last_error = Some(error); continue; } - - return Err(e); + return Err(error); } } } - if let Some(e) = last_error { - return Err(e); + if let Some(error) = last_error { + return Err(error); } } else if !config.file.as_os_str().is_empty() { warn!("LUN {} file does not exist: {}", lun, config.file.display()); @@ -276,6 +341,52 @@ impl MsdFunction { self.disconnect_lun_path(&lun_path, lun as u16) } + fn medium_removal_was_prevented(error: &std::io::Error) -> bool { + error.raw_os_error() == Some(libc::EBUSY) + } + + fn clear_lun_file(file_path: &Path, lun: u16) -> Result<()> { + let mut file = OpenOptions::new() + .write(true) + .open(file_path) + .map_err(|error| { + warn!( + lun, + path = %file_path.display(), + %error, + "Failed to open MSD LUN backing-file attribute while disconnecting" + ); + AppError::from(MsdErrorCode::MsdDisconnectFailed) + })?; + + // An empty configfs value is represented by a newline. Keep this as one + // write operation so EBUSY can be attributed to fsg_store_file(). + if let Err(error) = file.write_all(b"\n") { + warn!( + lun, + path = %file_path.display(), + errno = error.raw_os_error(), + %error, + "Kernel rejected MSD LUN disconnect" + ); + return if Self::medium_removal_was_prevented(&error) { + Err(MsdErrorCode::MsdMediumRemovalPrevented.into()) + } else { + Err(MsdErrorCode::MsdDisconnectFailed.into()) + }; + } + + file.flush().map_err(|error| { + warn!( + lun, + path = %file_path.display(), + %error, + "Failed to flush MSD LUN backing-file attribute while disconnecting" + ); + MsdErrorCode::MsdDisconnectFailed.into() + }) + } + fn disconnect_lun_path(&self, lun_path: &Path, lun: u16) -> Result<()> { if lun_path.exists() { let forced_eject_path = lun_path.join("forced_eject"); @@ -293,14 +404,14 @@ impl MsdFunction { ); let file_path = lun_path.join("file"); if file_path.exists() { - write_file(&file_path, "")?; + Self::clear_lun_file(&file_path, lun)?; } } } } else { let file_path = lun_path.join("file"); if file_path.exists() { - write_file(&file_path, "")?; + Self::clear_lun_file(&file_path, lun)?; } } info!("LUN {} disconnected", lun); @@ -448,6 +559,98 @@ mod tests { assert!(MsdFunction::new(0, 9).is_err()); } + #[test] + fn only_ebusy_means_the_host_prevented_medium_removal() { + let busy = std::io::Error::from_raw_os_error(libc::EBUSY); + let io = std::io::Error::from_raw_os_error(libc::EIO); + + assert!(MsdFunction::medium_removal_was_prevented(&busy)); + assert!(!MsdFunction::medium_removal_was_prevented(&io)); + } + + #[test] + fn disconnect_lun_prefers_forced_eject() { + let temp_dir = TempDir::new().unwrap(); + let lun_path = temp_dir.path().join("functions/mass_storage.usb0/lun.0"); + std::fs::create_dir_all(&lun_path).unwrap(); + std::fs::write(lun_path.join("file"), b"backing.img\n").unwrap(); + std::fs::write(lun_path.join("forced_eject"), b"0\n").unwrap(); + let msd = MsdFunction::new(0, 1).unwrap(); + + msd.disconnect_lun(temp_dir.path(), 0).unwrap(); + + assert_eq!( + std::fs::read(lun_path.join("forced_eject")).unwrap(), + b"1\n" + ); + assert_eq!( + std::fs::read(lun_path.join("file")).unwrap(), + b"backing.img\n" + ); + } + + #[test] + fn disconnect_lun_without_forced_eject_clears_file() { + let temp_dir = TempDir::new().unwrap(); + let lun_path = temp_dir.path().join("functions/mass_storage.usb0/lun.0"); + std::fs::create_dir_all(&lun_path).unwrap(); + std::fs::write(lun_path.join("file"), b"backing.img\n").unwrap(); + let msd = MsdFunction::new(0, 1).unwrap(); + + msd.disconnect_lun(temp_dir.path(), 0).unwrap(); + + assert!(std::fs::read(lun_path.join("file")) + .unwrap() + .starts_with(b"\n")); + } + + #[test] + fn disconnect_lun_falls_back_when_forced_eject_write_fails() { + let temp_dir = TempDir::new().unwrap(); + let lun_path = temp_dir.path().join("functions/mass_storage.usb0/lun.0"); + std::fs::create_dir_all(lun_path.join("forced_eject")).unwrap(); + std::fs::write(lun_path.join("file"), b"backing.img\n").unwrap(); + let msd = MsdFunction::new(0, 1).unwrap(); + + msd.disconnect_lun(temp_dir.path(), 0).unwrap(); + + assert!(std::fs::read(lun_path.join("file")) + .unwrap() + .starts_with(b"\n")); + } + + #[test] + fn disconnect_lun_only_changes_the_selected_lun() { + let temp_dir = TempDir::new().unwrap(); + let function_path = temp_dir.path().join("functions/mass_storage.usb0"); + for lun in 0..2 { + let lun_path = function_path.join(format!("lun.{lun}")); + std::fs::create_dir_all(&lun_path).unwrap(); + std::fs::write(lun_path.join("file"), format!("backing-{lun}.img\n")).unwrap(); + std::fs::write(lun_path.join("forced_eject"), b"0\n").unwrap(); + } + let msd = MsdFunction::new(0, 8).unwrap(); + + msd.disconnect_lun(temp_dir.path(), 1).unwrap(); + + assert_eq!( + std::fs::read(function_path.join("lun.0/forced_eject")).unwrap(), + b"0\n" + ); + assert_eq!( + std::fs::read(function_path.join("lun.1/forced_eject")).unwrap(), + b"1\n" + ); + assert_eq!( + std::fs::read(function_path.join("lun.0/file")).unwrap(), + b"backing-0.img\n" + ); + assert_eq!( + std::fs::read(function_path.join("lun.1/file")).unwrap(), + b"backing-1.img\n" + ); + } + #[test] fn create_uses_configured_lun_capacity() { for capacity in [1, 8] { @@ -486,6 +689,81 @@ mod tests { ); } + #[test] + fn media_type_changes_require_udc_rebind() { + let iso = MsdLunConfig::cdrom(PathBuf::from("/tmp/test.iso")); + let disk = MsdLunConfig::disk(PathBuf::from("/tmp/test.img"), false); + + assert!(MsdFunction::media_type_rebind_required("0", &iso)); + assert!(!MsdFunction::media_type_rebind_required("1", &iso)); + assert!(MsdFunction::media_type_rebind_required("1", &disk)); + assert!(!MsdFunction::media_type_rebind_required("0", &disk)); + } + + #[test] + fn configure_cdrom_restores_bound_udc() { + let temp_dir = TempDir::new().unwrap(); + let lun_path = temp_dir.path().join("functions/mass_storage.usb0/lun.0"); + std::fs::create_dir_all(&lun_path).unwrap(); + for attr in ["file", "cdrom", "ro", "removable", "nofua"] { + std::fs::write(lun_path.join(attr), b"0\n").unwrap(); + } + std::fs::write(temp_dir.path().join("UDC"), b"test.udc\n").unwrap(); + let image_path = temp_dir.path().join("test.iso"); + std::fs::write(&image_path, b"iso").unwrap(); + let msd = MsdFunction::new(0, 1).unwrap(); + + msd.configure_lun(temp_dir.path(), 0, &MsdLunConfig::cdrom(image_path.clone())) + .unwrap(); + + assert_eq!( + std::fs::read_to_string(temp_dir.path().join("UDC")) + .unwrap() + .trim(), + "test.udc" + ); + assert_eq!( + std::fs::read_to_string(lun_path.join("cdrom")) + .unwrap() + .trim(), + "1" + ); + assert_eq!( + std::fs::read_to_string(lun_path.join("ro")).unwrap().trim(), + "1" + ); + assert_eq!( + std::fs::read_to_string(lun_path.join("file")) + .unwrap() + .trim(), + image_path.to_string_lossy() + ); + } + + #[test] + fn configure_failure_still_restores_bound_udc() { + let temp_dir = TempDir::new().unwrap(); + let lun_path = temp_dir.path().join("functions/mass_storage.usb0/lun.0"); + std::fs::create_dir_all(lun_path.join("file")).unwrap(); + for attr in ["cdrom", "ro", "removable", "nofua"] { + std::fs::write(lun_path.join(attr), b"0\n").unwrap(); + } + std::fs::write(temp_dir.path().join("UDC"), b"test.udc\n").unwrap(); + let image_path = temp_dir.path().join("test.iso"); + std::fs::write(&image_path, b"iso").unwrap(); + let msd = MsdFunction::new(0, 1).unwrap(); + + assert!(msd + .configure_lun(temp_dir.path(), 0, &MsdLunConfig::cdrom(image_path),) + .is_err()); + assert_eq!( + std::fs::read_to_string(temp_dir.path().join("UDC")) + .unwrap() + .trim(), + "test.udc" + ); + } + #[test] fn cleanup_removes_all_dynamic_luns_including_stale_capacity() { let temp_dir = TempDir::new().unwrap(); @@ -500,6 +778,30 @@ mod tests { assert!(!func_path.exists()); } + #[test] + fn cleanup_forced_ejects_every_existing_lun() { + let temp_dir = TempDir::new().unwrap(); + let func_path = temp_dir.path().join("functions/mass_storage.usb0"); + for lun in 0..3 { + let lun_path = func_path.join(format!("lun.{lun}")); + std::fs::create_dir_all(&lun_path).unwrap(); + std::fs::write(lun_path.join("file"), format!("backing-{lun}.img\n")).unwrap(); + std::fs::write(lun_path.join("forced_eject"), b"0\n").unwrap(); + } + let msd = MsdFunction::new(0, 1).unwrap(); + + // Ordinary files do not disappear with configfs groups, so cleanup is + // expected to report directory-removal failures in this test fixture. + assert!(msd.cleanup(temp_dir.path()).is_err()); + + for lun in 0..3 { + assert_eq!( + std::fs::read(func_path.join(format!("lun.{lun}/forced_eject"))).unwrap(), + b"1\n" + ); + } + } + #[test] fn cleanup_reports_when_non_configfs_cannot_release_default_lun() { let temp_dir = TempDir::new().unwrap(); diff --git a/src/otg/service.rs b/src/otg/service.rs index f520c1cc..64bc480c 100644 --- a/src/otg/service.rs +++ b/src/otg/service.rs @@ -6,7 +6,7 @@ use typeshare::typeshare; use super::bridge::NetworkBridgeRuntime; use super::manager::{wait_for_hid_devices, GadgetDescriptor, OtgGadgetManager}; -use super::msd::MsdFunction; +use super::msd::{MsdFunction, MsdLunConfig}; use crate::config::{ HidBackend, HidConfig, MsdConfig, OtgDescriptorConfig, OtgHidFunctions, OtgNetworkConfig, UacConfig, @@ -214,6 +214,27 @@ impl OtgService { self.desired.read().await.msd_lun_capacity } + pub async fn configure_msd_lun(&self, lun: u8, config: &MsdLunConfig) -> Result<()> { + // Keep the manager locked across a possible UDC rebind so an OTG + // reconcile cannot replace the gadget halfway through the media-type + // transition. + let manager = self.manager.lock().await; + let gadget_path = manager + .as_ref() + .map(|value| value.gadget_path().clone()) + .ok_or_else(|| AppError::Internal("OTG gadget is not active".to_string()))?; + let function = self + .msd_function + .read() + .await + .clone() + .ok_or_else(|| AppError::Internal("MSD function is not active".to_string()))?; + + function + .configure_lun_async(&gadget_path, lun, config) + .await + } + pub async fn network_status(&self) -> OtgNetworkStatus { let state = self.state.read().await; OtgNetworkStatus { diff --git a/src/redfish/auth.rs b/src/redfish/auth.rs index fc4489fc..1b86ce6a 100644 --- a/src/redfish/auth.rs +++ b/src/redfish/auth.rs @@ -88,10 +88,7 @@ mod tests { #[test] fn only_service_discovery_and_session_creation_are_public() { assert!(is_redfish_public_endpoint("/v1/", &Method::GET)); - assert!(is_redfish_public_endpoint( - "/v1/$metadata", - &Method::GET - )); + assert!(is_redfish_public_endpoint("/v1/$metadata", &Method::GET)); assert!(is_redfish_public_endpoint( "/v1/SessionService/Sessions", &Method::POST diff --git a/src/redfish/routes/virtual_media.rs b/src/redfish/routes/virtual_media.rs index b7e03488..f6ecfb33 100644 --- a/src/redfish/routes/virtual_media.rs +++ b/src/redfish/routes/virtual_media.rs @@ -9,8 +9,8 @@ use std::sync::Arc; use tracing::{info, warn}; use super::super::schema::*; -use super::{empty_collection, resource_not_found, service_unavailable, validate_id}; -use crate::error::AppError; +use super::{empty_collection, resource_not_found, validate_id}; +use crate::error::{AppError, MsdErrorCode}; use crate::msd::{ImageInfo, ImageManager, MountedMedia, MountedMediaKind}; use crate::state::AppState; @@ -46,7 +46,7 @@ async fn virtual_media_collection( let capacity = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; msd.state().await.disk_mode.capacity() }; @@ -81,7 +81,7 @@ async fn virtual_media_detail( let (msd_state, lun) = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; let msd_state = msd.state().await; let Some(lun) = parse_slot_id(&media_id, msd_state.disk_mode.capacity()) else { @@ -164,17 +164,14 @@ async fn virtual_media_insert( let lun = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; let msd_state = msd.state().await; let Some(lun) = parse_slot_id(&media_id, msd_state.disk_mode.capacity()) else { return resource_not_found(); }; if msd_state.mounted_media.iter().any(|media| media.lun == lun) { - return redfish_error( - StatusCode::CONFLICT, - "Virtual media slot is already occupied", - ); + return msd_error_response(MsdErrorCode::MsdMediaSlotsFull); } lun }; @@ -194,7 +191,7 @@ async fn virtual_media_insert( let result = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; msd.mount_image_at_lun(&image, cdrom, read_only, lun).await }; @@ -222,7 +219,7 @@ async fn virtual_media_eject( let lun = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; let capacity = msd.state().await.disk_mode.capacity(); let Some(lun) = parse_slot_id(&media_id, capacity) else { @@ -234,7 +231,7 @@ async fn virtual_media_eject( let result = { let guard = state.msd.read().await; let Some(msd) = guard.as_ref() else { - return service_unavailable("MSD not available"); + return msd_error_response(MsdErrorCode::MsdUnavailable); }; msd.unmount_lun(lun).await }; @@ -352,6 +349,9 @@ async fn resolve_image( } fn app_error_response(error: AppError) -> Response { + if let AppError::Msd(error) = error { + return msd_error_response(error.code()); + } let status = match &error { AppError::BadRequest(_) => StatusCode::BAD_REQUEST, AppError::NotFound(_) => StatusCode::NOT_FOUND, @@ -361,6 +361,35 @@ fn app_error_response(error: AppError) -> Response { redfish_error(status, &error.to_string()) } +fn msd_error_response(code: MsdErrorCode) -> Response { + use MsdErrorCode::*; + let status = match code { + MsdUnavailable => StatusCode::SERVICE_UNAVAILABLE, + MsdResourceNotFound | MsdDriveNotInitialized => StatusCode::NOT_FOUND, + MsdOperationInProgress + | MsdResourceAlreadyExists + | MsdMediaSlotsFull + | MsdMediaAlreadyMounted + | MsdMediaInUse + | MsdDriveConnected + | MsdMediumRemovalPrevented => StatusCode::CONFLICT, + MsdInvalidRequest + | MsdImageTooLarge + | MsdInvalidUrl + | MsdDriveFilesystemUnsupported + | MsdDriveSizeInvalid + | MsdStorageSpaceUnavailable + | MsdStorageFull + | MsdStorageReadOnly + | MsdStoragePermissionDenied => StatusCode::BAD_REQUEST, + MsdOperationFailed + | MsdRemoteDownloadFailed + | MsdDownloadIncomplete + | MsdDisconnectFailed => StatusCode::INTERNAL_SERVER_ERROR, + }; + (status, Json(RedfishError::msd(code))).into_response() +} + fn redfish_error(status: StatusCode, message: &str) -> Response { (status, Json(RedfishError::general_error(message))).into_response() } @@ -421,4 +450,45 @@ mod tests { not_inserted.inserted = Some(false); assert!(validate_insert_request(¬_inserted).is_err()); } + + #[test] + fn msd_redfish_errors_use_the_one_kvm_registry_shape() { + for code in MsdErrorCode::ALL { + let body = RedfishError::msd(code); + let expected = format!("OneKVM.1.0.{}", code.redfish_key()); + assert_eq!(body.error.code, expected); + assert_eq!(body.error.message, code.message()); + assert_eq!(body.error.extended_info.len(), 1); + let info = &body.error.extended_info[0]; + assert_eq!(info.message_id, expected); + assert_eq!(info.message, code.message()); + assert_eq!(info.severity, code.severity()); + assert_eq!(info.resolution, code.resolution()); + } + } + + #[tokio::test] + async fn msd_and_validation_errors_keep_separate_redfish_registries() { + let response = msd_error_response(MsdErrorCode::MsdStoragePermissionDenied); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!( + json["error"]["code"], + "OneKVM.1.0.MsdStoragePermissionDenied" + ); + assert_eq!( + json["error"]["@Message.ExtendedInfo"][0]["MessageId"], + "OneKVM.1.0.MsdStoragePermissionDenied" + ); + + let validation = app_error_response(AppError::BadRequest("invalid property".into())); + let body = axum::body::to_bytes(validation.into_body(), usize::MAX) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"]["code"], "Base.1.18.GeneralError"); + } } diff --git a/src/redfish/schema.rs b/src/redfish/schema.rs index 7663a4f1..fb8d1840 100644 --- a/src/redfish/schema.rs +++ b/src/redfish/schema.rs @@ -1,3 +1,4 @@ +use crate::error::MsdErrorCode; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -546,6 +547,23 @@ pub struct RedfishExtendedInfo { } impl RedfishError { + pub fn msd(code: MsdErrorCode) -> Self { + let message_id = format!("OneKVM.1.0.{}", code.redfish_key()); + Self { + error: RedfishErrorBody { + code: message_id.clone(), + message: code.message().to_string(), + extended_info: vec![RedfishExtendedInfo { + odata_type: "#Message.v1_2_1.Message".to_string(), + message_id, + message: code.message().to_string(), + severity: code.severity().to_string(), + resolution: code.resolution().to_string(), + }], + }, + } + } + pub fn general_error(message: &str) -> Self { Self { error: RedfishErrorBody { diff --git a/src/web/error.rs b/src/web/error.rs index 41edda1c..abf809c1 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -9,6 +9,8 @@ use serde::Serialize; #[derive(Serialize)] pub struct ErrorResponse { pub success: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub code: Option<&'static str>, pub message: String, } @@ -17,7 +19,8 @@ impl IntoResponse for AppError { let status = status_code(&self); let body = ErrorResponse { success: false, - message: self.to_string(), + code: error_code(&self), + message: public_message(&self), }; tracing::error!( @@ -38,10 +41,53 @@ fn status_code(error: &AppError) -> StatusCode { AppError::RateLimited(_) => StatusCode::TOO_MANY_REQUESTS, AppError::NotFound(_) => StatusCode::NOT_FOUND, AppError::ServiceUnavailable(_) => StatusCode::SERVICE_UNAVAILABLE, + AppError::Msd(error) => msd_status_code(error.code()), _ => StatusCode::INTERNAL_SERVER_ERROR, } } +fn error_code(error: &AppError) -> Option<&'static str> { + match error { + AppError::Msd(error) => Some(error.code().as_str()), + _ => None, + } +} + +fn public_message(error: &AppError) -> String { + match error { + AppError::Msd(error) => error.code().message().to_string(), + _ => error.to_string(), + } +} + +pub(crate) fn msd_status_code(code: crate::error::MsdErrorCode) -> StatusCode { + use crate::error::MsdErrorCode::*; + match code { + MsdUnavailable => StatusCode::SERVICE_UNAVAILABLE, + MsdResourceNotFound | MsdDriveNotInitialized => StatusCode::NOT_FOUND, + MsdOperationInProgress + | MsdResourceAlreadyExists + | MsdMediaSlotsFull + | MsdMediaAlreadyMounted + | MsdMediaInUse + | MsdDriveConnected + | MsdMediumRemovalPrevented => StatusCode::CONFLICT, + MsdInvalidRequest + | MsdImageTooLarge + | MsdInvalidUrl + | MsdDriveFilesystemUnsupported + | MsdDriveSizeInvalid + | MsdStorageSpaceUnavailable + | MsdStorageFull + | MsdStorageReadOnly + | MsdStoragePermissionDenied => StatusCode::BAD_REQUEST, + MsdOperationFailed + | MsdRemoteDownloadFailed + | MsdDownloadIncomplete + | MsdDisconnectFailed => StatusCode::INTERNAL_SERVER_ERROR, + } +} + #[cfg(test)] mod tests { use super::*; @@ -72,6 +118,18 @@ mod tests { status_code(&AppError::RateLimited("limited".to_string())), StatusCode::TOO_MANY_REQUESTS ); + assert_eq!( + status_code(&AppError::from( + crate::error::MsdErrorCode::MsdMediumRemovalPrevented + )), + StatusCode::CONFLICT + ); + assert_eq!( + error_code(&AppError::from( + crate::error::MsdErrorCode::MsdMediumRemovalPrevented + )), + Some("MSD_MEDIUM_REMOVAL_PREVENTED") + ); } #[test] @@ -81,4 +139,60 @@ mod tests { StatusCode::INTERNAL_SERVER_ERROR ); } + + #[test] + fn every_msd_error_has_a_stable_code_message_and_status() { + use crate::error::MsdErrorCode::*; + let cases = [ + (MsdUnavailable, StatusCode::SERVICE_UNAVAILABLE), + (MsdOperationInProgress, StatusCode::CONFLICT), + (MsdOperationFailed, StatusCode::INTERNAL_SERVER_ERROR), + (MsdInvalidRequest, StatusCode::BAD_REQUEST), + (MsdResourceNotFound, StatusCode::NOT_FOUND), + (MsdResourceAlreadyExists, StatusCode::CONFLICT), + (MsdMediaSlotsFull, StatusCode::CONFLICT), + (MsdMediaAlreadyMounted, StatusCode::CONFLICT), + (MsdMediaInUse, StatusCode::CONFLICT), + (MsdImageTooLarge, StatusCode::BAD_REQUEST), + (MsdInvalidUrl, StatusCode::BAD_REQUEST), + (MsdRemoteDownloadFailed, StatusCode::INTERNAL_SERVER_ERROR), + (MsdDownloadIncomplete, StatusCode::INTERNAL_SERVER_ERROR), + (MsdDriveNotInitialized, StatusCode::NOT_FOUND), + (MsdDriveConnected, StatusCode::CONFLICT), + (MsdDriveFilesystemUnsupported, StatusCode::BAD_REQUEST), + (MsdDriveSizeInvalid, StatusCode::BAD_REQUEST), + (MsdStorageSpaceUnavailable, StatusCode::BAD_REQUEST), + (MsdStorageFull, StatusCode::BAD_REQUEST), + (MsdStorageReadOnly, StatusCode::BAD_REQUEST), + (MsdStoragePermissionDenied, StatusCode::BAD_REQUEST), + (MsdMediumRemovalPrevented, StatusCode::CONFLICT), + (MsdDisconnectFailed, StatusCode::INTERNAL_SERVER_ERROR), + ]; + assert_eq!(cases.len(), crate::error::MsdErrorCode::ALL.len()); + for (code, expected_status) in cases { + let error = AppError::from(code); + assert_eq!(error_code(&error), Some(code.as_str())); + assert_eq!(public_message(&error), code.message()); + assert!(!code.message().contains('/')); + assert_eq!(msd_status_code(code), expected_status); + } + } + + #[tokio::test] + async fn msd_response_contains_only_the_public_error_contract() { + let response = + AppError::from(crate::error::MsdErrorCode::MsdOperationFailed).into_response(); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + + assert_eq!(json["success"], false); + assert_eq!(json["code"], "MSD_OPERATION_FAILED"); + assert_eq!( + json["message"], + crate::error::MsdErrorCode::MsdOperationFailed.message() + ); + assert_eq!(json.as_object().unwrap().len(), 3); + } } diff --git a/src/web/handlers/msd_api.rs b/src/web/handlers/msd_api.rs index 9f0e7943..05ca0c60 100644 --- a/src/web/handlers/msd_api.rs +++ b/src/web/handlers/msd_api.rs @@ -3,12 +3,14 @@ use super::*; use crate::msd::{ DiskModeRequest, DownloadProgress, DriveFile, DriveInfo, DriveInitRequest, - ImageDownloadRequest, ImageInfo, ImageManager, ImageMountRequest, MsdState, MsdStateResponse, - VentoyDrive, MIN_DRIVE_SIZE_MB, + ImageDownloadRequest, ImageInfo, ImageManager, ImageMountRequest, MsdErrorCode, MsdState, + MsdStateResponse, VentoyDrive, MIN_DRIVE_SIZE_MB, }; #[cfg(unix)] use axum::body::Body; #[cfg(unix)] +use axum::extract::{multipart::MultipartRejection, rejection::JsonRejection}; +#[cfg(unix)] use axum::extract::{Multipart, Path as AxumPath}; #[cfg(unix)] use axum::http::{header, StatusCode}; @@ -29,10 +31,7 @@ async fn assert_drive_not_connected(state: &Arc) -> Result<()> { let msd_guard = state.msd.read().await; if let Some(controller) = msd_guard.as_ref() { if controller.is_drive_connected().await { - return Err(AppError::BadRequest( - "Virtual drive is connected to the USB host; disconnect it before modifying files" - .to_string(), - )); + return Err(MsdErrorCode::MsdDriveConnected.into()); } } Ok(()) @@ -42,35 +41,61 @@ async fn assert_drive_not_connected(state: &Arc) -> Result<()> { fn validate_drive_init_size(size_mb: u32, available_bytes: u64) -> Result<()> { let requested_bytes = size_mb as u64 * MIB; if size_mb < MIN_DRIVE_SIZE_MB { - return Err(AppError::BadRequest(format!( - "Virtual drive size must be at least {} MB", - MIN_DRIVE_SIZE_MB - ))); + return Err(MsdErrorCode::MsdDriveSizeInvalid.into()); } if requested_bytes > available_bytes { - return Err(AppError::BadRequest(format!( - "Virtual drive size cannot exceed available space on the MSD directory filesystem (available {} MB, requested {} MB)", - available_bytes / MIB, - size_mb - ))); + return Err(MsdErrorCode::MsdStorageFull.into()); } Ok(()) } #[cfg(unix)] -fn is_unsupported_drive_filesystem(error: &str) -> bool { - error.contains("Filesystem error") - || error.contains("Image error") - || error.contains("Partition error") +fn msd_controller<'a>( + guard: &'a tokio::sync::RwLockReadGuard<'_, Option>, +) -> Result<&'a crate::msd::MsdController> { + guard + .as_ref() + .ok_or_else(|| MsdErrorCode::MsdUnavailable.into()) } #[cfg(unix)] -fn unsupported_drive_filesystem_error(error: &str) -> AppError { - tracing::warn!( - error = %error, - "Virtual drive filesystem is not supported" - ); - AppError::BadRequest("Unsupported drive filesystem".to_string()) +fn classify_storage_error(operation: &'static str, error: std::io::Error) -> AppError { + tracing::warn!(operation, %error, "MSD storage operation failed"); + match error.raw_os_error() { + Some(libc::ENOSPC) => MsdErrorCode::MsdStorageFull.into(), + Some(libc::EROFS) => MsdErrorCode::MsdStorageReadOnly.into(), + Some(libc::EACCES | libc::EPERM) => MsdErrorCode::MsdStoragePermissionDenied.into(), + _ => MsdErrorCode::MsdOperationFailed.into(), + } +} + +#[cfg(unix)] +fn operation_failed(operation: &'static str, error: AppError) -> AppError { + match error { + AppError::Msd(error) => AppError::Msd(error), + error => { + tracing::warn!(operation, %error, "Unclassified MSD operation failed"); + MsdErrorCode::MsdOperationFailed.into() + } + } +} + +#[cfg(unix)] +fn parse_msd_json(payload: std::result::Result, JsonRejection>) -> Result { + payload.map(|Json(value)| value).map_err(|error| { + tracing::warn!(%error, "Failed to parse MSD JSON request"); + MsdErrorCode::MsdInvalidRequest.into() + }) +} + +#[cfg(unix)] +fn parse_msd_multipart( + payload: std::result::Result, +) -> Result { + payload.map_err(|error| { + tracing::warn!(%error, "Failed to parse MSD multipart request"); + MsdErrorCode::MsdInvalidRequest.into() + }) } /// MSD status response @@ -115,22 +140,22 @@ pub async fn msd_images_list(State(state): State>) -> Result>, - mut multipart: Multipart, + multipart: std::result::Result, ) -> Result> { + let mut multipart = parse_msd_multipart(multipart)?; let config = state.config.get(); let images_path = config.msd.images_dir(); let manager = ImageManager::new(images_path); - while let Some(field) = multipart - .next_field() - .await - .map_err(|e| AppError::Internal(format!("Multipart error: {}", e)))? - { + while let Some(field) = multipart.next_field().await.map_err(|error| { + tracing::warn!(%error, "Failed to parse MSD image upload"); + AppError::from(MsdErrorCode::MsdInvalidRequest) + })? { let name = field.name().unwrap_or("file").to_string(); if name == "file" { let filename = field .file_name() - .ok_or_else(|| AppError::BadRequest("Missing filename".to_string()))? + .ok_or_else(|| AppError::from(MsdErrorCode::MsdInvalidRequest))? .to_string(); // Use streaming upload - chunks are written directly to disk @@ -142,7 +167,7 @@ pub async fn msd_image_upload( } } - Err(AppError::BadRequest("No file provided".to_string())) + Err(MsdErrorCode::MsdInvalidRequest.into()) } /// Get image by ID @@ -166,10 +191,11 @@ pub async fn msd_image_delete( AxumPath(id): AxumPath, ) -> Result> { let msd_guard = state.msd.read().await; - let controller = msd_guard - .as_ref() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; - controller.delete_image(&id).await?; + let controller = msd_controller(&msd_guard)?; + controller + .delete_image(&id) + .await + .map_err(|error| operation_failed("delete image", error))?; Ok(Json(LoginResponse { success: true, message: Some("Image deleted".to_string()), @@ -180,14 +206,16 @@ pub async fn msd_image_delete( #[cfg(unix)] pub async fn msd_image_download( State(state): State>, - Json(req): Json, + payload: std::result::Result, JsonRejection>, ) -> Result> { + let req = parse_msd_json(payload)?; let msd_guard = state.msd.read().await; - let controller = msd_guard - .as_ref() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + let controller = msd_controller(&msd_guard)?; - let progress = controller.download_image(req.url, req.filename).await?; + let progress = controller + .download_image(req.url, req.filename) + .await + .map_err(|error| operation_failed("start image download", error))?; Ok(Json(progress)) } @@ -202,14 +230,16 @@ pub struct CancelDownloadRequest { #[cfg(unix)] pub async fn msd_image_download_cancel( State(state): State>, - Json(req): Json, + payload: std::result::Result, JsonRejection>, ) -> Result> { + let req = parse_msd_json(payload)?; let msd_guard = state.msd.read().await; - let controller = msd_guard - .as_ref() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + let controller = msd_controller(&msd_guard)?; - controller.cancel_download(&req.download_id).await?; + controller + .cancel_download(&req.download_id) + .await + .map_err(|error| operation_failed("cancel image download", error))?; Ok(Json(LoginResponse { success: true, @@ -221,14 +251,16 @@ pub async fn msd_image_download_cancel( #[cfg(unix)] pub async fn msd_disk_mode_put( State(state): State>, - Json(req): Json, + payload: std::result::Result, JsonRejection>, ) -> Result> { - let _otg_guard = try_apply_lock(&state.config_apply_locks.otg, "OTG")?; + let req = parse_msd_json(payload)?; + let _otg_guard = try_apply_lock(&state.config_apply_locks.otg, "OTG").map_err(|error| { + tracing::warn!(%error, "MSD disk mode change is blocked by another OTG operation"); + AppError::from(MsdErrorCode::MsdOperationInProgress) + })?; let current_mode = { let msd_guard = state.msd.read().await; - let controller = msd_guard - .as_ref() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + let controller = msd_controller(&msd_guard)?; controller.state().await.disk_mode }; if current_mode == req.disk_mode { @@ -248,14 +280,14 @@ pub async fn msd_disk_mode_put( .hid .prepare_otg_rebuild() .await - .map_err(|e| AppError::Config(format!("Failed to prepare OTG HID for rebuild: {e}")))?; + .map_err(|error| operation_failed("prepare HID for disk mode switch", error))?; } let switch_result = { let mut msd_guard = state.msd.write().await; let controller = msd_guard .as_mut() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable))?; controller.set_disk_mode(req.disk_mode).await }; @@ -271,12 +303,18 @@ pub async fn msd_disk_mode_put( match (switch_result, hid_reload_result) { (Err(switch_error), Err(hid_error)) => { - return Err(AppError::Internal(format!( - "MSD disk mode switch failed: {switch_error}; HID recovery failed: {hid_error}" - ))); + tracing::warn!(%switch_error, %hid_error, "MSD mode switch and HID recovery failed"); + return Err(MsdErrorCode::MsdOperationFailed.into()); + } + (Err(switch_error), Ok(())) => { + return Err(operation_failed("switch disk mode", switch_error)) + } + (Ok(_), Err(hid_error)) => { + return Err(operation_failed( + "recover HID after disk mode switch", + hid_error, + )) } - (Err(switch_error), Ok(())) => return Err(switch_error), - (Ok(_), Err(hid_error)) => return Err(hid_error), (Ok(_), Ok(())) => {} } @@ -291,13 +329,14 @@ pub async fn msd_disk_mode_put( pub async fn msd_image_mount( State(state): State>, AxumPath(id): AxumPath, - Json(req): Json, + payload: std::result::Result, JsonRejection>, ) -> Result> { + let req = parse_msd_json(payload)?; let config = state.config.get(); let mut msd_guard = state.msd.write().await; let controller = msd_guard .as_mut() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable))?; let images_path = config.msd.images_dir(); let manager = ImageManager::new(images_path); @@ -305,7 +344,8 @@ pub async fn msd_image_mount( controller .mount_image(&image, req.cdrom, req.read_only) - .await?; + .await + .map_err(|error| operation_failed("mount image", error))?; Ok(Json(LoginResponse { success: true, @@ -322,9 +362,12 @@ pub async fn msd_image_unmount( let mut msd_guard = state.msd.write().await; let controller = msd_guard .as_mut() - .ok_or_else(|| AppError::Internal("MSD not initialized".to_string()))?; + .ok_or_else(|| AppError::from(MsdErrorCode::MsdUnavailable))?; - controller.unmount_image(&id).await?; + controller + .unmount_image(&id) + .await + .map_err(|error| operation_failed("unmount image", error))?; Ok(Json(LoginResponse { success: true, @@ -338,9 +381,12 @@ pub async fn msd_drive_mount(State(state): State>) -> Result>) -> Result>) -> Result Ok(Json(info)), - Err(e) => { - let msg = e.to_string(); - // Detect filesystem-level failures (unrecognized format, bad partition table, etc.) - // These mean the drive FILE exists but was formatted to an unsupported type - // (e.g. the controlled machine reformatted it as NTFS/exFAT). - // Return 400 so the frontend can distinguish this from 404 (file missing). - if is_unsupported_drive_filesystem(&msg) { - return Err(unsupported_drive_filesystem_error(&msg)); - } - Err(e) - } - } + drive + .info() + .await + .map(Json) + .map_err(|error| operation_failed("read virtual drive info", error)) } /// Initialize Ventoy drive #[cfg(unix)] pub async fn msd_drive_init( State(state): State>, - Json(req): Json, + payload: std::result::Result, JsonRejection>, ) -> Result> { + let req = parse_msd_json(payload)?; + assert_drive_not_connected(&state).await?; let config = state.config.get(); let msd_dir = config.msd.msd_dir_path(); - let disk_space = get_disk_space(&msd_dir).map_err(|e| { - AppError::BadRequest(format!( - "Failed to read available space for the MSD directory filesystem: {}", - e - )) + let disk_space = get_disk_space(&msd_dir).map_err(|error| { + tracing::warn!(%error, "Failed to read MSD storage space"); + AppError::from(MsdErrorCode::MsdStorageSpaceUnavailable) })?; validate_drive_init_size(req.size_mb, disk_space.available)?; let drive_path = config.msd.drive_path(); let drive = VentoyDrive::new(drive_path); - let info = drive.init(req.size_mb).await?; + let info = drive + .init(req.size_mb) + .await + .map_err(|error| operation_failed("initialize virtual drive", error))?; Ok(Json(info)) } @@ -425,9 +468,7 @@ pub async fn msd_drive_delete(State(state): State>) -> Result>) -> Result>, Query(params): Query>, - mut multipart: Multipart, + multipart: std::result::Result, ) -> Result> { + let mut multipart = parse_msd_multipart(multipart)?; // Block when connected: writing to image while USB host has it mounted // causes filesystem corruption (Windows error 0x80070570) assert_drive_not_connected(&state).await?; @@ -489,16 +525,15 @@ pub async fn msd_drive_upload( let target_dir = params.get("path").map(|s| s.as_str()).unwrap_or("/"); - while let Some(field) = multipart - .next_field() - .await - .map_err(|e| AppError::Internal(format!("Multipart error: {}", e)))? - { + while let Some(field) = multipart.next_field().await.map_err(|error| { + tracing::warn!(%error, "Failed to parse virtual drive file upload"); + AppError::from(MsdErrorCode::MsdInvalidRequest) + })? { let name = field.name().unwrap_or("file").to_string(); if name == "file" { let filename = field .file_name() - .ok_or_else(|| AppError::BadRequest("Missing filename".to_string()))? + .ok_or_else(|| AppError::from(MsdErrorCode::MsdInvalidRequest))? .to_string(); let file_path = if target_dir == "/" { @@ -511,7 +546,8 @@ pub async fn msd_drive_upload( // This avoids loading the entire file into memory drive .write_file_from_multipart_field(&file_path, field) - .await?; + .await + .map_err(|error| operation_failed("upload virtual drive file", error))?; return Ok(Json(LoginResponse { success: true, @@ -520,7 +556,7 @@ pub async fn msd_drive_upload( } } - Err(AppError::BadRequest("No file provided".to_string())) + Err(MsdErrorCode::MsdInvalidRequest.into()) } /// Download file from drive (streaming for large files) @@ -538,7 +574,10 @@ pub async fn msd_drive_download( let drive = VentoyDrive::new(drive_path); // Get file stream (returns file size and channel receiver) - let (file_size, mut rx) = drive.read_file_stream(&file_path).await?; + let (file_size, mut rx) = drive + .read_file_stream(&file_path) + .await + .map_err(|error| operation_failed("download virtual drive file", error))?; // Extract filename for Content-Disposition let filename = file_path.split('/').next_back().unwrap_or("download"); @@ -576,7 +615,10 @@ pub async fn msd_drive_file_delete( let drive_path = config.msd.drive_path(); let drive = VentoyDrive::new(drive_path); - drive.delete(&file_path).await?; + drive + .delete(&file_path) + .await + .map_err(|error| operation_failed("delete virtual drive file", error))?; Ok(Json(LoginResponse { success: true, @@ -598,7 +640,10 @@ pub async fn msd_drive_mkdir( let drive_path = config.msd.drive_path(); let drive = VentoyDrive::new(drive_path); - drive.mkdir(&dir_path).await?; + drive + .mkdir(&dir_path) + .await + .map_err(|error| operation_failed("create virtual drive directory", error))?; Ok(Json(LoginResponse { success: true, @@ -618,25 +663,24 @@ mod tests { #[test] fn validate_drive_init_size_rejects_below_64mb() { let err = validate_drive_init_size(MIN_DRIVE_SIZE_MB - 1, 1024 * MIB).unwrap_err(); - assert!(err.to_string().contains("at least 64 MB")); + assert!( + matches!(err, AppError::Msd(error) if error.code() == MsdErrorCode::MsdDriveSizeInvalid) + ); } #[test] fn validate_drive_init_size_rejects_available_space_overflow() { let err = validate_drive_init_size(65, 64 * MIB).unwrap_err(); - assert!(err.to_string().contains("cannot exceed available space")); + assert!( + matches!(err, AppError::Msd(error) if error.code() == MsdErrorCode::MsdStorageFull) + ); } #[test] - fn detects_unsupported_drive_filesystem_errors() { - assert!(is_unsupported_drive_filesystem( - "Internal error: Filesystem error: Invalid exFAT signature" - )); - assert!(is_unsupported_drive_filesystem( - "Internal error: Partition error: invalid partition table" - )); - assert!(!is_unsupported_drive_filesystem( - "IO error: permission denied" - )); + fn classifies_storage_permissions_without_exposing_the_io_error() { + let error = classify_storage_error("test", std::io::Error::from_raw_os_error(libc::EACCES)); + assert!( + matches!(error, AppError::Msd(error) if error.code() == MsdErrorCode::MsdStoragePermissionDenied) + ); } } diff --git a/web/src/api/index.ts b/web/src/api/index.ts index f795039d..e449ccaa 100644 --- a/web/src/api/index.ts +++ b/web/src/api/index.ts @@ -1,4 +1,4 @@ -import { request, ApiError } from './request' +import { request, uploadRequest, ApiError } from './request' import type { CanonicalKey, Ch9329DescriptorState, @@ -620,61 +620,42 @@ export const msdApi = { } | null usb_reenumerating: boolean } - }>('/msd/status'), + }>('/msd/status', {}, { toastOnError: false }), - listImages: () => request('/msd/images'), + listImages: () => request('/msd/images', {}, { toastOnError: false }), uploadImage: async (file: File, onProgress?: (progress: number) => void) => { const formData = new FormData() formData.append('file', file) - const xhr = new XMLHttpRequest() - xhr.open('POST', `${API_BASE}/msd/images`) - xhr.withCredentials = true - - return new Promise((resolve, reject) => { - xhr.upload.onprogress = (e) => { - if (e.lengthComputable && onProgress) { - onProgress((e.loaded / e.total) * 100) - } - } - - xhr.onload = () => { - if (xhr.status >= 200 && xhr.status < 300) { - resolve(JSON.parse(xhr.responseText)) - } else { - reject(new ApiError(xhr.status, 'Upload failed')) - } - } - - xhr.onerror = () => reject(new ApiError(0, 'Network error')) - xhr.send(formData) + return uploadRequest('/msd/images', formData, onProgress, { + errorTitleKey: 'msd.operations.uploadImage', }) }, deleteImage: (id: string) => - request<{ success: boolean }>(`/msd/images/${id}`, { method: 'DELETE' }), + request<{ success: boolean }>(`/msd/images/${id}`, { method: 'DELETE' }, { errorTitleKey: 'msd.operations.deleteImage' }), setDiskMode: (diskMode: DiskMode) => request<{ success: boolean }>('/msd/disk-mode', { method: 'PUT', body: JSON.stringify({ disk_mode: diskMode }), - }), + }, { errorTitleKey: 'msd.operations.changeMode' }), mountImage: (id: string, cdrom: boolean, readOnly: boolean) => request<{ success: boolean }>(`/msd/images/${id}/mount`, { method: 'POST', body: JSON.stringify({ cdrom, read_only: readOnly }), - }), + }, { errorTitleKey: 'msd.operations.mountImage' }), unmountImage: (id: string) => - request<{ success: boolean }>(`/msd/images/${id}/mount`, { method: 'DELETE' }), + request<{ success: boolean }>(`/msd/images/${id}/mount`, { method: 'DELETE' }, { errorTitleKey: 'msd.operations.unmountImage' }), mountDrive: () => - request<{ success: boolean }>('/msd/drive/mount', { method: 'POST' }), + request<{ success: boolean }>('/msd/drive/mount', { method: 'POST' }, { errorTitleKey: 'msd.operations.mountDrive' }), unmountDrive: () => - request<{ success: boolean }>('/msd/drive/mount', { method: 'DELETE' }), + request<{ success: boolean }>('/msd/drive/mount', { method: 'DELETE' }, { errorTitleKey: 'msd.operations.unmountDrive' }), driveInfo: () => request<{ @@ -696,11 +677,11 @@ export const msdApi = { method: 'POST', body: JSON.stringify({ size_mb: sizeMb }), }, - { toastOnError: false }, + { errorTitleKey: 'msd.operations.initializeDrive' }, ), deleteDrive: () => - request<{ success: boolean }>('/msd/drive', { method: 'DELETE' }), + request<{ success: boolean }>('/msd/drive', { method: 'DELETE' }, { errorTitleKey: 'msd.operations.deleteDrive' }), listDriveFiles: (path = '/') => request( @@ -713,28 +694,12 @@ export const msdApi = { const formData = new FormData() formData.append('file', file) - const xhr = new XMLHttpRequest() - xhr.open('POST', `${API_BASE}/msd/drive/files?path=${encodeURIComponent(targetPath)}`) - xhr.withCredentials = true - - return new Promise<{ success: boolean; message?: string }>((resolve, reject) => { - xhr.upload.onprogress = (e) => { - if (e.lengthComputable && onProgress) { - onProgress((e.loaded / e.total) * 100) - } - } - - xhr.onload = () => { - if (xhr.status >= 200 && xhr.status < 300) { - resolve(JSON.parse(xhr.responseText)) - } else { - reject(new ApiError(xhr.status, 'Upload failed')) - } - } - - xhr.onerror = () => reject(new ApiError(0, 'Network error')) - xhr.send(formData) - }) + return uploadRequest<{ success: boolean; message?: string }>( + `/msd/drive/files?path=${encodeURIComponent(targetPath)}`, + formData, + onProgress, + { errorTitleKey: 'msd.operations.uploadDriveFile' }, + ) }, downloadDriveFile: (path: string) => @@ -743,12 +708,12 @@ export const msdApi = { deleteDriveFile: (path: string) => request<{ success: boolean }>(`/msd/drive/files${encodeDrivePath(path)}`, { method: 'DELETE', - }), + }, { errorTitleKey: 'msd.operations.deleteDriveFile' }), createDirectory: (path: string) => request<{ success: boolean }>(`/msd/drive/mkdir${encodeDrivePath(path)}`, { method: 'POST', - }), + }, { errorTitleKey: 'msd.operations.createDirectory' }), downloadFromUrl: (url: string, filename?: string) => request<{ @@ -759,17 +724,17 @@ export const msdApi = { total_bytes: number | null progress_pct: number | null status: string - error: string | null + error_code: string | null }>('/msd/images/download', { method: 'POST', body: JSON.stringify({ url, filename }), - }), + }, { errorTitleKey: 'msd.operations.startDownload' }), cancelDownload: (downloadId: string) => request<{ success: boolean }>('/msd/images/download/cancel', { method: 'POST', body: JSON.stringify({ download_id: downloadId }), - }), + }, { errorTitleKey: 'msd.operations.cancelDownload' }), } interface SerialDeviceOption { diff --git a/web/src/api/request.ts b/web/src/api/request.ts index 90efc88d..f32fc44b 100644 --- a/web/src/api/request.ts +++ b/web/src/api/request.ts @@ -29,11 +29,13 @@ function hasTranslation(key: string): boolean { export class ApiError extends Error { status: number + code?: string - constructor(status: number, message: string) { + constructor(status: number, message: string, code?: string) { super(message) this.name = 'ApiError' this.status = status + this.code = code } } @@ -47,26 +49,69 @@ export interface ApiRequestConfig { * Toast debounce key. Defaults to `error_${endpoint}`. */ toastKey?: string + /** Translation key used as the error toast title. */ + errorTitleKey?: string } function getToastKey(endpoint: string, config?: ApiRequestConfig): string { return config?.toastKey ?? `error_${endpoint}` } -function getErrorMessage(data: unknown, fallback: string): string { +function isAuthenticationIssue(status: number, message: string): boolean { + const normalized = message.toLowerCase() + return status === 401 && ( + normalized.includes('not authenticated') + || normalized.includes('session expired') + || normalized.includes('logged in elsewhere') + ) +} + +const msdErrorKeys: Record = { + MSD_UNAVAILABLE: 'msd.errors.unavailable', + MSD_OPERATION_IN_PROGRESS: 'msd.errors.operationInProgress', + MSD_OPERATION_FAILED: 'msd.errors.operationFailed', + MSD_INVALID_REQUEST: 'msd.errors.invalidRequest', + MSD_RESOURCE_NOT_FOUND: 'msd.errors.resourceNotFound', + MSD_RESOURCE_ALREADY_EXISTS: 'msd.errors.resourceAlreadyExists', + MSD_MEDIA_SLOTS_FULL: 'msd.errors.mediaSlotsFull', + MSD_MEDIA_ALREADY_MOUNTED: 'msd.errors.mediaAlreadyMounted', + MSD_MEDIA_IN_USE: 'msd.errors.mediaInUse', + MSD_IMAGE_TOO_LARGE: 'msd.errors.imageTooLarge', + MSD_INVALID_URL: 'msd.errors.invalidUrl', + MSD_REMOTE_DOWNLOAD_FAILED: 'msd.errors.remoteDownloadFailed', + MSD_DOWNLOAD_INCOMPLETE: 'msd.errors.downloadIncomplete', + MSD_DRIVE_NOT_INITIALIZED: 'msd.errors.driveNotInitialized', + MSD_DRIVE_CONNECTED: 'msd.errors.driveConnected', + MSD_DRIVE_FILESYSTEM_UNSUPPORTED: 'msd.errors.driveFilesystemUnsupported', + MSD_DRIVE_SIZE_INVALID: 'msd.errors.driveSizeInvalid', + MSD_STORAGE_SPACE_UNAVAILABLE: 'msd.errors.storageSpaceUnavailable', + MSD_STORAGE_FULL: 'msd.errors.storageFull', + MSD_STORAGE_READ_ONLY: 'msd.errors.storageReadOnly', + MSD_STORAGE_PERMISSION_DENIED: 'msd.errors.storagePermissionDenied', + MSD_MEDIUM_REMOVAL_PREVENTED: 'msd.errors.mediumRemovalPrevented', + MSD_DISCONNECT_FAILED: 'msd.errors.disconnectFailed', +} + +export function localizeMsdErrorCode(code?: string, fallback?: string): string { + const key = code ? msdErrorKeys[code] : undefined + if (key && hasTranslation(key)) return t(key) + return fallback ? localizeBackendErrorMessage(fallback) : t('msd.errors.operationFailed') +} + +function getErrorDetails(data: unknown, fallback: string): { message: string; code?: string } { if (data && typeof data === 'object') { const code = (data as any).code - const keyByCode: Record = { - MSD_MEDIUM_REMOVAL_PREVENTED: 'msd.errors.mediumRemovalPrevented', - MSD_DISCONNECT_FAILED: 'msd.errors.disconnectFailed', + const normalizedCode = typeof code === 'string' ? code : undefined + if (normalizedCode && msdErrorKeys[normalizedCode]) { + return { message: localizeMsdErrorCode(normalizedCode), code: normalizedCode } } - const key = typeof code === 'string' ? keyByCode[code] : undefined - if (key && hasTranslation(key)) return t(key) const message = (data as any).message - if (typeof message === 'string' && message.trim()) return localizeBackendErrorMessage(message) + if (typeof message === 'string' && message.trim()) { + return { message: localizeBackendErrorMessage(message), code: normalizedCode } + } } - return localizeBackendErrorMessage(fallback) + return { message: localizeBackendErrorMessage(fallback) } } function extractCh9329Command(reason: string): string { @@ -141,6 +186,7 @@ export async function request( const url = `${API_BASE}${endpoint}` const toastOnError = config.toastOnError !== false const toastKey = getToastKey(endpoint, config) + const errorTitle = t(config.errorTitleKey ?? 'api.operationFailed') try { const response = await fetch(url, { @@ -156,40 +202,35 @@ export async function request( // Handle HTTP errors (in case backend returns non-2xx) if (!response.ok) { - const message = getErrorMessage(data, `HTTP ${response.status}`) - const normalized = message.toLowerCase() - const isNotAuthenticated = normalized.includes('not authenticated') - const isSessionExpired = normalized.includes('session expired') - const isLoggedInElsewhere = normalized.includes('logged in elsewhere') - const isAuthIssue = response.status === 401 && (isNotAuthenticated || isSessionExpired || isLoggedInElsewhere) - if (toastOnError && shouldShowToast(toastKey) && !isAuthIssue) { - toast.error(t('api.operationFailed'), { + const { message, code } = getErrorDetails(data, `HTTP ${response.status}`) + if (toastOnError && shouldShowToast(toastKey) && !isAuthenticationIssue(response.status, message)) { + toast.error(errorTitle, { description: message, duration: 4000, }) } - throw new ApiError(response.status, message) + throw new ApiError(response.status, message, code) } // Handle backend "success=false" convention (even when HTTP is 200) if (data && typeof (data as any).success === 'boolean' && !(data as any).success) { - const message = getErrorMessage(data, t('api.operationFailedDesc')) + const { message, code } = getErrorDetails(data, t('api.operationFailedDesc')) if (toastOnError && shouldShowToast(toastKey)) { - toast.error(t('api.operationFailed'), { + toast.error(errorTitle, { description: message, duration: 4000, }) } - throw new ApiError(response.status, message) + throw new ApiError(response.status, message, code) } // If response body isn't JSON (or empty), treat as failure for callers expecting JSON. if (data === null) { const message = t('api.parseResponseFailed') if (toastOnError && shouldShowToast(toastKey)) { - toast.error(t('api.operationFailed'), { + toast.error(errorTitle, { description: message, duration: 4000, }) @@ -211,3 +252,55 @@ export async function request( throw new ApiError(0, t('api.networkError')) } } + +export function uploadRequest( + endpoint: string, + formData: FormData, + onProgress?: (progress: number) => void, + config: ApiRequestConfig = {}, +): Promise { + const xhr = new XMLHttpRequest() + xhr.open('POST', `${API_BASE}${endpoint}`) + xhr.withCredentials = true + + return new Promise((resolve, reject) => { + xhr.upload.onprogress = (event) => { + if (event.lengthComputable && onProgress) onProgress((event.loaded / event.total) * 100) + } + + xhr.onload = () => { + const data: unknown = (() => { + try { return JSON.parse(xhr.responseText) } catch { return null } + })() + if (xhr.status >= 200 && xhr.status < 300 && data !== null) { + resolve(data as T) + return + } + + const { message, code } = getErrorDetails(data, `HTTP ${xhr.status}`) + const error = new ApiError(xhr.status, message, code) + if ( + config.toastOnError !== false + && shouldShowToast(getToastKey(endpoint, config)) + && !isAuthenticationIssue(xhr.status, message) + ) { + toast.error(t(config.errorTitleKey ?? 'api.operationFailed'), { + description: message, + duration: 4000, + }) + } + reject(error) + } + + xhr.onerror = () => { + if (config.toastOnError !== false && shouldShowToast('network_error')) { + toast.error(t('api.networkError'), { + description: t('api.networkErrorDesc'), + duration: 4000, + }) + } + reject(new ApiError(0, t('api.networkError'))) + } + xhr.send(formData) + }) +} diff --git a/web/src/components/ActionBar.vue b/web/src/components/ActionBar.vue index bf7789e1..dc28a2f0 100644 --- a/web/src/components/ActionBar.vue +++ b/web/src/components/ActionBar.vue @@ -352,7 +352,7 @@ const hasRightOverflow = computed(() => { - + @@ -575,7 +575,7 @@ const hasRightOverflow = computed(() => { {{ t('actionbar.paste') }} - + diff --git a/web/src/components/MsdDialog.vue b/web/src/components/MsdDialog.vue index 39006480..efcbc126 100644 --- a/web/src/components/MsdDialog.vue +++ b/web/src/components/MsdDialog.vue @@ -4,7 +4,7 @@ import { useI18n } from 'vue-i18n' import { toast } from 'vue-sonner' import { useSystemStore } from '@/stores/system' import { msdApi, type MsdImage, type DriveFile, type MountedMedia, type DiskMode } from '@/api' -import { ApiError } from '@/api/request' +import { ApiError, localizeMsdErrorCode } from '@/api/request' import { useWebSocket } from '@/composables/useWebSocket' import { Dialog, @@ -64,9 +64,11 @@ const systemStore = useSystemStore() const { on, off } = useWebSocket() const activeTab = ref('images') +const msdStatusError = ref(null) const images = ref([]) const loadingImages = ref(false) +const imagesError = ref(null) const uploadProgress = ref(0) const uploading = ref(false) @@ -92,6 +94,8 @@ const driveInitialized = ref(false) const uploadingFile = ref(false) const fileUploadProgress = ref(0) const driveError = ref(null) // filesystem error (e.g. unsupported format) +const driveErrorCode = ref(null) +const driveFilesystemUnsupported = computed(() => driveErrorCode.value === 'MSD_DRIVE_FILESYSTEM_UNSUPPORTED') const showDeleteDialog = ref(false) const deleteTarget = ref<{ type: 'image' | 'file'; id: string; name: string } | null>(null) @@ -150,7 +154,9 @@ const downloadProgress = ref<{ total_bytes: number | null progress_pct: number | null status: string + error_code: string | null } | null>(null) +const downloadFailureNotifiedId = ref(null) const TWO_POINT_TWO_GB = 2.2 * 1024 * 1024 * 1024 const tabTriggerClass = 'h-8 rounded-md border-0 bg-transparent text-center text-muted-foreground shadow-none hover:text-foreground data-[state=active]:border-0 data-[state=active]:bg-background data-[state=active]:text-foreground data-[state=active]:shadow-sm' @@ -241,7 +247,7 @@ async function refreshDiskSpace() { async function loadData() { await refreshDiskSpace() - await systemStore.fetchMsdState() + await refreshMsdState() await loadImages() await loadDriveInfo() if (driveInitialized.value) { @@ -249,12 +255,24 @@ async function loadData() { } } +async function refreshMsdState() { + msdStatusError.value = null + try { + await systemStore.fetchMsdState() + } catch (e: any) { + msdStatusError.value = e?.message ?? t('msd.errors.operationFailed') + } +} + async function loadImages() { loadingImages.value = true + imagesError.value = null try { images.value = await msdApi.listImages() - } catch (e) { + } catch (e: any) { console.error('Failed to load images:', e) + imagesError.value = e?.message ?? t('msd.errors.operationFailed') + images.value = [] } finally { loadingImages.value = false } @@ -308,7 +326,7 @@ async function confirmImageMount() { connecting.value = true try { await msdApi.mountImage(image.id, cdromMode.value, cdromMode.value || readOnly.value) - await systemStore.fetchMsdState() + await refreshMsdState() showMountOptionsDialog.value = false pendingMountImage.value = null } catch (e) { @@ -333,7 +351,7 @@ async function connectDrive() { connecting.value = true try { await msdApi.mountDrive() - await systemStore.fetchMsdState() + await refreshMsdState() } catch (e) { console.error('Failed to mount drive:', e) } finally { @@ -352,7 +370,7 @@ async function unmountMedia(media: MountedMedia) { } else { await msdApi.unmountImage(media.id) } - await systemStore.fetchMsdState() + await refreshMsdState() } catch (e) { console.error('Failed to unmount media:', e) } finally { @@ -374,7 +392,7 @@ async function changeDiskMode(value: unknown) { modeChanging.value = true try { await msdApi.setDiskMode(next as DiskMode) - await systemStore.fetchMsdState() + await refreshMsdState() } catch (e) { console.error('Failed to change MSD disk mode:', e) } finally { @@ -408,9 +426,8 @@ async function executeDelete() { await msdApi.deleteDriveFile(deleteTarget.value.id) await loadDriveFiles() } - } catch (e: any) { + } catch (e) { console.error('Failed to delete:', e) - toast.error(t('common.error'), { description: e?.message }) } finally { showDeleteDialog.value = false deleteTarget.value = null @@ -420,12 +437,13 @@ async function executeDelete() { async function loadDriveInfo() { driveError.value = null + driveErrorCode.value = null try { driveInfo.value = await msdApi.driveInfo() driveInitialized.value = true } catch (e: any) { if (e instanceof ApiError) { - if (e.status === 404) { + if (e.code === 'MSD_DRIVE_NOT_INITIALIZED' || e.status === 404) { // Drive image file does not exist — truly not initialized driveInitialized.value = false driveInfo.value = null @@ -435,6 +453,7 @@ async function loadDriveInfo() { // an error banner instead of the misleading "Initialize Drive" button. driveInitialized.value = true driveError.value = e.message + driveErrorCode.value = e.code ?? null driveInfo.value = null } } else { @@ -469,16 +488,6 @@ async function createDrive() { showDriveInitDialog.value = false } catch (e) { console.error('Failed to initialize drive:', e) - let description: string | undefined - if (e instanceof ApiError) { - const message = e.message - if (message.includes('does not support a virtual drive file')) description = t('msd.driveFileTooLarge') - else if (message.includes('does not have enough free space')) description = t('msd.driveSpaceUnavailable') - else if (message.includes('filesystem is read-only')) description = t('msd.driveReadOnly') - else if (message.includes('permission to write')) description = t('msd.drivePermissionDenied') - else description = message - } - toast.error(t('msd.driveCreateFailed'), { description }) } finally { initializingDrive.value = false } @@ -510,12 +519,14 @@ async function loadDriveFiles() { } loadingDrive.value = true driveError.value = null + driveErrorCode.value = null try { driveFiles.value = await msdApi.listDriveFiles(currentPath.value) } catch (e: any) { console.error('Failed to load drive files:', e) // Surface the error — could be unsupported filesystem format driveError.value = e?.message ?? String(e) + driveErrorCode.value = e instanceof ApiError ? (e.code ?? null) : null driveFiles.value = [] } finally { loadingDrive.value = false @@ -564,9 +575,8 @@ async function handleFileUpload(e: Event) { fileUploadProgress.value = progress }) await loadDriveFiles() - } catch (e: any) { + } catch (e) { console.error('Failed to upload file:', e) - toast.error(t('msd.uploadFailed'), { description: e?.message }) } finally { uploadingFile.value = false fileUploadProgress.value = 0 @@ -591,9 +601,8 @@ async function createFolder() { : currentPath.value + '/' + newFolderName.value await msdApi.createDirectory(path) await loadDriveFiles() - } catch (e: any) { + } catch (e) { console.error('Failed to create folder:', e) - toast.error(t('common.error'), { description: e?.message }) } finally { showNewFolderDialog.value = false newFolderName.value = '' @@ -616,6 +625,7 @@ async function startUrlDownload() { total_bytes: result.total_bytes, progress_pct: result.progress_pct, status: result.status, + error_code: result.error_code, } } catch (e) { console.error('Failed to start download:', e) @@ -649,6 +659,7 @@ function handleDownloadProgress(data: { total_bytes: number | null progress_pct: number | null status: string + error_code: string | null }) { if (downloadProgress.value?.download_id === data.download_id) { downloadProgress.value = data @@ -659,8 +670,14 @@ function handleDownloadProgress(data: { showUrlDialog.value = false resetDownloadState() }, 1000) - } else if (data.status.startsWith('failed')) { + } else if (data.status === 'failed') { downloading.value = false + if (downloadFailureNotifiedId.value !== data.download_id) { + downloadFailureNotifiedId.value = data.download_id + toast.error(t('msd.operations.downloadImage'), { + description: localizeMsdErrorCode(data.error_code ?? undefined), + }) + } } } } @@ -747,6 +764,17 @@ onUnmounted(() => { +
+
+

{{ t('msd.operations.loadStatus') }}

+

{{ msdStatusError }}

+
+ +
+
@@ -796,6 +824,16 @@ onUnmounted(() => { +
+
+

{{ t('msd.operations.loadImages') }}

+

{{ imagesError }}

+
+ +
@@ -929,7 +967,7 @@ onUnmounted(() => {
-