refactor: change from global error enum to anyhow #218
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This changes to use (anyhow)[https://docs.rs/anyhow/latest/anyhow/] instead of a "global" error enum. This does not fully tackle adding context to places where errors happens, but more replace on places where it was straightforward to change one for the other. As a follow-up, should go on places that we currently have `.unwrap()`, `.expect()` and some `?` to add `.context()/.with_context()` to them.
ostenbom
approved these changes
Mar 27, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the error handling throughout the CLI by replacing the custom global error enum with anyhow for improved context and error propagation. Key changes include:
- Replacing custom error types (CliError) with anyhow::Result and context calls.
- Updating functions in multiple modules to return anyhow::Result.
- Adjusting error propagation in external command invocations and file operations.
Reviewed Changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| linkup-cli/src/services/cloudflare_tunnel.rs | Updated function signatures to use Result from anyhow and removed the type parameter from BackgroundService. |
| linkup-cli/src/main.rs | Refactored InstallationMethod::current and linkup_exe_path to return anyhow::Result with context messages. |
| linkup-cli/src/local_config.rs | Converted load, save, and config_path functions to use anyhow context and updated error propagation. |
| linkup-cli/src/env_files.rs | Switched error handling to use anyhow::Context in file operations. |
| linkup-cli/src/commands/update.rs | Adjusted update and update_command functions to return anyhow::Result. |
| linkup-cli/src/commands/uninstall.rs | Simplified signature and error handling for uninstall. |
| linkup-cli/src/commands/stop.rs | Updated error propagation and directory operations with anyhow context. |
| linkup-cli/src/commands/status.rs | Changed LocalState loading to use anyhow context. |
| linkup-cli/src/commands/start.rs | Updated multiple error handling paths to use anyhow::Result and propagated errors without boxing. |
| linkup-cli/src/commands/server.rs | Refactored function signature to support anyhow::Result. |
| linkup-cli/src/commands/reset.rs | Converted reset function signature to return anyhow::Result. |
| linkup-cli/src/commands/remote.rs | Updated remote command to use anyhow for error context. |
| linkup-cli/src/commands/preview.rs | Refactored preview function signatures and error messages to rely on anyhow. |
| linkup-cli/src/commands/local_dns.rs | Modified resolver and DNS cache operations using .context for improved error messages. |
| linkup-cli/src/commands/local.rs | Updated local command to use anyhow for error handling. |
| linkup-cli/src/commands/health.rs | Converted health-related functions to use anyhow::Result and simplified session handling. |
| linkup-cli/src/commands/deploy/* | Adapted deploy/destroy commands to use anyhow::Result, removing references to CliError. |
| linkup-cli/src/commands/completion.rs | Updated command completion signature to return anyhow::Result. |
| linkup-cli/Cargo.toml | Added the anyhow dependency. |
Comments suppressed due to low confidence (1)
linkup-cli/src/local_config.rs:230
- The error message contains a typo ('Unalbe'). Please change it to 'Unable'.
.with_context(|| format("Unalbe to resolve absolute path for {path:?}"))?
augustoccesar
added a commit
that referenced
this pull request
Apr 9, 2025
This will be merged to release `3.0.0`. Closes SHIP-2057 ### Changelog: - Drop Caddy as a dependency and use self-signed certificates. - #201 - #207 - #212 - #215 - #217 - Drop dnsmasq as a dependency and use a local Hickory server. - #219 - #224 - Support pre-release (beta) versions based on changes to `next` branch. - #204 - #205 - #211 - #213 - #214 - #221 - Use anyhow for application errors instead of "global" thiserror enum. - #218 - Stop relying on pidfiles for background services - #222 - Move installation script from Bash to Python - #223 - #226 - Improve possible orphans resolution - #225 Thank you @diegomartinrecillas @ludwigbacklund @solveigsg12 and @jauniusmentimeter for beta testing it! ❤️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This changes to use anyhow instead of a "global" error enum. This does not fully tackle adding context to places where errors happens, but more replace on places where it was straightforward to change one for the other.
As a follow-up, should go on places that we currently have
.unwrap(),.expect()and some?to add.context()/.with_context()to them.Difference of output
Before these changes, running
linkup statusbefore a state exists (runninglinkup start) would result on this error:With these changes, this is the error:
Closes SHIP-1866
Related to SHIP-2057