Skip to content

Commit f39c872

Browse files
authored
feat: improve file parsing error display (#123)
# Improve error handling and display in Vite crates Before: ![image.png](https://app.graphite.com/user-attachments/assets/e99275cc-4143-4ca9-b187-6da75e7e22a5.png) After: ![image.png](https://app.graphite.com/user-attachments/assets/04b07ca4-6edc-4862-b2db-cad91c303ed2.png) This PR enhances error handling and display across several Vite crates: 1. Simplifies `AbsolutePath` debug implementation 2. Improves error messages with better context: - Adds file paths to JSON/YAML parsing errors - Enhances task call stack display logic - Makes error messages more user-friendly 3. Introduces `FileWithPath` to bundle file handles with their paths for better error context 4. Updates error types to include more specific information about failures These changes make debugging easier by providing more context when errors occur, especially for file parsing operations.
1 parent 348e914 commit f39c872

7 files changed

Lines changed: 127 additions & 56 deletions

File tree

crates/vite_path/src/absolute/mod.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,7 @@ impl AsRef<Self> for AbsolutePath {
2727

2828
impl Debug for AbsolutePath {
2929
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
30-
let mut debug_tuple = f.debug_tuple("AbsolutePath");
31-
#[cfg(feature = "absolute-redaction")]
32-
if let Some(redacted_path) = self.try_redact().unwrap() {
33-
debug_tuple.field(&redacted_path);
34-
return debug_tuple.finish();
35-
}
36-
debug_tuple.field(&&self.0);
37-
debug_tuple.finish()
30+
Debug::fmt(&self.0, f)
3831
}
3932
}
4033

crates/vite_task_graph/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl vite_graph_ser::GetKey for TaskNode {
8888

8989
#[derive(Debug, thiserror::Error)]
9090
pub enum TaskGraphLoadError {
91-
#[error("Failed to load package graph: {0}")]
91+
#[error("Failed to load package graph")]
9292
PackageGraphLoadError(#[from] vite_workspace::Error),
9393

9494
#[error("Failed to load task config file for package at {package_path:?}")]

crates/vite_task_plan/src/context.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,14 @@ pub struct TaskCallStackDisplay {
6464
frames: Arc<[TaskCallStackFrameDisplay]>,
6565
}
6666

67+
impl TaskCallStackDisplay {
68+
pub fn is_empty(&self) -> bool {
69+
self.frames.is_empty()
70+
}
71+
}
72+
6773
impl Display for TaskCallStackDisplay {
6874
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
69-
if self.frames.is_empty() {
70-
write!(f, "<empty>")?;
71-
return Ok(());
72-
}
7375
for (i, frame) in self.frames.iter().enumerate() {
7476
if i > 0 {
7577
write!(f, " -> ")?;

crates/vite_task_plan/src/error.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,23 @@ pub enum TaskPlanErrorKind {
126126
}
127127

128128
#[derive(Debug, thiserror::Error)]
129-
#[error("Failed to plan execution, task call stack: {task_call_stack}")]
130129
pub struct Error {
131130
task_call_stack: TaskCallStackDisplay,
132131

133132
#[source]
134133
kind: TaskPlanErrorKind,
135134
}
136135

136+
impl Display for Error {
137+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
138+
write!(f, "Failed to plan execution")?;
139+
if !self.task_call_stack.is_empty() {
140+
write!(f, ", task call stack: {}", self.task_call_stack)?
141+
}
142+
Ok(())
143+
}
144+
}
145+
137146
impl TaskPlanErrorKind {
138147
pub fn with_empty_call_stack(self) -> Error {
139148
Error { task_call_stack: TaskCallStackDisplay::default(), kind: self }

crates/vite_workspace/src/error.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use std::{io, path::Path};
1+
use std::{io, path::Path, sync::Arc};
22

33
use vite_path::{
4-
AbsolutePathBuf, RelativePathBuf, absolute::StripPrefixError, relative::InvalidPathDataError,
4+
AbsolutePath, AbsolutePathBuf, RelativePathBuf, absolute::StripPrefixError,
5+
relative::InvalidPathDataError,
56
};
67
use vite_str::Str;
78

@@ -14,7 +15,7 @@ pub enum Error {
1415
PackageJsonNotFound(AbsolutePathBuf),
1516

1617
#[error("Package at `{package_path:?}` is outside workspace root `{workspace_root:?}`")]
17-
PackageOutsideWorkspace { package_path: AbsolutePathBuf, workspace_root: AbsolutePathBuf },
18+
PackageOutsideWorkspace { package_path: Arc<AbsolutePath>, workspace_root: Arc<AbsolutePath> },
1819

1920
#[error(
2021
"The stripped path ({stripped_path:?}) is not a valid relative path because: {invalid_path_data_error}"
@@ -25,11 +26,19 @@ pub enum Error {
2526
#[error(transparent)]
2627
Io(#[from] io::Error),
2728

28-
#[error(transparent)]
29-
Serde(#[from] serde_json::Error),
30-
31-
#[error(transparent)]
32-
SerdeYml(#[from] serde_yml::Error),
29+
#[error("Failed to parse JSON file at {file_path:?}")]
30+
SerdeJson {
31+
file_path: Arc<AbsolutePath>,
32+
#[source]
33+
serde_json_error: serde_json::Error,
34+
},
35+
36+
#[error("Failed to parse YAML file at {file_path:?}")]
37+
SerdeYml {
38+
file_path: Arc<AbsolutePath>,
39+
#[source]
40+
serde_yml_error: serde_yml::Error,
41+
},
3342

3443
#[error(transparent)]
3544
WaxBuild(#[from] wax::BuildError),

crates/vite_workspace/src/lib.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ use wax::Glob;
1616
pub use crate::{
1717
error::Error,
1818
package::{DependencyType, PackageJson},
19-
package_manager::{WorkspaceFile, WorkspaceRoot, find_package_root, find_workspace_root},
19+
package_manager::{
20+
FileWithPath, WorkspaceFile, WorkspaceRoot, find_package_root, find_workspace_root,
21+
},
2022
};
2123

2224
/// The workspace configuration for pnpm.
@@ -202,17 +204,29 @@ pub fn load_package_graph(
202204
) -> Result<DiGraph<PackageInfo, DependencyType, PackageIx>, Error> {
203205
let mut graph_builder = PackageGraphBuilder::default();
204206
let workspaces = match &workspace_root.workspace_file {
205-
WorkspaceFile::PnpmWorkspaceYaml(file) => {
206-
let workspace: PnpmWorkspace = serde_yml::from_reader(file)?;
207+
WorkspaceFile::PnpmWorkspaceYaml(file_with_path) => {
208+
let workspace: PnpmWorkspace =
209+
serde_yml::from_reader(file_with_path.file()).map_err(|e| Error::SerdeYml {
210+
file_path: Arc::clone(file_with_path.path()),
211+
serde_yml_error: e,
212+
})?;
207213
workspace.packages
208214
}
209-
WorkspaceFile::NpmWorkspaceJson(file) => {
210-
let workspace: NpmWorkspace = serde_json::from_reader(file)?;
215+
WorkspaceFile::NpmWorkspaceJson(file_with_path) => {
216+
let workspace: NpmWorkspace =
217+
serde_json::from_reader(file_with_path.file()).map_err(|e| Error::SerdeJson {
218+
file_path: Arc::clone(file_with_path.path()),
219+
serde_json_error: e,
220+
})?;
211221
workspace.workspaces
212222
}
213-
WorkspaceFile::NonWorkspacePackage(file) => {
223+
WorkspaceFile::NonWorkspacePackage(file_with_path) => {
214224
// For non-workspace packages, add the package.json to the graph as a root package
215-
let package_json: PackageJson = serde_json::from_reader(file)?;
225+
let package_json: PackageJson = serde_json::from_reader(file_with_path.file())
226+
.map_err(|e| Error::SerdeJson {
227+
file_path: Arc::clone(file_with_path.path()),
228+
serde_json_error: e,
229+
})?;
216230
graph_builder.add_package(
217231
RelativePathBuf::default(),
218232
Arc::clone(&workspace_root.path),
@@ -226,12 +240,16 @@ pub fn load_package_graph(
226240
let member_globs = WorkspaceMemberGlobs::new(workspaces);
227241
let mut has_root_package = false;
228242
for package_json_path in member_globs.get_package_json_paths(&*workspace_root.path)? {
229-
let package_json: PackageJson = serde_json::from_slice(&fs::read(&package_json_path)?)?;
243+
let package_json_path: Arc<AbsolutePath> = package_json_path.clone().into();
244+
let package_json: PackageJson =
245+
serde_json::from_slice(&fs::read(&*package_json_path)?).map_err(|e| {
246+
Error::SerdeJson { file_path: Arc::clone(&package_json_path), serde_json_error: e }
247+
})?;
230248
let absolute_path = package_json_path.parent().unwrap();
231249
let Some(package_path) = absolute_path.strip_prefix(&*workspace_root.path)? else {
232250
return Err(Error::PackageOutsideWorkspace {
233251
package_path: package_json_path,
234-
workspace_root: workspace_root.path.to_absolute_path_buf(),
252+
workspace_root: Arc::clone(&workspace_root.path),
235253
});
236254
};
237255

@@ -242,8 +260,11 @@ pub fn load_package_graph(
242260
if !has_root_package {
243261
let package_json_path = workspace_root.path.join("package.json");
244262
match fs::read(&package_json_path) {
245-
Ok(package_json) => {
246-
let package_json: PackageJson = serde_json::from_slice(&package_json)?;
263+
Ok(content) => {
264+
let package_json_path: Arc<AbsolutePath> = package_json_path.into();
265+
let package_json: PackageJson = serde_json::from_slice(&content).map_err(|e| {
266+
Error::SerdeJson { file_path: package_json_path, serde_json_error: e }
267+
})?;
247268
graph_builder.add_package(
248269
RelativePathBuf::default(),
249270
Arc::clone(&workspace_root.path),

crates/vite_workspace/src/package_manager.rs

Lines changed: 60 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,58 @@
11
use std::{
22
fs::File,
33
io::{BufReader, Seek, SeekFrom},
4-
path::Path,
54
sync::Arc,
65
};
76

87
use vite_path::{AbsolutePath, RelativePathBuf};
98

109
use crate::Error;
1110

11+
/// A file handle bundled with its absolute path for error context.
12+
#[derive(Debug)]
13+
pub struct FileWithPath {
14+
file: File,
15+
path: Arc<AbsolutePath>,
16+
}
17+
18+
impl FileWithPath {
19+
/// Open a file at the given path.
20+
pub fn open(path: Arc<AbsolutePath>) -> Result<Self, Error> {
21+
let file = File::open(&*path)?;
22+
Ok(Self { file, path })
23+
}
24+
25+
/// Try to open a file, returning None if it doesn't exist.
26+
pub fn open_if_exists(path: Arc<AbsolutePath>) -> Result<Option<Self>, Error> {
27+
match File::open(&*path) {
28+
Ok(file) => Ok(Some(Self { file, path })),
29+
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
30+
Err(e) => Err(e.into()),
31+
}
32+
}
33+
34+
/// Get a reference to the file handle.
35+
pub fn file(&self) -> &File {
36+
&self.file
37+
}
38+
39+
/// Get a mutable reference to the file handle.
40+
pub fn file_mut(&mut self) -> &mut File {
41+
&mut self.file
42+
}
43+
44+
/// Get the file path.
45+
pub fn path(&self) -> &Arc<AbsolutePath> {
46+
&self.path
47+
}
48+
}
49+
1250
/// The package root directory and its package.json file.
1351
#[derive(Debug)]
1452
pub struct PackageRoot<'a> {
1553
pub path: &'a AbsolutePath,
1654
pub cwd: RelativePathBuf,
17-
pub package_json: File,
55+
pub package_json: FileWithPath,
1856
}
1957

2058
/// Find the package root directory from the current working directory. `original_cwd` must be absolute.
@@ -24,11 +62,12 @@ pub fn find_package_root(original_cwd: &AbsolutePath) -> Result<PackageRoot<'_>,
2462
let mut cwd = original_cwd;
2563
loop {
2664
// Check for package.json
27-
if let Some(file) = open_exists_file(cwd.join("package.json"))? {
65+
let package_json_path: Arc<AbsolutePath> = cwd.join("package.json").into();
66+
if let Some(file_with_path) = FileWithPath::open_if_exists(package_json_path)? {
2867
return Ok(PackageRoot {
2968
path: cwd,
3069
cwd: original_cwd.strip_prefix(cwd)?.expect("cwd must be within the package root"),
31-
package_json: file,
70+
package_json: file_with_path,
3271
});
3372
}
3473

@@ -50,11 +89,11 @@ pub fn find_package_root(original_cwd: &AbsolutePath) -> Result<PackageRoot<'_>,
5089
#[derive(Debug)]
5190
pub enum WorkspaceFile {
5291
/// The pnpm-workspace.yaml file of a pnpm workspace.
53-
PnpmWorkspaceYaml(File),
92+
PnpmWorkspaceYaml(FileWithPath),
5493
/// The package.json file of a yarn/npm workspace.
55-
NpmWorkspaceJson(File),
94+
NpmWorkspaceJson(FileWithPath),
5695
/// The package.json file of a non-workspace package.
57-
NonWorkspacePackage(File),
96+
NonWorkspacePackage(FileWithPath),
5897
}
5998

6099
/// The workspace root directory and its workspace file.
@@ -82,31 +121,38 @@ pub fn find_workspace_root(
82121

83122
loop {
84123
// Check for pnpm-workspace.yaml for pnpm workspace
85-
if let Some(file) = open_exists_file(cwd.join("pnpm-workspace.yaml"))? {
124+
let pnpm_workspace_path: Arc<AbsolutePath> = cwd.join("pnpm-workspace.yaml").into();
125+
if let Some(file_with_path) = FileWithPath::open_if_exists(pnpm_workspace_path)? {
86126
let relative_cwd =
87127
original_cwd.strip_prefix(cwd)?.expect("cwd must be within the pnpm workspace");
88128
return Ok((
89129
WorkspaceRoot {
90130
path: Arc::from(cwd),
91-
workspace_file: WorkspaceFile::PnpmWorkspaceYaml(file),
131+
workspace_file: WorkspaceFile::PnpmWorkspaceYaml(file_with_path),
92132
},
93133
relative_cwd,
94134
));
95135
}
96136

97137
// Check for package.json with workspaces field for npm/yarn workspace
98-
let package_json_path = cwd.join("package.json");
99-
if let Some(mut file) = open_exists_file(&package_json_path)? {
100-
let package_json: serde_json::Value = serde_json::from_reader(BufReader::new(&file))?;
138+
let package_json_path: Arc<AbsolutePath> = cwd.join("package.json").into();
139+
if let Some(mut file_with_path) = FileWithPath::open_if_exists(package_json_path)? {
140+
let package_json: serde_json::Value =
141+
serde_json::from_reader(BufReader::new(file_with_path.file())).map_err(|e| {
142+
Error::SerdeJson {
143+
file_path: Arc::clone(file_with_path.path()),
144+
serde_json_error: e,
145+
}
146+
})?;
101147
if package_json.get("workspaces").is_some() {
102148
// Reset the file cursor since we consumed it reading
103-
file.seek(SeekFrom::Start(0))?;
149+
file_with_path.file_mut().seek(SeekFrom::Start(0))?;
104150
let relative_cwd =
105151
original_cwd.strip_prefix(cwd)?.expect("cwd must be within the workspace");
106152
return Ok((
107153
WorkspaceRoot {
108154
path: Arc::from(cwd),
109-
workspace_file: WorkspaceFile::NpmWorkspaceJson(file),
155+
workspace_file: WorkspaceFile::NpmWorkspaceJson(file_with_path),
110156
},
111157
relative_cwd,
112158
));
@@ -129,12 +175,3 @@ pub fn find_workspace_root(
129175
}
130176
}
131177
}
132-
133-
fn open_exists_file(path: impl AsRef<Path>) -> Result<Option<File>, Error> {
134-
match File::open(path) {
135-
Ok(file) => Ok(Some(file)),
136-
// if the file does not exist, return None
137-
Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
138-
Err(e) => Err(e.into()),
139-
}
140-
}

0 commit comments

Comments
 (0)