Refine more errors #696
Merged
Refine more errors #696
Conversation
This file contains 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
- Add `kube::client::AuthError` (`kube::client::auth::Error`)
- Move error variants from `kube::ConfigError` related to client auth
- `InvalidBasicAuth`
- `InvalidBearerToken`
- `UnrefreshableTokenResponse`
- `ExecPluginFailed`
- `MalformedTokenExpirationDate`
- `OAuth`
- Add `AuthError::ReadTokenFile` (was `ConfigError::ReadFile`)
- Add `AuthError::ParseTokenKey` (was `kube::Error::SerdeError`)
- Move `kube::error::OAuthError` to `kube::client::OAuthError`
- Add `OAuthError::BuildRequest` (was `kube::Error::HttpError`)
- Add `OAuthError::ConcatBuffers` (was `kube::Error::HyperError`)
- Change all error under `kube::client::auth` to `kube::client::AuthError`
- Add `kube::Error::Auth(kube::client::AuthError)`
Signed-off-by: kazk <kazk.dev@gmail.com>
- Move and split `kube::error::ConfigError` into in-cluster and kubeconfig - Add `kube::config::InferConfigError` - Add `kube::config::InClusterError` - Add `kube::config::KubeconfigError` - Remove `kube::Error::InvalidUri` - Change `kube::Error::Kubeconfig` to `kube::Error::InferConfig(kube::config::InferConfigError)` - Change errors in `kube::config` to new `kube::config` errors - Refine errors to preserve context Signed-off-by: kazk <kazk.dev@gmail.com>
Signed-off-by: kazk <kazk.dev@gmail.com>
kazk
force-pushed
the
refine-more-errors
branch
from
683860d
to
9bacbcd
Compare
clux
approved these changes
Nov 6, 2021
There was a problem hiding this comment.
That's a huge batch you've tackled there. Looks great. Really starting to see the benefit now.
The previously daunting src/errors.rs file is looking very small now.
kazk
commented
Nov 6, 2021
|
|
||
| /// Failed to exec auth | ||
| #[error("failed exec auth: {0}")] | ||
| AuthExec(String), |
There was a problem hiding this comment.
AuthExec(String) should be broken down, but we can do that later.
In general, we shouldn't have code like Error::AuthExec(format!("result is not a string {:?}", e)).
Comment on lines
+378
to
+388
| fn load_from_base64_or_file<P: AsRef<Path>>( | ||
| value: &Option<String>, | ||
| file: &Option<P>, | ||
| ) -> Result<Vec<u8>, LoadDataError> { | ||
| let data = value | ||
| .as_deref() | ||
| .map(load_from_base64) | ||
| .or_else(|| file.as_ref().map(load_from_file)) | ||
| .unwrap_or(Err(LoadDataError::NoBase64DataOrFile))?; | ||
| Ok(ensure_trailing_newline(data)) | ||
| } |
There was a problem hiding this comment.
The corresponding function looked like the following before this change:
fn data_or_file_with_base64<P: AsRef<Path>>(data: &Option<String>, file: &Option<P>) -> Result<Vec<u8>> {
let mut blob = match (data, file) {
(Some(d), _) => base64::decode(&d)
.map_err(ConfigError::Base64Decode)
.map_err(Error::Kubeconfig),
(_, Some(f)) => read_file(f),
_ => Err(Error::Kubeconfig(ConfigError::NoBase64FileOrData)),
}?;
//Ensure there is a trailing newline in the blob
//Don't bother if the blob is empty
if blob.last().map(|end| *end != b'\n').unwrap_or(false) {
blob.push(b'\n');
}
Ok(blob)
}I changed the error to a separate LoadDataError, so we can describe errors like "decoding base64 failed when loading client key" instead of "decoding base64 failed in kubeconfig".
Changed to .map, .or_else, .unwrap_or(err) because I think it makes the behavior more obvious. Not sure if we should allow both value and file to be Some.
Signed-off-by: kazk <kazk.dev@gmail.com>
kazk
force-pushed
the
refine-more-errors
branch
from
5557837
to
9c4edc7
Compare
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.
Continuing to refine errors for #688.
kube::ConfigErrortokube::client::AuthErrorincludingOAuthError. Change all errors underkube::client::authtoAuthError. Addkube::Error::Auth(AuthError).kube::ConfigErrortokube::configand split intoInferConfigError(error fromConfig::infer),InClusterError(in-cluster config errors),KubeconfigError. No more boxing.kube::configcan be extracted tokube-configif we want to. Refine errors to preserve context of each error as much as possible. Replacekube::Error::Kubeconfig(ConfigError)withkube::Error::InferConfig(InferConfigError).kube::client::UpgradeConnectionError, and replace withkube::Error::UpgradeConnection(UpgradeConnectionError)variant.