Skip to content

Commit 179b6a6

Browse files
authored
refactor: switch to anyhow (#247)
Replace string-based error handling with `anyhow` `Result` type throughout the extension code. This provides more structured error handling with better context propagation and formatting.
1 parent 5a59ddd commit 179b6a6

6 files changed

Lines changed: 102 additions & 85 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ regex = "1.11.1"
1414
serde_json = "1.0"
1515
serde = {version = "1.0", features = ["derive"]}
1616
zed_extension_api = "0.7.0"
17+
anyhow = "1.0.89"
1718

1819
[dev-dependencies]
1920
tree-sitter = "0.25"

src/bundler.rs

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::command_executor::CommandExecutor;
2+
use anyhow::{bail, Context, Result};
23
use std::path::PathBuf;
34

45
/// A simple wrapper around the `bundle` command.
@@ -27,11 +28,7 @@ impl<E: CommandExecutor> Bundler<E> {
2728
///
2829
/// # Returns
2930
/// A `Result` containing the version string if successful, or an error message.
30-
pub fn installed_gem_version(
31-
&self,
32-
name: &str,
33-
envs: &[(&str, &str)],
34-
) -> Result<String, String> {
31+
pub fn installed_gem_version(&self, name: &str, envs: &[(&str, &str)]) -> Result<String> {
3532
let args = &["--version", name];
3633

3734
self.execute_bundle_command("info", args, envs)
@@ -42,11 +39,11 @@ impl<E: CommandExecutor> Bundler<E> {
4239
cmd: &str,
4340
args: &[&str],
4441
envs: &[(&str, &str)],
45-
) -> Result<String, String> {
42+
) -> Result<String> {
4643
let bundle_gemfile_path = self.working_dir.join("Gemfile");
47-
let bundle_gemfile = bundle_gemfile_path
48-
.to_str()
49-
.ok_or_else(|| "Invalid path to Gemfile".to_string())?;
44+
let bundle_gemfile = bundle_gemfile_path.to_str().with_context(|| {
45+
format!("Invalid path to Gemfile: {}", bundle_gemfile_path.display())
46+
})?;
5047

5148
let full_args: Vec<&str> = std::iter::once(cmd).chain(args.iter().copied()).collect();
5249
let command_envs: Vec<(&str, &str)> = envs
@@ -55,21 +52,22 @@ impl<E: CommandExecutor> Bundler<E> {
5552
.chain(std::iter::once(("BUNDLE_GEMFILE", bundle_gemfile)))
5653
.collect();
5754

58-
self.command_executor
55+
let output = self
56+
.command_executor
5957
.execute("bundle", &full_args, &command_envs)
60-
.and_then(|output| match output.status {
61-
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).into_owned()),
62-
Some(status) => {
63-
let stderr = String::from_utf8_lossy(&output.stderr);
64-
Err(format!(
65-
"'bundle' command failed (status: {status})\nError: {stderr}",
66-
))
67-
}
68-
None => {
69-
let stderr = String::from_utf8_lossy(&output.stderr);
70-
Err(format!("Failed to execute 'bundle' command: {stderr}"))
71-
}
72-
})
58+
.map_err(|e| anyhow::anyhow!(e))?;
59+
60+
match output.status {
61+
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).into_owned()),
62+
Some(status) => {
63+
let stderr = String::from_utf8_lossy(&output.stderr);
64+
bail!("'bundle' command failed (status: {status})\nError: {stderr}")
65+
}
66+
None => {
67+
let stderr = String::from_utf8_lossy(&output.stderr);
68+
bail!("Failed to execute 'bundle' command: {stderr}")
69+
}
70+
}
7371
}
7472
}
7573

