refactor(cli): improve errors #14126
Conversation
Package Changes Through 32442c2There are 9 changes which include @tauri-apps/api with minor, tauri-cli with minor, tauri-utils with minor, tauri-runtime-wry with minor, tauri-runtime with minor, tauri with minor, tauri-bundler with minor, @tauri-apps/cli with minor, tauri-macros with patch Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
pretty sure CI will fail here, I only did the macOS part so far |
|
will be paired with tauri-apps/cargo-mobile2#485 on release |
|
Didn't look at the whole thing yet, but I think if we want to actually enforce this, we need a helper function like |
that's a good idea, i wanted to do a second pass on it.. i'm running some prompts now |
| /// Bundler error. | ||
| #[error("{0:#}")] | ||
| BundlerError(#[from] anyhow::Error), |
There was a problem hiding this comment.
Can we revert this and add a todo and change the docs saying that this variant is unused now? There are projects using tauri-bundler and this is a breaking api change.
There was a problem hiding this comment.
otherwise this looks good i think (at least for a start). couldn't test it that much though cause i can't think of many ways to trigger errors on windows. i guess time will tell how good of a start this is :D
| use std::{borrow::Cow, fmt::Display, path::PathBuf}; | ||
|
|
||
| #[derive(Debug, thiserror::Error)] | ||
| pub enum Error { |
There was a problem hiding this comment.
To be honest, I'm a bit concerned about so many errors, this could be quite hard to follow through and find the right error to propagate, and in most cases, CLI errors are just some plain texts explaining what's going on
There was a problem hiding this comment.
most of them are using #[from] except the ones that have context
alternatively we can remove most of the variants here and enforce usage of the context trait (which was the idea here tbh, adding context to every single error out there, but it's impossible to do so using the anyhow crate because it doesn't enforce it, so i had to break compilation for the entire codebase to spot the places that needed it)
There was a problem hiding this comment.
alternatively we can remove most of the variants here and enforce usage of the context trait (which was the idea here tbh, adding context to every single error out there, but it's impossible to do so using the anyhow crate because it doesn't enforce it, so i had to break compilation for the entire codebase to spot the places that needed it)
That sounds much better to me, enforcing an error context is much more useful than having an error saying it is a JsonError (not sure how annoying would that be when writing staffs though 😂, we could try that out anyways)
There was a problem hiding this comment.
pushed some further cleanup; i'd say we should make additional changes in a separate PR 😂
There was a problem hiding this comment.
Looks much better, we could maybe add a few variant and helps like fs_context for some commonly used ones in the future
|
easiest thing you can do is get something to fail 😂 break cargo.toml / tauri.conf.json for instance |
I know this is controversial but I've had a weird "directory not empty" error when building iOS apps that seems to be related to the
gen/apple/buildfolder, but I had to guess it and it's impossible to trace it.. so I decided to dropanyhowto force us to be more explicit about what happened when something goes wrong.This PR: