Skip to content

Commit f1652fc

Browse files
liangmiQwQfengmk2
liangmiQwQ
and
fengmk2
authored
fix: correct error type mismatch (#1934)
In codebase, `ErrorConfig` seems overused a lot. There are a lot of errors should not be wrapped and should be other error types. It may be created by mistake, and be treated as a project standard by contributors and agents and copied over and over again. This PR corrects them. Co-authored-by: MK (fengmk2) <fengmk2@gmail.com>
1 parent 8f58ac9 commit f1652fc

17 files changed

Lines changed: 51 additions & 83 deletions

File tree

‎crates/vite_global_cli/src/cli.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -816,9 +816,7 @@ async fn managed_update(
816816
}
817817

818818
async fn get_current_node_version() -> Result<String, Error> {
819-
let cwd = vite_path::current_dir().map_err(|error| {
820-
Error::ConfigError(format!("Cannot get current directory: {error}").into())
821-
})?;
819+
let cwd = vite_path::current_dir()?;
822820
Ok(resolve_version(&cwd).await?.version)
823821
}
824822

‎crates/vite_global_cli/src/commands/env/list.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ fn compare_versions(a: &str, b: &str) -> Ordering {
5252

5353
/// Execute the list command (local installed versions).
5454
pub async fn execute(cwd: AbsolutePathBuf, json_output: bool) -> Result<ExitStatus, Error> {
55-
let home_dir =
56-
vite_shared::get_vp_home().map_err(|e| Error::ConfigError(format!("{e}").into()))?;
55+
let home_dir = vite_shared::get_vp_home()?;
5756
let node_dir = home_dir.join("js_runtime").join("node");
5857

5958
let versions = list_installed_versions(node_dir.as_path());

‎crates/vite_global_cli/src/commands/env/mod.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,14 @@ pub async fn execute(cwd: AbsolutePathBuf, args: EnvArgs) -> Result<ExitStatus,
8080
crate::cli::EnvSubcommands::Uninstall { version } => {
8181
let provider = vite_js_runtime::NodeProvider::new();
8282
let resolved = config::resolve_version_alias(&version, &provider).await?;
83-
let home_dir = vite_shared::get_vp_home()
84-
.map_err(|e| crate::error::Error::ConfigError(format!("{e}").into()))?;
83+
let home_dir = vite_shared::get_vp_home()?;
8584
let version_dir = home_dir.join("js_runtime").join("node").join(&resolved);
8685
if !version_dir.as_path().exists() {
8786
eprintln!("Node.js v{} is not installed", resolved);
8887
return Ok(exit_status(1));
8988
}
9089
tokio::fs::remove_dir_all(version_dir.as_path()).await.map_err(|e| {
91-
crate::error::Error::ConfigError(
90+
crate::error::Error::Other(
9291
format!("Failed to remove Node.js v{}: {}", resolved, e).into(),
9392
)
9493
})?;

‎crates/vite_global_cli/src/commands/env/package_metadata.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,9 +86,7 @@ impl PackageMetadata {
8686
return Ok(None);
8787
}
8888
let content = tokio::fs::read_to_string(&path).await?;
89-
let metadata: Self = serde_json::from_str(&content).map_err(|e| {
90-
Error::ConfigError(format!("Failed to parse package metadata: {e}").into())
91-
})?;
89+
let metadata: Self = serde_json::from_str(&content).map_err(Error::JsonError)?;
9290
Ok(Some(metadata))
9391
}
9492

@@ -100,9 +98,7 @@ impl PackageMetadata {
10098
tokio::fs::create_dir_all(parent).await?;
10199
}
102100

103-
let content = serde_json::to_string_pretty(self).map_err(|e| {
104-
Error::ConfigError(format!("Failed to serialize package metadata: {e}").into())
105-
})?;
101+
let content = serde_json::to_string_pretty(self).map_err(Error::JsonError)?;
106102
tokio::fs::write(&path, content).await?;
107103
Ok(())
108104
}

‎crates/vite_global_cli/src/commands/env/pin.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ async fn do_pin(
155155
}
156156
PinTarget::DevEngines => {
157157
if !package_json_exists {
158-
return Err(Error::ConfigError(
158+
return Err(Error::Other(
159159
format!(
160160
"cannot pin to devEngines: no {} in {}",
161161
PACKAGE_JSON_FILE,
@@ -398,7 +398,7 @@ async fn write_dev_engines_node_version(cwd: &AbsolutePathBuf, version: &str) ->
398398
let updated = vite_shared::edit_json_object(&content, |obj| {
399399
set_dev_engines_runtime_node(obj, version);
400400
})
401-
.map_err(|e| Error::ConfigError(format!("failed to update package.json: {e}").into()))?;
401+
.map_err(|e| Error::Other(format!("failed to update package.json: {e}").into()))?;
402402
tokio::fs::write(&package_json_path, updated).await?;
403403
Ok(())
404404
}
@@ -493,7 +493,7 @@ async fn remove_dev_engines_runtime_node(cwd: &AbsolutePathBuf) -> Result<bool,
493493
obj.remove("devEngines");
494494
}
495495
})
496-
.map_err(|e| Error::ConfigError(format!("failed to update package.json: {e}").into()))?;
496+
.map_err(|e| Error::Other(format!("failed to update package.json: {e}").into()))?;
497497

498498
if removed {
499499
tokio::fs::write(&package_json_path, updated).await?;

‎crates/vite_global_cli/src/commands/env/setup.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ pub async fn execute(refresh: bool, env_only: bool) -> Result<ExitStatus, Error>
8484
tokio::fs::create_dir_all(&bin_dir).await?;
8585

8686
// Get the current executable path (for shims)
87-
let current_exe = std::env::current_exe()
88-
.map_err(|e| Error::ConfigError(format!("Cannot find current executable: {e}").into()))?;
87+
let current_exe = std::env::current_exe()?;
8988

9089
// Create wrapper script in bin/
9190
setup_vp_wrapper(&current_exe, &bin_dir, refresh).await?;
@@ -328,7 +327,7 @@ async fn create_unix_shim(
328327
tool: &str,
329328
) -> Result<(), Error> {
330329
let bin_dir = shim_path.parent().ok_or_else(|| {
331-
Error::ConfigError(format!("Cannot find parent directory for {tool} shim").into())
330+
Error::Other(format!("Cannot find parent directory for {tool} shim").into())
332331
})?;
333332
let target = resolve_unix_vp_shim_target(source, bin_dir).await?;
334333
tokio::fs::symlink(&target, shim_path).await?;
@@ -414,19 +413,18 @@ pub(crate) fn get_trampoline_path() -> Result<vite_path::AbsolutePathBuf, Error>
414413
let path = std::path::PathBuf::from(override_path);
415414
if path.exists() {
416415
return vite_path::AbsolutePathBuf::new(path)
417-
.ok_or_else(|| Error::ConfigError("Invalid trampoline override path".into()));
416+
.ok_or_else(|| Error::Other("Invalid trampoline override path".into()));
418417
}
419418
}
420419

421-
let current_exe = std::env::current_exe()
422-
.map_err(|e| Error::ConfigError(format!("Cannot find current executable: {e}").into()))?;
420+
let current_exe = std::env::current_exe()?;
423421
let bin_dir = current_exe
424422
.parent()
425-
.ok_or_else(|| Error::ConfigError("Cannot find parent directory of vp.exe".into()))?;
423+
.ok_or_else(|| Error::Other("Cannot find parent directory of vp.exe".into()))?;
426424
let trampoline = bin_dir.join("vp-shim.exe");
427425

428426
if !trampoline.exists() {
429-
return Err(Error::ConfigError(
427+
return Err(Error::Other(
430428
format!(
431429
"Trampoline binary not found at {}. Re-install vite-plus to fix this.",
432430
trampoline.display()
@@ -436,7 +434,7 @@ pub(crate) fn get_trampoline_path() -> Result<vite_path::AbsolutePathBuf, Error>
436434
}
437435

438436
vite_path::AbsolutePathBuf::new(trampoline)
439-
.ok_or_else(|| Error::ConfigError("Invalid trampoline path".into()))
437+
.ok_or_else(|| Error::Other("Invalid trampoline path".into()))
440438
}
441439

442440
/// Try to delete an `.exe` file; if deletion fails (e.g., file is locked by a

‎crates/vite_global_cli/src/commands/env/use.rs‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,8 @@ pub async fn execute(
137137

138138
// Ensure version is installed (unless --no-install)
139139
if !no_install {
140-
let home_dir = vite_shared::get_vp_home()
141-
.map_err(|e| Error::ConfigError(format!("{e}").into()))?
142-
.join("js_runtime")
143-
.join("node")
144-
.join(&resolved_version);
140+
let home_dir =
141+
vite_shared::get_vp_home()?.join("js_runtime").join("node").join(&resolved_version);
145142

146143
#[cfg(windows)]
147144
let binary_path = home_dir.join("node.exe");

‎crates/vite_global_cli/src/commands/env/which.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,13 @@ fn locate_package_binary(package_name: &str, binary_name: &str) -> Result<Absolu
229229
let package_json_path = node_modules_dir.join("package.json");
230230

231231
if !package_json_path.as_path().exists() {
232-
return Err(Error::ConfigError(format!("Package {} not found", package_name).into()));
232+
return Err(Error::Other(format!("Package {} not found", package_name).into()));
233233
}
234234

235235
// Read package.json to find the binary path
236236
let content = std::fs::read_to_string(package_json_path.as_path())?;
237237
let package_json: serde_json::Value = serde_json::from_str(&content)
238-
.map_err(|e| Error::ConfigError(format!("Failed to parse package.json: {e}").into()))?;
238+
.map_err(|e| Error::Other(format!("Failed to parse package.json: {e}").into()))?;
239239

240240
let binary_path = match package_json.get("bin") {
241241
Some(serde_json::Value::String(path)) => {
@@ -245,7 +245,7 @@ fn locate_package_binary(package_name: &str, binary_name: &str) -> Result<Absolu
245245
if expected_name == binary_name {
246246
node_modules_dir.join(path)
247247
} else {
248-
return Err(Error::ConfigError(
248+
return Err(Error::Other(
249249
format!("Binary {} not found in package", binary_name).into(),
250250
));
251251
}
@@ -255,13 +255,13 @@ fn locate_package_binary(package_name: &str, binary_name: &str) -> Result<Absolu
255255
if let Some(serde_json::Value::String(path)) = map.get(binary_name) {
256256
node_modules_dir.join(path)
257257
} else {
258-
return Err(Error::ConfigError(
258+
return Err(Error::Other(
259259
format!("Binary {} not found in package", binary_name).into(),
260260
));
261261
}
262262
}
263263
_ => {
264-
return Err(Error::ConfigError(
264+
return Err(Error::Other(
265265
format!("No bin field in package.json for {}", package_name).into(),
266266
));
267267
}

‎crates/vite_global_cli/src/commands/global/install.rs‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,7 @@ pub async fn install(
129129
// Resolve from current directory
130130
let cwd = match current_dir() {
131131
Ok(cwd) => cwd,
132-
Err(error) => {
133-
let error =
134-
Error::ConfigError(format!("Cannot get current directory: {}", error).into());
135-
return Err((None, error));
136-
}
132+
Err(error) => return Err((None, error.into())),
137133
};
138134
let resolution = match resolve_version(&cwd).await {
139135
Ok(resolution) => resolution,
@@ -535,7 +531,7 @@ async fn install_one(
535531
}
536532
let _ = std::io::stderr().write_all(&output.stderr);
537533
cleanup_failed_install(package_name, backup).await?;
538-
return Err(Error::ConfigError(
534+
return Err(Error::Other(
539535
format!("npm install failed with exit code: {:?}", output.status.code()).into(),
540536
));
541537
}
@@ -545,7 +541,7 @@ async fn install_one(
545541

546542
if !tokio::fs::try_exists(&package_json_path).await.unwrap_or(false) {
547543
cleanup_failed_install(package_name, backup).await?;
548-
return Err(Error::ConfigError(
544+
return Err(Error::Other(
549545
format!(
550546
"Package was not installed correctly, package.json not found at {}",
551547
package_json_path.as_path().display()
@@ -565,9 +561,7 @@ async fn install_one(
565561
Ok(package_json) => package_json,
566562
Err(error) => {
567563
cleanup_failed_install(package_name, backup).await?;
568-
return Err(Error::ConfigError(
569-
format!("Failed to parse package.json: {error}").into(),
570-
));
564+
return Err(Error::Other(format!("Failed to parse package.json: {error}").into()));
571565
}
572566
};
573567

@@ -648,7 +642,7 @@ fn unique_backup_dir(package_name: &str) -> Result<AbsolutePathBuf, Error> {
648642
backup_path.set_file_name(backup_name);
649643

650644
AbsolutePathBuf::new(backup_path)
651-
.ok_or_else(|| Error::ConfigError("Invalid global package backup path".into()))
645+
.ok_or_else(|| Error::Other("Invalid global package backup path".into()))
652646
}
653647

654648
async fn cleanup_failed_install(
@@ -723,7 +717,7 @@ async fn stale_bin_names_for_package(
723717
pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
724718
if is_local_package_spec(package_name) {
725719
// We can't resolve local packages for uninstall, follow npm's behavior
726-
return Err(Error::ConfigError(
720+
return Err(Error::Other(
727721
format!(
728722
"Local path {} can't be resolved, please enter a package name instead",
729723
package_name
@@ -741,9 +735,7 @@ pub async fn uninstall(package_name: &str, dry_run: bool) -> Result<(), Error> {
741735
// Phase 2: Fallback - scan BinConfig files for orphaned binaries
742736
let orphan_bins = BinConfig::find_by_package(&package_name).await?;
743737
if orphan_bins.is_empty() {
744-
return Err(Error::ConfigError(
745-
format!("Package {} is not installed", package_name).into(),
746-
));
738+
return Err(Error::Other(format!("Package {} is not installed", package_name).into()));
747739
}
748740
orphan_bins
749741
};

‎crates/vite_global_cli/src/commands/global/mod.rs‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@ struct NpmRegistry {
3838

3939
impl NpmRegistry {
4040
async fn resolve() -> Result<Self, Error> {
41-
let cwd = current_dir().map_err(|error| {
42-
Error::ConfigError(format!("Cannot get current directory: {error}").into())
43-
})?;
41+
let cwd = current_dir()?;
4442
let resolution = resolve_version(&cwd).await?;
4543
let runtime = vite_js_runtime::download_runtime(
4644
vite_js_runtime::JsRuntimeType::Node,
@@ -78,9 +76,7 @@ async fn npm_view(
7876

7977
if !output.status.success() {
8078
let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
81-
return Err(Error::ConfigError(
82-
format!("npm view failed for {package_spec}: {stderr}").into(),
83-
));
79+
return Err(Error::Other(format!("npm view failed for {package_spec}: {stderr}").into()));
8480
}
8581

8682
Ok(output.stdout)
@@ -159,7 +155,7 @@ pub(crate) fn parse_package_spec(spec: &str) -> Result<(String, Option<String>),
159155
if is_local_package_spec(spec) {
160156
let package_json = read_local_package_json(spec)?;
161157
let Some(package_name) = package_json.get("name").and_then(|name| name.as_str()) else {
162-
return Err(Error::ConfigError(
158+
return Err(Error::Other(
163159
format!("Local package {spec} must have a string name in package.json").into(),
164160
));
165161
};
@@ -187,13 +183,9 @@ fn resolve_local_package_path(spec: &str) -> Result<AbsolutePathBuf, Error> {
187183
let path = std::path::Path::new(path_spec);
188184
if path.is_absolute() {
189185
AbsolutePathBuf::new(path.to_path_buf())
190-
.ok_or_else(|| Error::ConfigError(format!("Invalid local package path {spec}").into()))
186+
.ok_or_else(|| Error::Other(format!("Invalid local package path {spec}").into()))
191187
} else {
192-
Ok(current_dir()
193-
.map_err(|error| {
194-
Error::ConfigError(format!("Cannot get current directory: {error}").into())
195-
})?
196-
.join(path))
188+
Ok(current_dir()?.join(path))
197189
}
198190
}
199191

@@ -206,7 +198,7 @@ fn read_local_package_json(spec: &str) -> Result<serde_json::Value, Error> {
206198
let package_json_path = package_path.join("package.json");
207199
let package_json_content =
208200
std::fs::read_to_string(package_json_path.as_path()).map_err(|error| {
209-
Error::ConfigError(
201+
Error::Other(
210202
format!(
211203
"Failed to read package.json for local package {spec} at {}: {error}",
212204
package_json_path.as_path().display()
@@ -227,7 +219,7 @@ fn read_package_json_from_tarball(
227219
package_path: &AbsolutePathBuf,
228220
) -> Result<serde_json::Value, Error> {
229221
let file = File::open(package_path.as_path()).map_err(|error| {
230-
Error::ConfigError(
222+
Error::Other(
231223
format!(
232224
"Failed to read package tarball {spec} at {}: {error}",
233225
package_path.as_path().display()
@@ -239,44 +231,42 @@ fn read_package_json_from_tarball(
239231
let mut archive = Archive::new(decoder);
240232

241233
for entry in archive.entries().map_err(|error| {
242-
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
234+
Error::Other(format!("Failed to read package tarball {spec}: {error}").into())
243235
})? {
244236
let mut entry = entry.map_err(|error| {
245-
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
237+
Error::Other(format!("Failed to read package tarball {spec}: {error}").into())
246238
})?;
247239
let path = entry.path().map_err(|error| {
248-
Error::ConfigError(format!("Failed to read package tarball {spec}: {error}").into())
240+
Error::Other(format!("Failed to read package tarball {spec}: {error}").into())
249241
})?;
250242
if path.as_ref() != std::path::Path::new("package/package.json") {
251243
continue;
252244
}
253245

254246
let mut package_json_content = String::new();
255247
entry.read_to_string(&mut package_json_content).map_err(|error| {
256-
Error::ConfigError(
248+
Error::Other(
257249
format!("Failed to read package.json from package tarball {spec}: {error}").into(),
258250
)
259251
})?;
260252
return serde_json::from_str(&package_json_content).map_err(Error::JsonError);
261253
}
262254

263-
Err(Error::ConfigError(
264-
format!("Package tarball {spec} must contain package/package.json").into(),
265-
))
255+
Err(Error::Other(format!("Package tarball {spec} must contain package/package.json").into()))
266256
}
267257

268258
fn parse_npm_view_version(stdout: &[u8]) -> Result<String, Error> {
269259
let raw = String::from_utf8_lossy(stdout);
270260
let trimmed = raw.trim();
271261
if trimmed.is_empty() {
272-
return Err(Error::ConfigError("npm view returned an empty version".into()));
262+
return Err(Error::Other("npm view returned an empty version".into()));
273263
}
274264

275265
match serde_json::from_str::<serde_json::Value>(trimmed) {
276266
Ok(serde_json::Value::String(version)) => Ok(version),
277267
Ok(serde_json::Value::Array(versions)) => {
278268
let Some(version) = versions.iter().rev().find_map(|version| version.as_str()) else {
279-
return Err(Error::ConfigError("npm view returned an empty version list".into()));
269+
return Err(Error::Other("npm view returned an empty version list".into()));
280270
};
281271
Ok(version.to_string())
282272
}

0 commit comments

Comments
 (0)