feat: enhance error handling with IoError and IoErrorMetadata for improved std::io::Error diagnostics for fs errors #480
Conversation
…roved std::io::Error diagnostics for fs errors
wmmc88
changed the title
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances error handling in the wdk-build crate by introducing dedicated IoError and IoErrorMetadata types for I/O operations, providing better context for filesystem-related failures.
- Introduces new error types (
IoErrorandIoErrorMetadata) to wrapstd::io::Errorwith detailed path context - Refactors all filesystem operations throughout the codebase to use these enhanced error types
- Updates error handling in binding generation and file operations to capture and propagate path metadata
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/wdk-sys/build.rs | Updates bindgen file generation to use IoError with path metadata for write operations |
| crates/wdk-build/src/utils.rs | Converts filesystem operations to use IoError with path context for directory reading |
| crates/wdk-build/src/lib.rs | Defines new error types and refactors include/library path validation to use enhanced error handling |
| crates/wdk-build/src/cargo_make.rs | Updates path resolution and file operations to use IoError with detailed path metadata |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
crates/wdk-build/src/lib.rs
Outdated
| @@ -557,7 +594,7 @@ impl Config { | |||
| // Based off of logic from WindowsDriver.KernelMode.props & | |||
| // WindowsDriver.UserMode.props in NI(22H2) WDK | |||
| let windows_sdk_library_path = self.sdk_library_path(sdk_version)?; | |||
| library_paths.push(absolute(&windows_sdk_library_path)?); | |||
| Self::validate_and_add_include_path(&mut library_paths, &windows_sdk_library_path)?; | |||
There was a problem hiding this comment.
The function validate_and_add_include_path is being used to validate library paths, but based on its name and implementation, it's designed for include paths. This could lead to confusion and incorrect error messages that reference 'include paths' when dealing with library paths.
Copilot uses AI. Check for mistakes.
crates/wdk-build/src/lib.rs
Outdated
| }); | ||
| } | ||
| library_paths.push(absolute(&kmdf_library_path)?); | ||
| Self::validate_and_add_include_path(&mut library_paths, &kmdf_library_path)?; |
There was a problem hiding this comment.
The function validate_and_add_include_path is being used to validate library paths, but based on its name and implementation, it's designed for include paths. This could lead to confusion and incorrect error messages that reference 'include paths' when dealing with library paths.
Copilot uses AI. Check for mistakes.
crates/wdk-build/src/lib.rs
Outdated
| }); | ||
| } | ||
| library_paths.push(absolute(&umdf_library_path)?); | ||
| Self::validate_and_add_include_path(&mut library_paths, &umdf_library_path)?; |
There was a problem hiding this comment.
The function validate_and_add_include_path is being used to validate library paths, but based on its name and implementation, it's designed for include paths. This could lead to confusion and incorrect error messages that reference 'include paths' when dealing with library paths.
Copilot uses AI. Check for mistakes.
| @@ -1198,8 +1225,7 @@ impl Config { | |||
| fn ucx_header(&self) -> Result<String, ConfigError> { | |||
| let sdk_version = utils::detect_windows_sdk_version(&self.wdk_content_root)?; | |||
| let ucx_header_root_dir = self.sdk_library_path(sdk_version)?.join("ucx"); | |||
| let max_version = utils::find_max_version_in_directory(&ucx_header_root_dir) | |||
| .map_err(|e| ConfigError::HeaderNotFound("ucxclass.h".into(), e))?; | |||
| let max_version = utils::find_max_version_in_directory(&ucx_header_root_dir)?; | |||
There was a problem hiding this comment.
The error handling for find_max_version_in_directory has been simplified, but this removes the specific context that this is looking for the 'ucxclass.h' header file. The original error provided better diagnostics for header file lookup failures.
Copilot uses AI. Check for mistakes.
This pull request introduces a new, dedicated error type for I/O operations in the
wdk-buildcrate, providing richer context for filesystem errors. The changes refactor the codebase to use this new error type (IoErrorandIoErrorMetadata) throughout, improving error reporting and debugging. Additionally, all filesystem operations that can fail now capture and propagate detailed path metadata, enhancing traceability.Error handling improvements:
IoErrorandIoErrorMetadatatypes inlib.rsto wrapstd::io::Errorwith detailed path context, and updatedConfigErrorand related APIs to use them for all I/O error cases. [1] [2]canonicalize,copy,create_dir,symlink_file,read_link,read_dir) incargo_make.rs,lib.rs,utils.rs, andbuild.rsto use the new error types, ensuring errors now include relevant path metadata. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]API and codebase consistency:
IoErrorandIoErrorMetadatainstead of rawstd::io::Error, ensuring consistent error handling. [1] [2] [3]These changes significantly improve the observability and maintainability of error handling in the build system, especially for filesystem-related failures.