Skip to content

Commit

Permalink
hand-curate auth error messages (#876)
Browse files Browse the repository at this point in the history
  • Loading branch information
ahl authored Oct 14, 2024
1 parent 81cfb3c commit ceafb75
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 5 deletions.
58 changes: 56 additions & 2 deletions cli/src/cmd_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

// Copyright 2024 Oxide Computer Company

use std::error::Error;
use std::fs::{File, OpenOptions};
use std::io::{self, Write};
use std::path::Path;
Expand Down Expand Up @@ -538,8 +539,61 @@ impl CmdAuthStatus {

fn error_msg(e: &oxide::Error<oxide::types::Error>) -> String {
match e {
oxide::Error::ErrorResponse(ee) => format!("Error Response: {}", ee.message),
ee => ee.to_string(),
oxide::Error::CommunicationError(error) => {
if error.is_timeout() {
"Request timed out".to_string()
} else if error.is_connect() {
// Reqwest will just tell us there was an error; we want to
// look at its internal error to understand the cause.
let details = error
.source()
.map_or("(no details)".to_string(), ToString::to_string);
format!("Failed to connect to server: {}", details)
} else {
let mut msg = "Unexpected error".to_string();
let mut next = Some(error as &(dyn std::error::Error + 'static));
while let Some(ee) = next {
msg += ": ";
msg += &ee.to_string();
next = ee.source();
}
msg
}
}
oxide::Error::ErrorResponse(response_value) => {
format!(
"Server responded with an error message: {}",
response_value.message,
)
}
oxide::Error::ResponseBodyError(_) => "Error reading the server's response".to_string(),
oxide::Error::InvalidResponsePayload(bytes, error) => {
// While this output might be big, it's a pretty unlikely
// failure mode for which we might reasonably want to see the
// output.
format!(
"Server responded with unexpected data: {} {}",
error,
bytes.escape_ascii(),
)
}
oxide::Error::UnexpectedResponse(response) => {
format!(
"Server responded with an unexpected status: {}",
response.status()
)
}
oxide::Error::InvalidRequest(msg) => {
// This would be indicative of a programming error where we
// didn't supply all required values.
format!("Internal error: {}", msg)
}
oxide::Error::InvalidUpgrade(_) => {
unreachable!("auth should not be establishing a websocket")
}
oxide::Error::PreHookError(_) => {
unreachable!("there is no pre-hook")
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/data/test_auth_status.stdout
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Profile "jennifer" (<TEST-SERVER>) status: Authenticated
Profile "lightman" (<TEST-SERVER>) status: Authenticated
Profile "malvin" (<TEST-SERVER>) status: Error Response: ** IMPROPER REQUEST **
Profile "sting" (https://unresolvabledomainnameihope) status: Communication Error: error sending request for url (https://unresolvabledomainnameihope/v1/me): error trying to connect:
Profile "malvin" (<TEST-SERVER>) status: Server responded with an error message: ** IMPROPER REQUEST **
Profile "sting" (https://unresolvabledomainnameihope) status: Failed to connect to server: error trying to connect:
5 changes: 4 additions & 1 deletion cli/tests/test_auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,9 @@ fn test_cmd_auth_status_env() {
.env("OXIDE_TOKEN", "oxide-token-bad")
.assert()
.success()
.stdout(format!("{}: Error Response: oops\n", server.url("")));
.stdout(format!(
"{}: Server responded with an error message: oops\n",
server.url("")
));
oxide_mock.assert();
}

0 comments on commit ceafb75

Please sign in to comment.