@@ -214,7 +212,7 @@ mod tests {
214212
result.is_err(),
215213
"Expected error for failed gem version check"
216214
);
217-
let err_msg = result.unwrap_err();
215+
let err_msg = format!("{:#}", result.unwrap_err());
218216
assert!(
219217
err_msg.contains("'bundle' command failed (status: 1)"),
220218
"Error message should contain status"
@@ -246,10 +244,10 @@ mod tests {
246244
let result = bundler.installed_gem_version(gem_name, &[]);
247245

248246
assert!(result.is_err(), "Expected error from executor failure");
249-
assert_eq!(
250-
result.unwrap_err(),
251-
specific_error_msg,
252-
"Error message should match executor error"
247+
let error_message = format!("{:#}", result.unwrap_err());
248+
assert!(
249+
error_message.contains(specific_error_msg),
250+
"Error message should contain executor error"
253251
);
254252
}
255253
}

src/gemset.rs

Lines changed: 43 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::command_executor::CommandExecutor;
2+
use anyhow::{anyhow, bail, Context, Result};
23
use regex::Regex;
34
use std::{
45
collections::hash_map::DefaultHasher,
@@ -11,10 +12,11 @@ pub fn versioned_gem_home(
1112
base_dir: &Path,
1213
envs: &[(&str, &str)],
1314
executor: &dyn CommandExecutor,
14-
) -> Result<PathBuf, String> {
15+
) -> Result<PathBuf> {
1516
let output = executor
1617
.execute("ruby", &["--version"], envs)
17-
.map_err(|e| format!("Failed to detect Ruby version: {e}"))?;
18+
.map_err(|e| anyhow::anyhow!(e))
19+
.context("Failed to detect Ruby version")?;
1820

1921
match output.status {
2022
Some(0) => {
@@ -24,8 +26,8 @@ pub fn versioned_gem_home(
2426
let version_hash = format!("{:x}", hasher.finish());
2527
Ok(base_dir.join("gems").join(version_hash))
2628
}
27-
Some(status) => Err(format!("Ruby version check failed with status {status}")),
28-
None => Err("Failed to execute ruby --version".to_string()),
29+
Some(status) => bail!("Ruby version check failed with status {status}"),
30+
None => bail!("Failed to execute ruby --version"),
2931
}
3032
}
3133

@@ -56,12 +58,12 @@ impl Gemset {
5658
}
5759

5860
/// Returns the full path to a gem binary executable.
59-
pub fn gem_bin_path(&self, bin_name: &str) -> Result<String, String> {
61+
pub fn gem_bin_path(&self, bin_name: &str) -> Result<String> {
6062
let path = self.gem_home.join("bin").join(bin_name);
6163

6264
path.to_str()
6365
.map(ToString::to_string)
64-
.ok_or_else(|| format!("Failed to convert path for '{bin_name}'"))
66+
.with_context(|| format!("Failed to convert path for '{bin_name}'"))
6567
}
6668

6769
pub fn env(&self) -> &[(String, String)] {
@@ -101,7 +103,7 @@ impl Gemset {
101103
})
102104
}
103105

104-
pub fn install_gem(&self, name: &str) -> Result<(), String> {
106+
pub fn install_gem(&self, name: &str) -> Result<()> {
105107
let args = &[
106108
"--no-user-install",
107109
"--no-format-executable",
@@ -110,26 +112,26 @@ impl Gemset {
110112
];
111113

112114
self.execute_gem_command("install", args)
113-
.map_err(|e| format!("Failed to install gem '{name}': {e}"))?;
115+
.with_context(|| format!("Failed to install gem '{name}'"))?;
114116

115117
Ok(())
116118
}
117119

118-
pub fn update_gem(&self, name: &str) -> Result<(), String> {
120+
pub fn update_gem(&self, name: &str) -> Result<()> {
119121
self.execute_gem_command("update", &[name])
120-
.map_err(|e| format!("Failed to update gem '{name}': {e}"))?;
122+
.with_context(|| format!("Failed to update gem '{name}'"))?;
121123
Ok(())
122124
}
123125

124-
pub fn uninstall_gem(&self, name: &str, version: &str) -> Result<(), String> {
126+
pub fn uninstall_gem(&self, name: &str, version: &str) -> Result<()> {
125127
let args = &[name, "--version", version];
126128
self.execute_gem_command("uninstall", args)
127-
.map_err(|e| format!("Failed to uninstall gem '{name}': {e}"))?;
129+
.with_context(|| format!("Failed to uninstall gem '{name}' version {version}"))?;
128130

129131
Ok(())
130132
}
131133

132-
pub fn installed_gem_version(&self, name: &str) -> Result<Option<String>, String> {
134+
pub fn installed_gem_version(&self, name: &str) -> Result<Option<String>> {
133135
static GEM_VERSION_REGEX: LazyLock<Regex> =
134136
LazyLock::new(|| Regex::new(r"^(\S+) \((.+)\)$").unwrap());
135137

@@ -152,23 +154,23 @@ impl Gemset {
152154
Ok(None)
153155
}
154156

155-
pub fn is_outdated_gem(&self, name: &str) -> Result<bool, String> {
157+
pub fn is_outdated_gem(&self, name: &str) -> Result<bool> {
156158
self.execute_gem_command("outdated", &[]).map(|output| {
157159
output
158160
.lines()
159161
.any(|line| line.split_whitespace().next().is_some_and(|n| n == name))
160162
})
161163
}
162164

163-
fn execute_gem_command(&self, cmd: &str, args: &[&str]) -> Result<String, String> {
165+
fn execute_gem_command(&self, cmd: &str, args: &[&str]) -> Result<String> {
164166
let full_args: Vec<&str> = std::iter::once(cmd)
165167
.chain(std::iter::once("--norc"))
166168
.chain(args.iter().copied())
167169
.collect();
168170
let gem_home_str = self
169171
.gem_home
170172
.to_str()
171-
.ok_or("Failed to convert gem_home path to string")?;
173+
.context("Failed to convert gem_home path to string")?;
172174

173175
let command_envs = vec![("GEM_HOME", gem_home_str)];
174176

@@ -177,21 +179,22 @@ impl Gemset {
177179
.chain(self.envs.iter().map(|(k, v)| (k.as_str(), v.as_str())))
178180
.collect();
179181

180-
self.command_executor
182+
let output = self
183+
.command_executor
181184
.execute("gem", &full_args, &merged_envs)
182-
.and_then(|output| match output.status {
183-
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).into_owned()),
184-
Some(status) => {
185-
let stderr = String::from_utf8_lossy(&output.stderr);
186-
Err(format!(
187-
"Gem command failed (status: {status})\nError: {stderr}",
188-
))
189-
}
190-
None => {
191-
let stderr = String::from_utf8_lossy(&output.stderr);
192-
Err(format!("Failed to execute gem command: {stderr}"))
193-
}
194-
})
185+
.map_err(|e| anyhow!(e))?;
186+
187+
match output.status {
188+
Some(0) => Ok(String::from_utf8_lossy(&output.stdout).into_owned()),
189+
Some(status) => {
190+
let stderr = String::from_utf8_lossy(&output.stderr);
191+
bail!("Gem command failed (status: {status})\nError: {stderr}")
192+
}
193+
None => {
194+
let stderr = String::from_utf8_lossy(&output.stderr);
195+
bail!("Failed to execute gem command: {stderr}")
196+
}
197+
}
195198
}
196199
}
197200

@@ -396,9 +399,8 @@ mod tests {
396399

397400
let result = versioned_gem_home(Path::new("/extension"), &[], &executor);
398401
assert!(result.is_err());
399-
assert!(result
400-
.expect_err("should return error")
401-
.contains("Ruby version check failed with status 127"));
402+
let error_message = format!("{:#}", result.expect_err("should return error"));
403+
assert!(error_message.contains("Ruby version check failed with status 127"));
402404
}
403405

404406
#[test]
@@ -413,9 +415,8 @@ mod tests {
413415

414416
let result = versioned_gem_home(Path::new("/extension"), &[], &executor);
415417
assert!(result.is_err());
416-
assert!(result
417-
.expect_err("should return error")
418-
.contains("Failed to detect Ruby version"));
418+
let error_message = format!("{:#}", result.expect_err("should return error"));
419+
assert!(error_message.contains("Failed to detect Ruby version"));
419420
}
420421

421422
#[test]
@@ -534,6 +535,7 @@ mod tests {
534535
assert!(result.is_err());
535536
assert!(result
536537
.unwrap_err()
538+
.to_string()
537539
.contains("Failed to install gem 'ruby-lsp'"));
538540
}
539541

@@ -574,6 +576,7 @@ mod tests {
574576
assert!(result.is_err());
575577
assert!(result
576578
.unwrap_err()
579+
.to_string()
577580
.contains("Failed to update gem 'ruby-lsp'"));
578581
}
579582

@@ -667,6 +670,7 @@ mod tests {
667670
assert!(result.is_err());
668671
assert!(result
669672
.unwrap_err()
673+
.to_string()
670674
.contains("Gem command failed (status: 127)"));
671675
}
672676

@@ -734,6 +738,7 @@ mod tests {
734738
assert!(result.is_err());
735739
assert!(result
736740
.unwrap_err()
741+
.to_string()
737742
.contains("Gem command failed (status: 1)"));
738743
}
739744

@@ -782,6 +787,7 @@ mod tests {
782787
assert!(result.is_err());
783788
assert!(result
784789
.unwrap_err()
790+
.to_string()
785791
.contains("Failed to uninstall gem 'solargraph'"));
786792
}
787793

@@ -800,7 +806,7 @@ mod tests {
800806
let gemset = create_gemset(None, mock_executor);
801807
let result = gemset.uninstall_gem(gem_name, gem_version);
802808
assert!(result.is_err());
803-
let error_message = result.unwrap_err();
809+
let error_message = format!("{:#}", result.unwrap_err());
804810
assert!(error_message.contains("Failed to uninstall gem 'solargraph'"));
805811
assert!(error_message.contains("Command not found: gem"));
806812
}

0 commit comments

Comments
 (0)