diff --git a/CHANGELOG.md b/CHANGELOG.md index ab5e33fb..26e2d55d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Upgrading from 2.x -3.0 is a rewrite in Rust. Commands, credentials, and config files carry over, but several 2.x quirks are gone, `--json` output has one consistent shape, and invalid input is rejected before anything is sent to Linear. Script authors should read the JSON output and command line sections. +3.0 is a rewrite in Rust. Commands, credentials, and config files carry over, but several 2.x quirks are gone, `--json` output has one consistent shape, invalid input is rejected before anything is sent to Linear, and the exit status tells failures apart. Script authors should read the JSON output and command line sections. #### Installation and runtime @@ -79,7 +79,7 @@ Every `--json` output follows one rule. Lists are a JSON array of entities, with - terminal Markdown wraps long lines at spaces to the terminal width, keeping list and quote indentation on continuation lines. Tables wider than the terminal shrink their columns and wrap cell text, and print one `Header: value` record per row when the terminal is too narrow for a grid - success messages share one form, `✓ Created issue ENG-123: Title` followed by the URL on its own line. Declining a confirmation prints `Canceled.` on stderr and exits 0 - errors read `✗ : `, often with a hint line underneath, and `LINEAR_DEBUG=1` adds the underlying causes. HTTP errors include a short excerpt of the response body. An ambiguous team, project, initiative, release, template, or user name lists the candidates. When a create's outcome is unknown, for example after a dropped connection, the error says the entity "may already exist" -- exit codes: `0` success, `1` error, `2` usage error (bad flags or values, a required value that is missing or empty, a confirmation or question that cannot be asked without a terminal, missing subcommand), `130` cancelled. A closed pipe (`linear issue list | head`) exits quietly +- exit codes: `0` success, `1` error, `2` usage error (bad flags or values, a required value that is missing or empty, a confirmation or question that cannot be asked without a terminal, missing subcommand), `3` not found (an issue, team, or other entity the command looked up), `4` authentication (no usable API key, or Linear rejected it), `5` unavailable (Linear could not be reached, timed out, rate limited the request, or failed with a server error; a create or update may still have taken effect), `130` cancelled. 2.x exited 1 for all of these, so a revoked key looked the same as a missing issue. `linear api` exits the same way for its response (GraphQL errors other than these stay `1`), and invalid input to it (no query, bad `--variables-json`) is now a usage error. A bulk command exits with the first of `4`, `5`, `1`, `3` among its failures, and says items "could not be found" only when they are all missing ("could not be looked up" otherwise). `linear --help` lists the statuses. A closed pipe (`linear issue list | head`) exits quietly ([#293](https://github.com/schpet/linear-cli/issues/293); thanks @sethfitz for the report) #### Security @@ -92,7 +92,7 @@ Every `--json` output follows one rule. Lists are a JSON array of entities, with #### Bug fixes -- `issue start` takes the team from the issue itself, so a full ID or URL works without a configured team. An argument that is not an issue ID errors instead of opening the picker, and a failed state update exits 1 (the branch or jj change is still prepared) instead of reporting success +- `issue start` takes the team from the issue itself, so a full ID or URL works without a configured team. An argument that is not an issue ID errors instead of opening the picker, and a failed state update exits nonzero (the branch or jj change is still prepared) instead of reporting success - finding the current issue from jj trailers reads one trailer per line. 2.x joined neighboring trailers (`Fixes A-1Fixes B-2`) and could pick the wrong issue. `issue commits` matches whole IDs, so `ENG-1` no longer matches `ENG-10` - `issue view` shows every label, child, attachment, document, and comment instead of the first page. `milestone view`, `team states`, issue state lookups, `linear config`'s team list, milestone names, agent session activities, `initiative view`'s projects, and the relations `issue relation list` shows and `issue relation delete` searches also read every page - `team delete --move-issues` reports a partial failure honestly and keeps the team, and bulk deletes skip items whose lookup failed instead of sending the mutation anyway diff --git a/README.md b/README.md index eefee8c7..8594123f 100644 --- a/README.md +++ b/README.md @@ -362,6 +362,22 @@ settings are read from two config files, a project file and a global file. each so the global file can hold defaults such as `issue_sort`, and a repository's `.linear.toml` overrides them for that project. every value is validated, even one a higher tier overrides, and an invalid value is an error naming its file and key. +## exit status + +scripts can tell why a command failed from its exit status, without parsing the error message: + +| status | meaning | +| ------ | ------- | +| `0` | success | +| `1` | any other failure, including GraphQL errors such as an invalid query in `linear api` | +| `2` | usage error: bad flags or values, rejected before anything is sent to Linear | +| `3` | not found: an issue, team, project, or other entity the command looked up does not exist | +| `4` | authentication: no usable API key, or Linear rejected it (revoked, mistyped, or lacking access) | +| `5` | unavailable: Linear could not be reached, timed out, rate limited the request, or failed with a server error. Retrying later may work, but a create or update may still have taken effect | +| `130` | cancelled at a prompt or in the editor | + +a bulk command (`--bulk`) that fails for several reasons exits with the first of `4`, `5`, `1`, `3` among them, so `3` means every failure was a missing item. `linear --help` lists the statuses too. + ## skills linear-cli includes a skill that helps AI agents use the CLI effectively. for use cases outside the CLI, it includes instructions to interact directly with the graphql api, including authentication. diff --git a/crates/linear-cli/src/app.rs b/crates/linear-cli/src/app.rs index d705e969..11ccd00b 100644 --- a/crates/linear-cli/src/app.rs +++ b/crates/linear-cli/src/app.rs @@ -78,11 +78,11 @@ fn run(cli: Cli, settings: &mut DisplaySettings) -> Result<()> { /// `LINEAR_DEBUG` the debug detail and source chain. fn report(error: &Error, settings: DisplaySettings) { let lines = match error.kind() { - ErrorKind::Exit(_) | ErrorKind::BrokenPipe => return, + ErrorKind::Reported(_) | ErrorKind::Exit(_) | ErrorKind::BrokenPipe => return, // The same word as declining a confirmation, though the status differs. ErrorKind::Cancelled => "Canceled.\n".to_owned(), ErrorKind::Usage(usage) => usage.render().to_string(), - ErrorKind::Other | ErrorKind::Invalid => { + ErrorKind::Failed(_) | ErrorKind::Invalid => { let terminal = Terminal::detect(settings.no_color); let color = terminal.stderr_color(); // On a terminal, lines wrap at spaces with their indent kept. diff --git a/crates/linear-cli/src/cli/api.rs b/crates/linear-cli/src/cli/api.rs index b40d0eb6..da360be9 100644 --- a/crates/linear-cli/src/cli/api.rs +++ b/crates/linear-cli/src/cli/api.rs @@ -19,7 +19,8 @@ pub struct Api { /// Follow the cursor of the one connection in the response and print every page #[arg(long)] pub paginate: bool, - /// Print nothing; the exit status still reports errors + /// Print nothing; the exit status still says whether and why it failed + /// (see `linear --help`) #[arg(long)] pub silent: bool, } diff --git a/crates/linear-cli/src/cli/mod.rs b/crates/linear-cli/src/cli/mod.rs index 59b3e4c7..4cbee54a 100644 --- a/crates/linear-cli/src/cli/mod.rs +++ b/crates/linear-cli/src/cli/mod.rs @@ -46,7 +46,21 @@ Environment: LINEAR_IGNORE_ENV_FILE=1 Do not load .env files Every .linear.toml setting can also be set with a LINEAR_* variable; run -`linear config` to write one for the current repository."; +`linear config` to write one for the current repository. + +Exit status: + 0 Success + 1 Any other failure, including GraphQL errors such as an invalid query + 2 Usage error: bad flags or values, rejected before anything is sent + 3 Not found: an issue, team, or other entity the command looked up + 4 Authentication: no usable API key, or Linear rejected it + 5 Unavailable: Linear could not be reached, timed out, rate limited + the request, or failed with a server error. A create or update may + still have taken effect + 130 Cancelled at a prompt or in the editor + +A bulk command that fails for several reasons exits with the first of 4, 5, +1, 3 among them."; /// Work with Linear from the command line #[derive(Debug, Parser)] diff --git a/crates/linear-cli/src/client.rs b/crates/linear-cli/src/client.rs index 60798b62..84178dad 100644 --- a/crates/linear-cli/src/client.rs +++ b/crates/linear-cli/src/client.rs @@ -29,7 +29,7 @@ use cynic::Operation; use crate::graphql::envelope::{GraphQlRequest, ResponseError, parse_response}; pub use config::{ApiKey, ClientBuildError, ClientConfig, Deadline, EndpointUrl, ResponseCap}; -pub use error::{HttpBodyShape, RawHttpResponse, RequestError}; +pub use error::{HttpBodyShape, RawHttpResponse, RequestError, classify_failure}; use config::build_client; use error::redact; diff --git a/crates/linear-cli/src/client/error.rs b/crates/linear-cli/src/client/error.rs index ae7928cd..5bcdef96 100644 --- a/crates/linear-cli/src/client/error.rs +++ b/crates/linear-cli/src/client/error.rs @@ -9,9 +9,9 @@ use reqwest::header::HeaderMap; use super::config::{Deadline, ResponseCap}; use super::content_type; -use crate::error::Error; +use crate::error::{Error, Failure}; use crate::graphql::envelope::{ - ResponseError, ResponseGraphQlError, graphql_message, is_not_found, + ResponseError, ResponseGraphQlError, graphql_failure, graphql_message, }; /// A `reqwest::Error` with its URL removed before it is stored or chained. @@ -313,6 +313,24 @@ impl StdError for RequestError { } } +/// The failure class of a response with `status` and GraphQL `errors` +/// (empty when the body had none or could not be read). +/// +/// HTTP 401 and 403 are [`Failure::Auth`] and HTTP 408, 429 and 5xx are +/// [`Failure::Unavailable`], combined with the class of the errors; another +/// status, such as a 404 from a misconfigured endpoint, adds nothing beyond +/// [`Failure::General`]. A recognized missing entity under HTTP 400 is still +/// [`Failure::NotFound`]. +pub fn classify_failure(status: StatusCode, errors: &[ResponseGraphQlError]) -> Failure { + let by_status = match status { + StatusCode::UNAUTHORIZED | StatusCode::FORBIDDEN => Some(Failure::Auth), + StatusCode::REQUEST_TIMEOUT | StatusCode::TOO_MANY_REQUESTS => Some(Failure::Unavailable), + status if status.is_server_error() => Some(Failure::Unavailable), + _ => None, + }; + Failure::fold(by_status.into_iter().chain(graphql_failure(errors))).unwrap_or(Failure::General) +} + impl RequestError { /// Whether the request may have reached Linear and taken effect anyway: a /// timeout, a network failure after connecting (a reset after the request @@ -327,9 +345,21 @@ impl RequestError { } } - /// Whether Linear answered that the requested entity does not exist. + /// The failure class this ends a command with. + pub fn failure(&self) -> Failure { + match self { + Self::GraphQl { status, errors, .. } => classify_failure(*status, errors), + Self::Http { response, .. } => classify_failure(response.status, &[]), + Self::ResponseTooLarge { status, .. } => classify_failure(*status, &[]), + Self::Timeout { .. } | Self::Network { .. } => Failure::Unavailable, + Self::RequestBody(_) | Self::Response(_) => Failure::General, + } + } + + /// Whether Linear answered that the requested entity does not exist, and + /// nothing worse: a missing entity next to a rejected key is not "absent". pub fn is_not_found(&self) -> bool { - matches!(self, Self::GraphQl { errors, .. } if is_not_found(errors)) + self.failure() == Failure::NotFound } /// [`Error::not_found`] for `entity` `identifier` when Linear answered @@ -357,8 +387,10 @@ impl RequestError { impl From for Error { fn from(failure: RequestError) -> Self { let message = failure.to_string(); + let class = failure.failure(); + let error = |message| Error::failed(class, message); match failure { - RequestError::RequestBody(source) => Error::new(message).with_source(source), + RequestError::RequestBody(source) => error(message).with_source(source), RequestError::GraphQl { status, errors, @@ -367,14 +399,15 @@ impl From for Error { } => { // The summary omits arbitrary response extensions, headers and // the request URL. - Error::new(message).with_debug_detail(format!( + error(message).with_debug_detail(format!( "GraphQL HTTP {status}; errors={}; partial_data={partial_data}", errors.len() )) } + // Only a 2xx body that is not usable data: always general. RequestError::Response(source) => Error::from(source), RequestError::Http { response, body } => { - let error = Error::new(message).with_debug_detail(format!( + let error = error(message).with_debug_detail(format!( "HTTP {} body: {}", response.status, response.body_text() @@ -384,10 +417,8 @@ impl From for Error { HttpBodyShape::Data => error, } } - RequestError::ResponseTooLarge { .. } | RequestError::Timeout { .. } => { - Error::new(message) - } - RequestError::Network { source, .. } => Error::new(message).with_source(source), + RequestError::ResponseTooLarge { .. } | RequestError::Timeout { .. } => error(message), + RequestError::Network { source, .. } => error(message).with_source(source), } } } diff --git a/crates/linear-cli/src/client/http.rs b/crates/linear-cli/src/client/http.rs index 31e4a70c..9e508b58 100644 --- a/crates/linear-cli/src/client/http.rs +++ b/crates/linear-cli/src/client/http.rs @@ -8,9 +8,9 @@ use reqwest::header::{AUTHORIZATION, CONTENT_TYPE, HeaderMap, HeaderValue}; use reqwest::{StatusCode, Url}; use super::config::{Deadline, EndpointUrl, ResponseCap}; -use super::error::{NetworkPhase, RawHttpResponse, SanitizedReqwestError}; +use super::error::{NetworkPhase, RawHttpResponse, SanitizedReqwestError, classify_failure}; use super::{CONTENT_TYPE_VALUE, LinearClient}; -use crate::error::Error; +use crate::error::{Error, Failure}; /// A failure below HTTP classification, before the client attaches its /// origin. @@ -73,20 +73,46 @@ pub(super) async fn collect( }) } -/// A short, display-safe description of a failed non-GraphQL request. +/// A short, display-safe description of a failed non-GraphQL request. A +/// timeout or network failure is [`Failure::Unavailable`]; an oversized body +/// is classified by its status. fn bounded_failure(prefix: &str, failure: ExchangeFailure, deadline: Deadline) -> Error { match failure { - ExchangeFailure::ResponseTooLarge { limit, .. } => Error::new(format!( - "{prefix}: response exceeds the {} byte limit", - limit.bytes() - )), - ExchangeFailure::Timeout => Error::new(format!( - "{prefix}: did not complete within {:?}", - deadline.duration() - )), - ExchangeFailure::Network { source, .. } => { - Error::new(format!("{prefix}: {}", source.root_message())).with_source(source) - } + ExchangeFailure::ResponseTooLarge { status, limit } => Error::failed( + status_failure(status), + format!( + "{prefix}: response exceeds the {} byte limit", + limit.bytes() + ), + ), + ExchangeFailure::Timeout => Error::failed( + Failure::Unavailable, + format!( + "{prefix}: did not complete within {:?}", + deadline.duration() + ), + ), + ExchangeFailure::Network { source, .. } => Error::failed( + Failure::Unavailable, + format!("{prefix}: {}", source.root_message()), + ) + .with_source(source), + } +} + +/// The failure class of a response with `status` and no usable body. +fn status_failure(status: StatusCode) -> Failure { + classify_failure(status, &[]) +} + +/// The failure class of a download or signed upload that got `status` from a +/// host other than the API: unavailable for 408, 429 and 5xx, otherwise +/// general. A 401 or 403 there is the URL's own access, which logging in to +/// Linear again does not fix. +fn storage_failure(status: StatusCode) -> Failure { + match status_failure(status) { + Failure::Unavailable => Failure::Unavailable, + Failure::General | Failure::NotFound | Failure::Auth => Failure::General, } } @@ -140,7 +166,10 @@ impl LinearClient { .map_err(|error| failed(classify_network(error)))?; let status = response.status(); if !status.is_success() { - return Err(Error::new(format!("{failure_prefix}: {status}"))); + return Err(Error::failed( + storage_failure(status), + format!("{failure_prefix}: {status}"), + )); } let response = collect(response, self.max_download_bytes) .await @@ -150,7 +179,7 @@ impl LinearClient { /// POSTs a raw GraphQL body for the `api` command and returns the status /// and body text unclassified, within the API deadline and size cap. - pub async fn fetch_api(&self, body: String) -> Result<(u16, String), Error> { + pub async fn fetch_api(&self, body: String) -> Result<(StatusCode, String), Error> { let response = self .http .post(self.endpoint.url.clone()) @@ -177,7 +206,7 @@ impl LinearClient { ) })?; Ok(( - response.status.as_u16(), + response.status, String::from_utf8_lossy(&response.body).into_owned(), )) } @@ -195,11 +224,14 @@ impl LinearClient { let mut url = Url::parse(url).map_err(|_| invalid())?; url.set_fragment(None); let target = EndpointUrl::from_url(url).map_err(|_| invalid())?; - let failed = |reason: String| { - Error::new(format!( - "Signed upload to {target} failed: {reason}; the object may already be \ + let failed = |failure: Failure, reason: String| { + Error::failed( + failure, + format!( + "Signed upload to {target} failed: {reason}; the object may already be \ stored remotely; no comment or attachment was created" - )) + ), + ) }; let response = self .http @@ -210,27 +242,32 @@ impl LinearClient { .await .map_err(|error| { let error = SanitizedReqwestError::new(error); - failed(error.root_message()).with_source(error) + failed(Failure::Unavailable, error.root_message()).with_source(error) })?; - if response.status().is_success() { + let status = response.status(); + if status.is_success() { return Ok(()); } let response = collect(response, self.max_response_bytes) .await .map_err(|failure| match failure { - ExchangeFailure::ResponseTooLarge { limit, .. } => { - failed(format!("response exceeded {} bytes", limit.bytes())) - } - ExchangeFailure::Timeout => failed("timed out".to_owned()), + ExchangeFailure::ResponseTooLarge { limit, .. } => failed( + storage_failure(status), + format!("response exceeded {} bytes", limit.bytes()), + ), + ExchangeFailure::Timeout => failed(Failure::Unavailable, "timed out".to_owned()), ExchangeFailure::Network { source, .. } => { - failed(source.root_message()).with_source(source) + failed(Failure::Unavailable, source.root_message()).with_source(source) } })?; - Err(Error::new(format!( - "Failed to upload file: {} - {}", - response.status, - String::from_utf8_lossy(&response.body) - ))) + Err(Error::failed( + storage_failure(response.status), + format!( + "Failed to upload file: {} - {}", + response.status, + String::from_utf8_lossy(&response.body) + ), + )) } } diff --git a/crates/linear-cli/src/client/tests.rs b/crates/linear-cli/src/client/tests.rs index 7ff73e88..f896418e 100644 --- a/crates/linear-cli/src/client/tests.rs +++ b/crates/linear-cli/src/client/tests.rs @@ -20,6 +20,7 @@ use super::{ ApiKey, CONTENT_TYPE_VALUE, ClientBuildError, ClientConfig, Deadline, EndpointUrl, HttpBodyShape, LinearClient, RawHttpResponse, RequestError, ResponseCap, classify_typed, }; +use crate::error::Failure; use crate::graphql::envelope::{GraphQlRequest, ResponseError}; use crate::graphql::operations::team::{GetTeams, GetTeamsVariables}; @@ -768,6 +769,7 @@ async fn raw_document_without_variables_returns_exact_bytes() { async fn raw_api_requests_stop_at_the_api_cap_and_deadline() { let server = Server::start(vec![ Reply::status(200, "application/json", vec![b' '; 8192]), + Reply::status(401, "application/json", vec![b' '; 8192]), Reply::Stall, ]); let client = client_for( @@ -783,6 +785,12 @@ async fn raw_api_requests_stop_at_the_api_cap_and_deadline() { "Failed to read API response; the request was sent and may have taken effect: \ response exceeds the 4096 byte limit" ); + assert_eq!(error.failure(), Some(Failure::General)); + let error = client + .fetch_api("{}".to_owned()) + .await + .expect_err("too large"); + assert_eq!(error.failure(), Some(Failure::Auth)); let started = Instant::now(); let error = client .fetch_api("{}".to_owned()) @@ -793,6 +801,57 @@ async fn raw_api_requests_stop_at_the_api_cap_and_deadline() { "{}", error.message() ); + assert_eq!(error.failure(), Some(Failure::Unavailable)); assert!(started.elapsed() < Duration::from_secs(30)); server.finish(); } + +#[test] +fn failures_are_classified_by_status_and_graphql_error() { + use super::classify_failure; + use crate::graphql::envelope::ResponseGraphQlError; + use Failure::{Auth, General, NotFound, Unavailable}; + let errors = |body: Value| -> Vec { + serde_json::from_value(body).expect("GraphQL errors") + }; + let missing = json!({ "message": "Entity not found: Issue" }); + let presentable_missing = json!({ + "message": "Argument Validation Error", + "extensions": { "userPresentableMessage": "Could not find referenced Issue." }, + }); + let auth = json!({ "message": "x", "extensions": { "code": "AUTHENTICATION_ERROR" } }); + let forbidden = json!({ "message": "x", "extensions": { "code": "FORBIDDEN" } }); + let limited = json!({ "message": "x", "extensions": { "code": "RATELIMITED" } }); + let other = json!({ "message": "Team not found in this workspace's settings page" }); + let input = json!({ "message": "Argument invalid", "extensions": { "code": "INVALID_INPUT" } }); + for (status, body, expected) in [ + (200, json!([missing]), NotFound), + (400, json!([missing, presentable_missing]), NotFound), + (200, json!([auth]), Auth), + (200, json!([forbidden]), Auth), + (400, json!([limited]), Unavailable), + (200, json!([other]), General), + (400, json!([input]), General), + (200, json!([missing, other]), General), + (200, json!([other, missing]), General), + (200, json!([missing, limited]), Unavailable), + (200, json!([limited, auth]), Auth), + (401, json!([missing]), Auth), + (503, json!([missing]), Unavailable), + (401, json!([]), Auth), + (403, json!([]), Auth), + (408, json!([]), Unavailable), + (429, json!([]), Unavailable), + (502, json!([]), Unavailable), + (404, json!([]), General), + (400, json!([]), General), + (200, json!([]), General), + ] { + let status = StatusCode::from_u16(status).expect("status"); + assert_eq!( + classify_failure(status, &errors(body.clone())), + expected, + "{status} {body}" + ); + } +} diff --git a/crates/linear-cli/src/commands/api.rs b/crates/linear-cli/src/commands/api.rs index 7733d9dc..7e1eb9ec 100644 --- a/crates/linear-cli/src/commands/api.rs +++ b/crates/linear-cli/src/commands/api.rs @@ -1,12 +1,14 @@ //! `linear api`: send a user-written GraphQL document and print the response. -use crate::client::LinearClient; +use crate::client::{LinearClient, classify_failure}; +use crate::graphql::envelope::ResponseGraphQlError; use crate::graphql::pagination::{Page, PageInfo, Pages}; use crate::{ cli::api::Api, commands::text_input, ctx::Ctx, - error::{Error, Result, ResultExt}, + error::{Error, Failure, Result, ResultExt}, }; +use reqwest::StatusCode; use serde_json::{Map, Number, Value}; pub fn run(ctx: &Ctx, args: &Api) -> Result<()> { @@ -25,24 +27,23 @@ fn request_and_print(ctx: &Ctx, args: &Api) -> Result<()> { ctx.stdout_tty(), ))?; // The response is printed either way; the exit status says whether it - // was a success. - let (text, succeeded) = match response { - Response::Data(text) => (text, true), - Response::Errors(text) => (text, false), - Response::HttpError(body) => { + // was a success and, if not, the failure's class. + let (text, failure) = match response { + Response::Data(text) => (text, None), + Response::Errors(text, failure) => (text, Some(failure)), + Response::HttpError(body, failure) => { if !args.silent { ctx.eprint(body)?; } - return Err(Error::reported()); + return Err(Error::reported(failure)); } }; if !args.silent { ctx.print(text)?; } - if succeeded { - Ok(()) - } else { - Err(Error::reported()) + match failure { + None => Ok(()), + Some(failure) => Err(Error::reported(failure)), } } @@ -51,7 +52,7 @@ fn decode(text: &str) -> Option { serde_json::from_str(text).ok() } fn no_query() -> Error { - Error::new("No query provided").with_hint("Provide a query as an argument: linear api '{ viewer { id } }'\n Or pipe from stdin: echo '{ viewer { id } }' | linear api") + Error::invalid("No query provided").with_hint("Provide a query as an argument: linear api '{ viewer { id } }'\n Or pipe from stdin: echo '{ viewer { id } }' | linear api") } fn stdin_all() -> Result { Ok(text_input::read_stdin(std::io::stdin().lock())? @@ -88,7 +89,7 @@ fn plain(text: &str) -> Result { } if text.parse::().is_ok_and(|number| !number.is_finite()) { return Err( - Error::new(format!("Variable value {text} is not a finite number")) + Error::invalid(format!("Variable value {text} is not a finite number")) .with_hint("Pass a finite number, or a JSON string through --variables-json."), ); } @@ -101,7 +102,7 @@ fn variables(action: &Api) -> Result> { let mut variables = Map::new(); if let Some(text) = action.variables_json.as_deref().filter(|s| !s.is_empty()) { let value = decode(text).ok_or_else(|| { - Error::new(format!("Invalid JSON for --variables-json: {text}")).with_hint( + Error::invalid(format!("Invalid JSON for --variables-json: {text}")).with_hint( "Provide a valid JSON object, e.g. --variables-json '{\"key\": \"value\"}'", ) })?; @@ -117,7 +118,7 @@ fn variables(action: &Api) -> Result> { Value::Array(_) => Some("array"), }; if let Some(kind) = kind { - return Err(Error::new(format!( + return Err(Error::invalid(format!( "--variables-json must be a JSON object, got {kind}" )) .with_hint("Provide a JSON object, e.g. --variables-json '{\"key\": \"value\"}'")); @@ -128,7 +129,7 @@ fn variables(action: &Api) -> Result> { let value = if raw == "@-" { let text = stdin_all()?; if text.is_empty() { - return Err(Error::new("No data on stdin for @- value")); + return Err(Error::invalid("No data on stdin for @- value")); } parsed_or_string(text) } else if let Some(path) = raw.strip_prefix('@') { @@ -158,10 +159,10 @@ fn request(query: &str, variables: &Map) -> String { enum Response { /// The response body as printed. Data(String), - /// GraphQL errors, or a body that is not JSON, as printed. - Errors(String), - /// A failed HTTP status, with the body for stderr. - HttpError(String), + /// GraphQL errors, or a body that is not JSON, as printed, and their class. + Errors(String, Failure), + /// A failed HTTP status, with the body for stderr, and its class. + HttpError(String, Failure), } /// The response as printed: pretty on a terminal, compact when piped, and @@ -182,6 +183,17 @@ fn has_errors(value: &Value) -> bool { .and_then(Value::as_array) .is_some_and(|errors| !errors.is_empty()) } +/// The class of a response with `status` and body `parsed`, read like a typed +/// operation's (see [`classify_failure`]). An `errors` array that does not +/// have GraphQL's error shape counts as no recognizable errors. +fn failure(status: StatusCode, parsed: Option<&Value>) -> Failure { + let errors: Vec = parsed + .and_then(|value| value.get("errors")) + .cloned() + .and_then(|errors| serde_json::from_value(errors).ok()) + .unwrap_or_default(); + classify_failure(status, &errors) +} fn is_connection(object: &Map) -> bool { object.contains_key("nodes") && object.contains_key("pageInfo") } @@ -245,11 +257,12 @@ async fn execute( ); } let (status, text) = client.fetch_api(request(query, &vars)).await?; - if status >= 400 { - return Ok(Response::HttpError(format!("{text}\n"))); + if status.as_u16() >= 400 { + let failure = failure(status, decode(&text).as_ref()); + return Ok(Response::HttpError(format!("{text}\n"), failure)); } let Some(parsed) = decode(&text) else { - return Ok(Response::Errors(format!("{text}\n"))); + return Ok(Response::Errors(format!("{text}\n"), Failure::General)); }; if parsed.is_null() { if paginate { @@ -258,7 +271,8 @@ async fn execute( return Ok(Response::Data(format!("{text}\n"))); } if has_errors(&parsed) { - return Ok(Response::Errors(json_output(&parsed, &text, tty))); + let failure = failure(status, Some(&parsed)); + return Ok(Response::Errors(json_output(&parsed, &text, tty), failure)); } if !paginate { return Ok(Response::Data(json_output(&parsed, &text, tty))); diff --git a/crates/linear-cli/src/commands/auth/default.rs b/crates/linear-cli/src/commands/auth/default.rs index 0f33194a..b23b555a 100644 --- a/crates/linear-cli/src/commands/auth/default.rs +++ b/crates/linear-cli/src/commands/auth/default.rs @@ -22,7 +22,7 @@ fn set_default(ctx: &Ctx, args: &AuthDefault) -> Result<()> { let target = match super::named_workspace(ctx, args.workspace_name.as_deref())? { Some(target) => { if !credentials.has_workspace(target) { - return Err(Error::not_found("Workspace", target) + return Err(Error::invalid(format!("Workspace not found: {target}")) .with_hint(format!("Available workspaces: {}", workspaces.join(", ")))); } target.to_owned() diff --git a/crates/linear-cli/src/commands/auth/list.rs b/crates/linear-cli/src/commands/auth/list.rs index ee7b31a9..b1adb9c9 100644 --- a/crates/linear-cli/src/commands/auth/list.rs +++ b/crates/linear-cli/src/commands/auth/list.rs @@ -1,13 +1,12 @@ //! `auth list`: every stored workspace with the organization and user its //! key belongs to, checked with one request per key, all at once. use futures_util::future::join_all; -use reqwest::StatusCode; use crate::auth::CredentialStore; use crate::client::{ApiKey, LinearClient, RequestError}; use crate::commands::table::{Cell, Column, Table}; use crate::ctx::Ctx; -use crate::error::{Result, ResultExt}; +use crate::error::{Failure, Result, ResultExt}; use crate::graphql::envelope::graphql_message; use crate::graphql::operations::user::GetViewerAccount; use crate::platform::style; @@ -96,21 +95,11 @@ async fn check(check: &Check) -> Outcome { } } -fn rejected(status: StatusCode) -> bool { - status == StatusCode::UNAUTHORIZED || status == StatusCode::FORBIDDEN -} - -/// A short cell for a failed check. A 401 or 403 means the key was refused. +/// A short cell for a failed check: a key Linear refused (see +/// [`Failure::Auth`]) is "invalid credentials". fn failure_cell(failure: &RequestError) -> String { match failure { - RequestError::GraphQl { status, .. } | RequestError::ResponseTooLarge { status, .. } - if rejected(*status) => - { - "invalid credentials".to_owned() - } - RequestError::Http { response, .. } if rejected(response.status) => { - "invalid credentials".to_owned() - } + failure if failure.failure() == Failure::Auth => "invalid credentials".to_owned(), RequestError::GraphQl { errors, .. } => { graphql_message(errors).unwrap_or_else(|| failure.to_string()) } diff --git a/crates/linear-cli/src/commands/auth/login.rs b/crates/linear-cli/src/commands/auth/login.rs index 5f8440bb..b2537649 100644 --- a/crates/linear-cli/src/commands/auth/login.rs +++ b/crates/linear-cli/src/commands/auth/login.rs @@ -1,8 +1,6 @@ //! `auth login`: check an API key with Linear, then store it. use std::io::Read; -use reqwest::StatusCode; - use crate::auth::keyring::Keyring; use crate::auth::mutation::Credentials; use crate::auth::{ApiKeyInput, CredentialFormat}; @@ -10,8 +8,7 @@ use crate::cli::auth::AuthLogin; use crate::client::{ApiKey, LinearClient, RequestError}; use crate::config::ConfigSecret; use crate::ctx::Ctx; -use crate::error::{Error, Result, ResultExt}; -use crate::graphql::envelope::ResponseGraphQlError; +use crate::error::{Error, Failure, Result, ResultExt}; use crate::graphql::operations::user::GetViewerAccount; use crate::platform::style; @@ -38,7 +35,7 @@ fn login(ctx: &Ctx, args: &AuthLogin) -> Result<()> { let client = LinearClient::new( ctx.options().endpoint().value().clone(), ApiKey::new(key.expose()).map_err(|error| { - Error::new("API key cannot be used as an HTTP header").with_source(error) + Error::auth("API key cannot be used as an HTTP header").with_source(error) })?, ctx.config().network_env.client_config(), )?; @@ -125,39 +122,14 @@ fn clean_key(key: ConfigSecret) -> Result { /// Linear refused the key: HTTP 401/403 or an authentication error. fn rejected_key(failure: RequestError) -> Error { - let refused = match &failure { - RequestError::GraphQl { status, errors, .. } => { - refused_status(*status) || errors.iter().any(authentication_error) - } - RequestError::Http { response, .. } => refused_status(response.status), - RequestError::ResponseTooLarge { status, .. } => refused_status(*status), - RequestError::RequestBody(_) - | RequestError::Response(_) - | RequestError::Timeout { .. } - | RequestError::Network { .. } => false, - }; - if refused { - Error::auth("Invalid API key") + match failure.failure() { + Failure::Auth => Error::auth("Invalid API key") .with_hint("Check that your API key is correct and not expired.") - .with_source(failure) - } else { - Error::from(failure) + .with_source(failure), + Failure::General | Failure::NotFound | Failure::Unavailable => Error::from(failure), } } -fn refused_status(status: StatusCode) -> bool { - status == StatusCode::UNAUTHORIZED || status == StatusCode::FORBIDDEN -} - -fn authentication_error(error: &ResponseGraphQlError) -> bool { - error - .extensions - .as_ref() - .and_then(|extensions| extensions.get("code")) - .and_then(serde_json::Value::as_str) - == Some("AUTHENTICATION_ERROR") -} - /// Offers to move plaintext keys to the keyring. Only asked on a terminal; /// otherwise the command is suggested. fn offer_migration(ctx: &Ctx, credentials: &mut Credentials, keyring: &dyn Keyring) -> Result<()> { diff --git a/crates/linear-cli/src/commands/auth/logout.rs b/crates/linear-cli/src/commands/auth/logout.rs index a1949e40..e1899f87 100644 --- a/crates/linear-cli/src/commands/auth/logout.rs +++ b/crates/linear-cli/src/commands/auth/logout.rs @@ -17,7 +17,7 @@ fn logout(ctx: &Ctx, args: &AuthLogout) -> Result<()> { let named = super::named_workspace(ctx, args.workspace_name.as_deref())?; let workspace = match (named, credentials.workspaces()) { (Some(name), _) if !credentials.has_workspace(name) => { - return Err(Error::not_found("Workspace", name)); + return Err(Error::invalid(format!("Workspace not found: {name}"))); } (Some(name), _) => name.to_owned(), (None, [only]) => only.clone(), diff --git a/crates/linear-cli/src/commands/bulk.rs b/crates/linear-cli/src/commands/bulk.rs index f095bbc0..daaa045a 100644 --- a/crates/linear-cli/src/commands/bulk.rs +++ b/crates/linear-cli/src/commands/bulk.rs @@ -6,7 +6,7 @@ use futures_util::{StreamExt, stream}; use crate::cli::BulkArgs; use crate::ctx::Ctx; -use crate::error::{Error, Result}; +use crate::error::{Error, Failure, Result}; pub struct BulkInput<'a> { pub argv: Option<&'a [String]>, @@ -39,7 +39,7 @@ pub fn collect_ids(input: &BulkInput<'_>, stdin: &mut impl Read) -> Result, stdin: &mut impl Read) -> Result Self { + Self::Failed { + message: error.message().to_owned(), + failure: item_failure(error), + } + } +} + +/// The class a failed item counts as: its runtime class, or general for an +/// item that could not be used at all, such as an ID that does not parse. +fn item_failure(error: &Error) -> Failure { + error.failure().unwrap_or(Failure::General) } #[derive(Clone, Debug, PartialEq, Eq)] pub struct BulkResult { @@ -110,16 +127,37 @@ impl Found { name: Some(self.name.clone()), outcome: match outcome { Ok(()) => BulkOutcome::Succeeded, - Err(error) => BulkOutcome::Failed(error.message().to_owned()), + Err(error) => BulkOutcome::failed(&error), }, } } } -/// A listed item that could not be looked up, and why. +/// A listed item that could not be looked up, why, and the failure's class. pub struct Skipped { pub original: String, pub reason: String, + pub failure: Failure, +} + +impl Skipped { + /// `original` names no `entity`, as in `Issue not found`. + pub fn not_found(original: String, entity: &str) -> Self { + Self { + original, + reason: format!("{entity} not found"), + failure: Failure::NotFound, + } + } + + /// Looking up `original` failed with `error`. + pub fn failed(original: String, error: &Error) -> Self { + Self { + original, + reason: error.message().to_owned(), + failure: item_failure(error), + } + } } impl From for BulkResult { @@ -127,11 +165,29 @@ impl From for BulkResult { Self { id: skipped.original, name: None, - outcome: BulkOutcome::Failed(skipped.reason), + outcome: BulkOutcome::Failed { + message: skipped.reason, + failure: skipped.failure, + }, } } } +/// The error when none of the listed `noun`s (plural) could be looked up: +/// "could not be found" only when every one was missing, otherwise the most +/// serious failure among `skipped`, which must not be empty. +pub fn none_found(skipped: &[Skipped], noun: &str) -> Error { + let failure = Failure::fold(skipped.iter().map(|skipped| skipped.failure)) + .expect("an empty lookup skipped at least one listed item"); + Error::failed( + failure, + format!( + "None of the listed {noun} could be {}", + skip_reason(skipped) + ), + ) +} + /// Looks up every listed item, five at a time behind a spinner, before /// anything is confirmed or changed. Both lists keep the input order. pub fn look_up(ctx: &Ctx, items: Vec, op: F) -> (Vec>, Vec) @@ -167,8 +223,9 @@ pub fn preview(found: &[Found], missing: &[Skipped], noun: &str, verb: Ver } if !missing.is_empty() { output.push_str(&format!( - "Skipping {} that could not be found:\n", - count(missing.len(), noun) + "Skipping {} that could not be {}:\n", + count(missing.len(), noun), + skip_reason(missing) )); for skipped in missing { output.push_str(&format!(" {}: {}\n", skipped.original, skipped.reason)); @@ -177,6 +234,16 @@ pub fn preview(found: &[Found], missing: &[Skipped], noun: &str, verb: Ver output } +/// What could not be done for `skipped` items: "found" only when every one +/// is missing, since a lookup that failed says nothing about whether the item +/// exists; otherwise "looked up". +fn skip_reason(skipped: &[Skipped]) -> &'static str { + match Failure::fold(skipped.iter().map(|skipped| skipped.failure)) { + Some(Failure::NotFound) | None => "found", + Some(Failure::General | Failure::Auth | Failure::Unavailable) => "looked up", + } +} + /// `count` of `noun`, as in `1 issue` or `3 issues`. pub fn count(count: usize, noun: &str) -> String { format!("{count} {noun}{}", if count == 1 { "" } else { "s" }) @@ -217,15 +284,15 @@ pub fn report(ctx: &Ctx, results: &[BulkResult], noun: &str, verb: Verb) -> Resu // stderr; on stdout it would be the first line a script reads. ctx.eprint("\n")?; ctx.print(summary)?; - if failed { - return Err(Error::reported()); + match failed { + Some(failure) => Err(Error::reported(failure)), + None => Ok(()), } - Ok(()) } -/// The summary after a bulk run, and whether anything failed. `noun` is -/// singular, as in `issue`. -pub fn summary(results: &[BulkResult], noun: &str, verb: Verb) -> (String, bool) { +/// The summary after a bulk run, and the combined class of its failures +/// (`None` when nothing failed). `noun` is singular, as in `issue`. +pub fn summary(results: &[BulkResult], noun: &str, verb: Verb) -> (String, Option) { let total = results.len(); let succeeded = results.iter().filter(|row| row.succeeded()).count(); let failed = total - succeeded; @@ -237,7 +304,7 @@ pub fn summary(results: &[BulkResult], noun: &str, verb: Verb) -> (String, bool) verb.past, count(succeeded) )); - return (output, false); + return (output, None); } if succeeded == 0 { output.push_str(&format!( @@ -254,7 +321,7 @@ pub fn summary(results: &[BulkResult], noun: &str, verb: Verb) -> (String, bool) } output.push_str("\nFailed operations:\n"); for row in results { - if let BulkOutcome::Failed(error) = &row.outcome { + if let BulkOutcome::Failed { message: error, .. } = &row.outcome { let name = row .name .as_deref() @@ -263,7 +330,11 @@ pub fn summary(results: &[BulkResult], noun: &str, verb: Verb) -> (String, bool) output.push_str(&format!(" - {}{name}: {error}\n", row.id)); } } - (output, true) + let failure = Failure::fold(results.iter().filter_map(|row| match &row.outcome { + BulkOutcome::Succeeded => None, + BulkOutcome::Failed { failure, .. } => Some(*failure), + })); + (output, failure) } fn progress(completed: usize, total: usize, succeeded: usize) -> String { @@ -278,6 +349,7 @@ fn progress(completed: usize, total: usize, succeeded: usize) -> String { #[cfg(test)] mod tests { use super::{BulkInput, BulkOutcome, BulkResult, Verb, collect_ids, summary}; + use crate::error::Failure; const ARCHIVE: Verb = Verb { present: "archive", @@ -292,20 +364,30 @@ mod tests { } } + fn failed(id: &str, message: &str, failure: Failure) -> BulkResult { + row( + id, + BulkOutcome::Failed { + message: message.to_owned(), + failure, + }, + ) + } + #[test] fn summaries_count_successes_and_list_failures() { let ok = row("ENG-1", BulkOutcome::Succeeded); - let failed = row("ENG-2", BulkOutcome::Failed("Issue not found".to_owned())); + let failed = failed("ENG-2", "Issue not found", Failure::NotFound); assert_eq!( summary(std::slice::from_ref(&ok), "issue", ARCHIVE), - ("✓ Successfully archived 1 issue\n".to_owned(), false) + ("✓ Successfully archived 1 issue\n".to_owned(), None) ); assert_eq!( summary(&[ok, failed.clone()], "issue", ARCHIVE), ( "Completed: 1/2 issues archived\n ✓ Succeeded: 1\n ✗ Failed: 1\n\nFailed operations:\n - ENG-2 (ENG-2: Title): Issue not found\n" .to_owned(), - true + Some(Failure::NotFound) ) ); assert!( diff --git a/crates/linear-cli/src/commands/document/common.rs b/crates/linear-cli/src/commands/document/common.rs index 220f6c7c..45d15879 100644 --- a/crates/linear-cli/src/commands/document/common.rs +++ b/crates/linear-cli/src/commands/document/common.rs @@ -55,7 +55,7 @@ pub fn read_file(path: &Path) -> Result> { pub fn read_source(source: &TextSource) -> Result> { text_input::read_source(source).map_err(|error| { if error.kind() == std::io::ErrorKind::NotFound { - Error::not_found("File", &source.to_string()) + Error::new(format!("File not found: {source}")) } else { Error::new(format!("Failed to read {source}: {error}")).with_source(error) } diff --git a/crates/linear-cli/src/commands/document/delete.rs b/crates/linear-cli/src/commands/document/delete.rs index 8f0f79e4..327d45d5 100644 --- a/crates/linear-cli/src/commands/document/delete.rs +++ b/crates/linear-cli/src/commands/document/delete.rs @@ -62,7 +62,7 @@ fn delete_bulk(ctx: &Ctx, args: &DocumentDelete, input: &BulkInput<'_>) -> Resul }; ctx.eprint(bulk::preview(&found, &missing, "document", verb))?; if found.is_empty() { - return Err(Error::new("None of the listed documents could be found")); + return Err(bulk::none_found(&missing, "documents")); } let question = format!("Delete {}?", bulk::count(found.len(), "document")); if !args.confirm.yes && !ctx.confirm(&question, "--yes")? { @@ -89,14 +89,8 @@ async fn look_up_item( name: document.title, item: document.id.into_inner(), }), - Ok(None) => Err(Skipped { - original, - reason: "Document not found".to_owned(), - }), - Err(error) => Err(Skipped { - original, - reason: error.message().to_owned(), - }), + Ok(None) => Err(Skipped::not_found(original, "Document")), + Err(error) => Err(Skipped::failed(original, &error)), } } diff --git a/crates/linear-cli/src/commands/initiative/archive_or_delete.rs b/crates/linear-cli/src/commands/initiative/archive_or_delete.rs index 27b83c28..b4618342 100644 --- a/crates/linear-cli/src/commands/initiative/archive_or_delete.rs +++ b/crates/linear-cli/src/commands/initiative/archive_or_delete.rs @@ -147,7 +147,7 @@ fn run_bulk(ctx: &Ctx, mode: Mode, request: &Request<'_>) -> Result<()> { }; ctx.eprint(bulk::preview(&found, &missing, "initiative", verb))?; if found.is_empty() { - return Err(Error::new("None of the listed initiatives could be found")); + return Err(bulk::none_found(&missing, "initiatives")); } if mode == Mode::Delete { ctx.eprint(PERMANENT)?; @@ -194,14 +194,8 @@ async fn look_up_item( }, item: details, }), - Ok(None) => Err(Skipped { - original, - reason: "Initiative not found".to_owned(), - }), - Err(error) => Err(Skipped { - original, - reason: error.message().to_owned(), - }), + Ok(None) => Err(Skipped::not_found(original, "Initiative")), + Err(error) => Err(Skipped::failed(original, &error)), } } diff --git a/crates/linear-cli/src/commands/issue/archive_or_delete.rs b/crates/linear-cli/src/commands/issue/archive_or_delete.rs index 2c2f8711..06ad7d97 100644 --- a/crates/linear-cli/src/commands/issue/archive_or_delete.rs +++ b/crates/linear-cli/src/commands/issue/archive_or_delete.rs @@ -75,7 +75,7 @@ fn run_bulk(ctx: &Ctx, mode: Mode, request: &Request<'_>) -> Result<()> { }; ctx.eprint(bulk::preview(&found, &missing, "issue", verb))?; if found.is_empty() { - return Err(Error::new("None of the listed issues could be found")); + return Err(bulk::none_found(&missing, "issues")); } let question = format!("{} {}?", mode.title(), bulk::count(found.len(), "issue")); if !request.yes && !ctx.confirm(&question, "--yes")? { @@ -200,15 +200,11 @@ struct Listed { } async fn look_up_item(client: &LinearClient, target: Target) -> Result, Skipped> { - let skipped = |reason: String| Skipped { - original: target.original.clone(), - reason, - }; + let not_found = || Skipped::not_found(target.original.clone(), "Issue"); let id = match target.reference { ReferenceOutcome::Resolved(id) => id, - ReferenceOutcome::Unresolved => return Err(skipped("Issue not found".to_owned())), - // Rows show only the error message, not its suggestion or context. - ReferenceOutcome::Failed(error) => return Err(skipped(error.message().to_owned())), + ReferenceOutcome::Unresolved => return Err(not_found()), + ReferenceOutcome::Failed(error) => return Err(Skipped::failed(target.original, &error)), }; match summary(client, &id).await { Ok(Some(details)) => Ok(Found { @@ -223,8 +219,8 @@ async fn look_up_item(client: &LinearClient, target: Target) -> Result Err(skipped("Issue not found".to_owned())), - Err(error) => Err(skipped(error.message().to_owned())), + Ok(None) => Err(not_found()), + Err(error) => Err(Skipped::failed(target.original, &error)), } } diff --git a/crates/linear-cli/src/commands/issue/commits.rs b/crates/linear-cli/src/commands/issue/commits.rs index c22b4865..3ee53c7c 100644 --- a/crates/linear-cli/src/commands/issue/commits.rs +++ b/crates/linear-cli/src/commands/issue/commits.rs @@ -28,7 +28,7 @@ fn show_commits(ctx: &Ctx, args: &IssueCommits) -> Result<()> { "commit_id", ]))?; if process::text(&probe.stdout).is_empty() { - return Err(Error::not_found("Commits", &identifier)); + return Err(Error::new(format!("Commits not found: {identifier}"))); } // jj reports its own failures, so its exit status becomes ours. process::check_attached(process::status(jj().args([ diff --git a/crates/linear-cli/src/commands/project/create.rs b/crates/linear-cli/src/commands/project/create.rs index ea0c6f22..228b49f7 100644 --- a/crates/linear-cli/src/commands/project/create.rs +++ b/crates/linear-cli/src/commands/project/create.rs @@ -10,7 +10,7 @@ use crate::commands::outcome; use crate::commands::team_key::configured_team_key; use crate::commands::{confirm, lookup_prompt}; use crate::ctx::Ctx; -use crate::error::{Error, Result, ResultExt}; +use crate::error::{Error, Failure, Result, ResultExt}; use crate::graphql::operations::project::ProjectStatusType; use crate::graphql::operations::project::{ AddProjectToInitiative, CreateProject, CreateProjectVariables, CreatedProject, @@ -191,7 +191,7 @@ fn create(ctx: &Ctx, args: &ProjectCreate) -> Result<()> { color ), ))?; - Err(Error::reported()) + Err(Error::reported(error.failure().unwrap_or(Failure::General))) } } } diff --git a/crates/linear-cli/src/commands/project/update.rs b/crates/linear-cli/src/commands/project/update.rs index 6ee11a30..dfaa1c48 100644 --- a/crates/linear-cli/src/commands/project/update.rs +++ b/crates/linear-cli/src/commands/project/update.rs @@ -457,9 +457,14 @@ async fn apply( Err(error) => (FailedWrite::Rejected, Error::from(error)), }; let diagnostic = collections::partial_diagnostic(changes, applied, outcome, updated_fields); - return Err(Error::new(format!("{} Cause: {cause}", diagnostic.message)) - .with_hint(diagnostic.suggestion) - .with_source(cause)); + let failure = cause + .failure() + .expect("a failed write is a runtime failure"); + return Err( + Error::failed(failure, format!("{} Cause: {cause}", diagnostic.message)) + .with_hint(diagnostic.suggestion) + .with_source(cause), + ); } Ok(()) } diff --git a/crates/linear-cli/src/commands/team/delete.rs b/crates/linear-cli/src/commands/team/delete.rs index 9a266d1e..052f5c9d 100644 --- a/crates/linear-cli/src/commands/team/delete.rs +++ b/crates/linear-cli/src/commands/team/delete.rs @@ -154,8 +154,8 @@ fn move_issues( }; let outcome = match client.mutate::(variables).await { Ok(result) if result.issue_update.success => BulkOutcome::Succeeded, - Ok(_) => BulkOutcome::Failed("Linear did not move the issue".to_owned()), - Err(error) => BulkOutcome::Failed(Error::from(error).to_string()), + Ok(_) => BulkOutcome::failed(&Error::new("Linear did not move the issue")), + Err(error) => BulkOutcome::failed(&Error::from(error)), }; BulkResult { id: issue.identifier.clone(), @@ -167,14 +167,18 @@ fn move_issues( present: "move", past: "moved", }; - ctx.print(bulk::summary(&results, "issue", moved).0)?; - let failed = results.iter().filter(|row| !row.succeeded()).count(); - if failed == 0 { + let (summary, failure) = bulk::summary(&results, "issue", moved); + ctx.print(summary)?; + let Some(failure) = failure else { return Ok(()); - } - Err(Error::new(format!( - "{failed} issue(s) could not be moved, so team {} was not deleted", - team.key - )) + }; + let failed = results.iter().filter(|row| !row.succeeded()).count(); + Err(Error::failed( + failure, + format!( + "{failed} issue(s) could not be moved, so team {} was not deleted", + team.key + ), + ) .with_hint("Run the command again to retry.")) } diff --git a/crates/linear-cli/src/commands/upload.rs b/crates/linear-cli/src/commands/upload.rs index 5fb8d1bd..df4f9b42 100644 --- a/crates/linear-cli/src/commands/upload.rs +++ b/crates/linear-cli/src/commands/upload.rs @@ -83,7 +83,7 @@ pub fn resolve_public(content_type: &str, requested: bool) -> Result Result { let info = std::fs::metadata(path).map_err(|error| { if error.kind() == std::io::ErrorKind::NotFound { - Error::not_found("File", &path.to_string_lossy()) + Error::new(format!("File not found: {}", path.to_string_lossy())) } else { Error::new(format!("Failed to read file metadata: {}", path.display())) .with_source(error) diff --git a/crates/linear-cli/src/ctx.rs b/crates/linear-cli/src/ctx.rs index fd5d00d3..e147e9db 100644 --- a/crates/linear-cli/src/ctx.rs +++ b/crates/linear-cli/src/ctx.rs @@ -479,7 +479,7 @@ fn select_credential<'a>( | OptionSource::ProjectConfig { .. } | OptionSource::GlobalConfig { .. } => String::new(), }; - Err(Error::new(format!( + Err(Error::invalid(format!( "Cannot use --workspace while LINEAR_API_KEY is set{place}" )) .with_hint("Unset LINEAR_API_KEY or remove the --workspace flag.")) @@ -488,7 +488,7 @@ fn select_credential<'a>( workspace, choice, stored: true, - } => Err(Error::new(format!( + } => Err(Error::auth(format!( "No usable API key for workspace \"{workspace}\"{}", chosen_by(&choice) )) @@ -499,7 +499,7 @@ fn select_credential<'a>( workspace, choice, stored: false, - } => Err(Error::new(format!( + } => Err(Error::auth(format!( "Workspace \"{workspace}\"{} not found in credentials", chosen_by(&choice) )) @@ -525,7 +525,7 @@ pub fn connect( network_env: &NetworkEnv, ) -> Result { let key = ApiKey::new(secret.expose()).map_err(|error| { - Error::new("API key cannot be used as an HTTP header").with_source(error) + Error::auth("API key cannot be used as an HTTP header").with_source(error) })?; Ok(LinearClient::new( options.endpoint().value().clone(), diff --git a/crates/linear-cli/src/error.rs b/crates/linear-cli/src/error.rs index 1c5839f9..df042e4d 100644 --- a/crates/linear-cli/src/error.rs +++ b/crates/linear-cli/src/error.rs @@ -2,19 +2,72 @@ //! //! An [`Error`] is a message plus an optional chain of context ("Failed to list //! cycles"), a hint line, and a source error shown under `LINEAR_DEBUG`. Its -//! [`ErrorKind`] exists only where the process must behave differently. +//! [`ErrorKind`] exists only where the process must behave differently, and a +//! runtime failure's [`Failure`] class picks its exit status. use std::error::Error as StdError; use std::fmt; use std::num::NonZeroU8; pub type Result = std::result::Result; +/// Why a command failed at run time, which scripts read from the exit status. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Failure { + /// Anything not below: Linear rejected the input, a response could not be + /// used, a local file could not be read. Exit status 1. + General, + /// Something the command looked up in Linear does not exist. Exit status 3. + NotFound, + /// No usable API key, or Linear rejected the key. Exit status 4. + Auth, + /// Linear could not be reached or could not serve the request now (a + /// network failure, a timeout, rate limiting, a server error). Exit status 5. + Unavailable, +} + +impl Failure { + pub fn exit_code(self) -> u8 { + match self { + Self::General => 1, + Self::NotFound => 3, + Self::Auth => 4, + Self::Unavailable => 5, + } + } + + /// The class that describes both failures: the one that needs attention + /// first. Authentication outranks unavailability, which outranks a general + /// failure, which outranks a missing entity, so a run reports "not found" + /// only when nothing worse happened. + pub fn combine(self, other: Self) -> Self { + if self.rank() >= other.rank() { + self + } else { + other + } + } + + /// The combined class of `failures`, or `None` when there are none. + pub fn fold(failures: impl IntoIterator) -> Option { + failures.into_iter().reduce(Self::combine) + } + + fn rank(self) -> u8 { + match self { + Self::NotFound => 0, + Self::General => 1, + Self::Unavailable => 2, + Self::Auth => 3, + } + } +} + #[derive(Debug)] pub enum ErrorKind { - /// An ordinary failure: `✗ message`, exit status 1. - Other, + /// A runtime failure: `✗ message`, with its class's exit status. + Failed(Failure), /// Input that parsed but cannot be used, such as a missing required value - /// or an empty field: reported like [`ErrorKind::Other`], exit status 2 + /// or an empty field: reported like [`ErrorKind::Failed`], exit status 2 /// like any usage error. Invalid, /// A command-line usage error, rendered and given its exit status by clap. @@ -22,7 +75,11 @@ pub enum ErrorKind { /// The user cancelled a prompt (Ctrl-C or Esc) or the editor: exit status /// 130 after `Canceled.`. Cancelled, - /// The command already reported its failure: exit with this status, no message. + /// The command already reported this failure itself: its class's exit + /// status, no message. + Reported(Failure), + /// A child process's status passed on, such as 143 after SIGTERM: exit + /// with it, no message. Exit(NonZeroU8), /// Stdout was closed by its reader: stop quietly with success. BrokenPipe, @@ -52,8 +109,14 @@ impl Error { } } + /// A [`Failure::General`] runtime failure. pub fn new(message: impl Into) -> Self { - Self::with_kind(ErrorKind::Other, message.into()) + Self::failed(Failure::General, message) + } + + /// A runtime failure of class `failure`. + pub fn failed(failure: Failure, message: impl Into) -> Self { + Self::with_kind(ErrorKind::Failed(failure), message.into()) } /// A usage error found after parsing; see [`ErrorKind::Invalid`]. @@ -63,25 +126,33 @@ impl Error { /// Missing or rejected credentials, with a hint to log in. pub fn auth(message: impl Into) -> Self { - Self::new(message).with_hint(LOGIN_HINT) + Self::failed(Failure::Auth, message).with_hint(LOGIN_HINT) } + /// A Linear entity (issue, team, label…) that does not exist. Not for + /// local things such as files, which fail with [`Error::new`]. pub fn not_found(entity: &str, identifier: &str) -> Self { - Self::new(format!("{entity} not found: {identifier}")) + Self::failed( + Failure::NotFound, + format!("{entity} not found: {identifier}"), + ) } pub fn cancelled() -> Self { Self::with_kind(ErrorKind::Cancelled, "Cancelled".to_owned()) } - /// The command printed its own failure report; exit with `status`. + /// Exit with a child process's `status`, which already reported itself. pub fn exit(status: NonZeroU8) -> Self { Self::with_kind(ErrorKind::Exit(status), format!("exit status {status}")) } - /// Exit status 1 after the command printed its own failure report. - pub fn reported() -> Self { - Self::exit(NonZeroU8::MIN) + /// The command printed its own report of a `failure`; exit with its status. + pub fn reported(failure: Failure) -> Self { + Self::with_kind( + ErrorKind::Reported(failure), + format!("exit status {}", failure.exit_code()), + ) } pub(crate) fn broken_pipe(source: std::io::Error) -> Self { @@ -92,6 +163,19 @@ impl Error { &self.kind } + /// The runtime failure class, or `None` for usage errors, cancellation, + /// passed-on child statuses and a closed stdout. + pub fn failure(&self) -> Option { + match &self.kind { + ErrorKind::Failed(failure) | ErrorKind::Reported(failure) => Some(*failure), + ErrorKind::Invalid + | ErrorKind::Usage(_) + | ErrorKind::Cancelled + | ErrorKind::Exit(_) + | ErrorKind::BrokenPipe => None, + } + } + /// The message without context. pub fn message(&self) -> &str { &self.message @@ -134,7 +218,7 @@ impl Error { /// The process exit status this error ends with. pub fn exit_code(&self) -> u8 { match &self.kind { - ErrorKind::Other => 1, + ErrorKind::Failed(failure) | ErrorKind::Reported(failure) => failure.exit_code(), ErrorKind::Invalid => 2, ErrorKind::Usage(error) => u8::try_from(error.exit_code()).unwrap_or(2), ErrorKind::Cancelled => 130, @@ -189,3 +273,6 @@ impl> ResultExt for std::result::Result { self.map_err(|error| error.into().context(context)) } } + +#[cfg(test)] +mod tests; diff --git a/crates/linear-cli/src/error/tests.rs b/crates/linear-cli/src/error/tests.rs new file mode 100644 index 00000000..6e4daec0 --- /dev/null +++ b/crates/linear-cli/src/error/tests.rs @@ -0,0 +1,56 @@ +use super::*; + +const ALL: [Failure; 4] = [ + Failure::General, + Failure::NotFound, + Failure::Auth, + Failure::Unavailable, +]; + +#[test] +fn combining_keeps_the_class_that_needs_attention_first() { + use Failure::{Auth, General, NotFound, Unavailable}; + for (a, b, expected) in [ + (NotFound, NotFound, NotFound), + (NotFound, General, General), + (NotFound, Unavailable, Unavailable), + (NotFound, Auth, Auth), + (General, Unavailable, Unavailable), + (General, Auth, Auth), + (Unavailable, Auth, Auth), + ] { + assert_eq!(a.combine(b), expected, "{a:?} + {b:?}"); + assert_eq!(b.combine(a), expected, "{b:?} + {a:?}"); + } + for failure in ALL { + assert_eq!(failure.combine(failure), failure); + } +} + +#[test] +fn folding_starts_from_the_first_failure() { + assert_eq!(Failure::fold([]), None); + assert_eq!( + Failure::fold([Failure::NotFound, Failure::NotFound]), + Some(Failure::NotFound) + ); + assert_eq!( + Failure::fold([Failure::NotFound, Failure::Unavailable, Failure::General]), + Some(Failure::Unavailable) + ); +} + +#[test] +fn the_class_survives_context_hints_and_message_changes() { + let wrapped = + || -> Result<()> { Err(Error::not_found("Issue", "ENG-1").with_hint("Check the ID.")) }; + let mut error = wrapped() + .context("Failed to view issue") + .expect_err("fails"); + error.push_message("; more"); + assert_eq!(error.failure(), Some(Failure::NotFound)); + assert_eq!(error.exit_code(), 3); + assert_eq!(Error::auth("No API key configured").exit_code(), 4); + assert_eq!(Error::reported(Failure::Unavailable).exit_code(), 5); + assert_eq!(Error::invalid("bad").failure(), None); +} diff --git a/crates/linear-cli/src/graphql/envelope.rs b/crates/linear-cli/src/graphql/envelope.rs index 4af51805..ba5fbb0f 100644 --- a/crates/linear-cli/src/graphql/envelope.rs +++ b/crates/linear-cli/src/graphql/envelope.rs @@ -16,7 +16,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use serde_json::error::Category; -use crate::error::Error; +use crate::error::{Error, Failure}; /// The JSON body sent for one GraphQL operation. /// @@ -160,7 +160,11 @@ impl From for Error { // Valid JSON that contradicts the schema the types were compiled // against is a broken contract, not a network or GraphQL failure. ResponseError::UnexpectedShape(source) => Error::new(message).with_source(source), - ResponseError::GraphQl { .. } | ResponseError::MissingData => Error::new(message), + ResponseError::GraphQl { errors, .. } => Error::failed( + graphql_failure(&errors).expect("a GraphQl error carries at least one error"), + message, + ), + ResponseError::MissingData => Error::new(message), } } } @@ -218,7 +222,33 @@ pub fn graphql_message(errors: &[ResponseGraphQlError]) -> Option { }) } -/// Whether GraphQL errors describe a missing entity. +/// The failure class of a non-empty set of GraphQL errors, combined with +/// [`Failure::combine`]; `None` when there are no errors. +/// +/// Each error is classified on its own: Linear's `AUTHENTICATION_ERROR` and +/// `FORBIDDEN` codes are [`Failure::Auth`], `RATELIMITED` (sent with HTTP 400) +/// is [`Failure::Unavailable`], a missing entity is [`Failure::NotFound`], and +/// anything else is [`Failure::General`]. So errors describe a missing entity +/// only when every one of them does. +pub fn graphql_failure(errors: &[ResponseGraphQlError]) -> Option { + Failure::fold(errors.iter().map(error_failure)) +} + +fn error_failure(error: &ResponseGraphQlError) -> Failure { + let code = error + .extensions + .as_ref() + .and_then(|extensions| extensions.get("code")) + .and_then(Value::as_str); + match code { + Some("AUTHENTICATION_ERROR" | "FORBIDDEN") => Failure::Auth, + Some("RATELIMITED") => Failure::Unavailable, + _ if names_missing_entity(error) => Failure::NotFound, + _ => Failure::General, + } +} + +/// Whether an error says the entity it refers to does not exist. /// /// Linear reports a missing entity with the same `INVALID_INPUT` code as any /// other bad argument, so the message is the only signal: the raw message is @@ -227,11 +257,20 @@ pub fn graphql_message(errors: &[ResponseGraphQlError]) -> Option { /// text; commands go through [`RequestError::is_not_found`]. /// /// [`RequestError::is_not_found`]: crate::client::RequestError::is_not_found -pub fn is_not_found(errors: &[ResponseGraphQlError]) -> bool { - graphql_message(errors).is_some_and(|message| { - let message = message.to_lowercase(); - message.contains("not found") || message.contains("could not find") - }) +fn names_missing_entity(error: &ResponseGraphQlError) -> bool { + let presentable = error + .extensions + .as_ref() + .and_then(|extensions| extensions.get("userPresentableMessage")) + .and_then(Value::as_str); + [Some(error.message.as_str()), presentable] + .into_iter() + .flatten() + .any(|message| { + let message = message.to_lowercase(); + message.starts_with("entity not found") + || message.starts_with("could not find referenced") + }) } #[cfg(test)] diff --git a/crates/linear-cli/src/graphql/envelope/tests.rs b/crates/linear-cli/src/graphql/envelope/tests.rs index 47bd9231..2b5e9d3b 100644 --- a/crates/linear-cli/src/graphql/envelope/tests.rs +++ b/crates/linear-cli/src/graphql/envelope/tests.rs @@ -1,4 +1,5 @@ -use super::{GraphQlRequest, ResponseError, graphql_message, is_not_found, parse_response}; +use super::{GraphQlRequest, ResponseError, graphql_failure, graphql_message, parse_response}; +use crate::error::Failure; use crate::graphql::operations::agent_session::GetAgentSessionDetails; use crate::graphql::operations::issue::UpdateIssue; use serde_json::Value; @@ -37,7 +38,7 @@ fn errors_only_classify_as_graphql_without_partial_data() { graphql_message(errors).as_deref(), Some("Could not find referenced Issue.") ); - assert!(is_not_found(errors)); + assert_eq!(graphql_failure(errors), Some(Failure::NotFound)); } other => panic!("expected GraphQl, got {other:?}"), } @@ -60,7 +61,7 @@ fn errors_with_partial_data_are_still_errors() { graphql_message(&errors).as_deref(), Some("Something failed") ); - assert!(!is_not_found(&errors)); + assert_eq!(graphql_failure(&errors), Some(Failure::General)); } other => panic!("expected GraphQl, got {other:?}"), } @@ -108,7 +109,7 @@ fn errors_with_null_root_field_incompatible_with_the_type_are_graphql_errors() { } => { assert!(partial_data); assert_eq!(errors.len(), 1); - assert!(is_not_found(errors)); + assert_eq!(graphql_failure(errors), Some(Failure::NotFound)); } other => panic!("expected GraphQl, got {other:?}"), } @@ -123,7 +124,7 @@ fn errors_with_null_root_field_incompatible_with_the_type_are_graphql_errors() { } => { assert!(partial_data); assert_eq!(errors[0].message, "Entity not found: AgentSession"); - assert!(is_not_found(errors)); + assert_eq!(graphql_failure(errors), Some(Failure::NotFound)); } other => panic!("expected GraphQl, got {other:?}"), } diff --git a/crates/linear-cli/src/platform/process.rs b/crates/linear-cli/src/platform/process.rs index ce803d25..ed6a7494 100644 --- a/crates/linear-cli/src/platform/process.rs +++ b/crates/linear-cli/src/platform/process.rs @@ -5,7 +5,7 @@ use std::path::Path; use std::process::{Command, ExitStatus, Output, Stdio}; use crate::config::ChildEnvOverlay; -use crate::error::{Error, Result}; +use crate::error::{Error, Failure, Result}; /// `program` run in `cwd` with the configured child environment on top of /// this process's own, and with stdin closed. @@ -74,7 +74,7 @@ pub fn check_attached(status: ExitStatus) -> Result<()> { .and_then(|code| u8::try_from(code).ok()) .and_then(NonZeroU8::new), }; - Err(interrupted.map_or_else(Error::reported, Error::exit)) + Err(interrupted.map_or_else(|| Error::reported(Failure::General), Error::exit)) } #[cfg(unix)] diff --git a/crates/linear-cli/tests/cli/api.rs b/crates/linear-cli/tests/cli/api.rs index be5e7672..98159f82 100644 --- a/crates/linear-cli/tests/cli/api.rs +++ b/crates/linear-cli/tests/cli/api.rs @@ -75,7 +75,7 @@ fn missing_document_fails_before_any_request() { let api = MockLinear::start(); Cli::for_api(&api) .run(&["api"]) - .failure() + .usage_error() .stderr_has("No query"); assert!(api.requests().is_empty()); } @@ -85,10 +85,10 @@ fn invalid_variables_json_fails_before_any_request() { let api = MockLinear::start(); let cli = Cli::for_api(&api); cli.run(&["api", QUERY, "--variables-json", "{nope"]) - .failure() + .usage_error() .stderr_has("--variables-json"); cli.run(&["api", QUERY, "--variables-json", "[1]"]) - .failure() + .usage_error() .stderr_has("object"); cli.run(&["api", QUERY, "--variable", "body=@missing.md"]) .failure() @@ -130,6 +130,64 @@ fn silent_suppresses_output_but_keeps_the_exit_status() { assert_eq!(run.stdout, ""); } +#[test] +fn the_exit_status_names_the_failure_class() { + let unauthenticated = r#"{"errors":[{"message":"Authentication required, not authenticated","extensions":{"code":"AUTHENTICATION_ERROR","userPresentableMessage":"You need to authenticate to access this operation."}}]}"#; + let missing = r#"{"errors":[{"message":"Entity not found: Issue","path":["issue"],"extensions":{"code":"INPUT_ERROR","userPresentableMessage":"Could not find referenced Issue."}}],"data":null}"#; + let invalid = r#"{"errors":[{"message":"Cannot query field \"nope\" on type \"Query\".","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}]}"#; + let limited = + r#"{"errors":[{"message":"Rate limit exceeded","extensions":{"code":"RATELIMITED"}}]}"#; + let api = MockLinear::start(); + api.on_raw("Probe", 401, unauthenticated) + .on_raw("Probe", 200, missing) + .on_raw("Probe", 400, invalid) + .on_raw("Probe", 400, limited) + .on_text("Probe", 502, "text/html", "bad gateway") + .on_raw("Probe", 401, unauthenticated); + let cli = Cli::for_api(&api); + cli.run(&["api", QUERY]) + .auth_failure() + .stderr_has("AUTHENTICATION_ERROR"); + let run = cli.run(&["api", QUERY]); + run.not_found(); + assert_eq!( + run.json()["errors"][0]["message"], + "Entity not found: Issue" + ); + cli.run(&["api", QUERY]) + .failure() + .stderr_has("GRAPHQL_VALIDATION_FAILED"); + cli.run(&["api", QUERY]).unavailable(); + cli.run(&["api", QUERY]).unavailable(); + let run = cli.run(&["api", QUERY, "--silent"]); + run.auth_failure(); + assert_eq!((run.stdout.as_str(), run.stderr.as_str()), ("", "")); +} + +#[test] +fn an_unreachable_endpoint_exits_5() { + Cli::new() + .env("LINEAR_API_KEY", API_KEY) + .env("LINEAR_GRAPHQL_ENDPOINT", "http://127.0.0.1:1/graphql") + .run(&["api", QUERY]) + .unavailable() + .stderr_has("127.0.0.1:1"); +} + +#[test] +fn a_failed_later_page_keeps_its_class() { + const ISSUES: &str = "query Issues($after: String) { issues(first: 1, after: $after) { nodes { id } pageInfo { hasNextPage endCursor } } }"; + let api = MockLinear::start(); + api.on( + "Issues", + json!({ "issues": { "nodes": [{ "id": "a" }], "pageInfo": { "hasNextPage": true, "endCursor": "c1" } } }), + ) + .on_text("Issues", 503, "text/plain", "maintenance"); + Cli::for_api(&api) + .run(&["api", ISSUES, "--paginate", "--silent"]) + .unavailable(); +} + #[test] fn paginate_follows_cursors_and_prints_every_node() { const ISSUES: &str = "query Issues($after: String) { issues(first: 2, after: $after) { nodes { id } pageInfo { hasNextPage endCursor } } }"; @@ -178,10 +236,10 @@ fn responses_that_are_not_graphql_json_are_printed_raw_and_fail() { run.failure(); assert_eq!(run.stdout, " not json \n"); let run = cli.run(&["api", QUERY]); - run.failure().stderr_has("upstream exploded"); + run.unavailable().stderr_has("upstream exploded"); assert_eq!(run.stdout, ""); let run = cli.run(&["api", QUERY, "--silent"]); - run.failure(); + run.unavailable(); assert_eq!((run.stdout.as_str(), run.stderr.as_str()), ("", "")); } diff --git a/crates/linear-cli/tests/cli/auth.rs b/crates/linear-cli/tests/cli/auth.rs index beb60808..2453c2bd 100644 --- a/crates/linear-cli/tests/cli/auth.rs +++ b/crates/linear-cli/tests/cli/auth.rs @@ -26,7 +26,7 @@ fn env_key_is_used() { fn missing_key_fails_with_guidance() { Cli::new() .run(&["auth", "token"]) - .failure() + .auth_failure() .stderr_has("No API key configured") .stderr_has("linear auth login"); } @@ -50,7 +50,7 @@ fn unknown_workspace_fails() { Cli::new() .credentials(INLINE) .run(&["auth", "token", "--workspace", "nope"]) - .failure() + .auth_failure() .stderr_has("\"nope\""); } @@ -60,7 +60,7 @@ fn env_key_conflicts_with_workspace_flag() { .credentials(INLINE) .env("LINEAR_API_KEY", "lin_env") .run(&["auth", "token", "--workspace", "acme"]) - .failure() + .usage_error() .stderr_has("--workspace"); } @@ -155,7 +155,7 @@ fn rejected_login_key_is_not_saved() { ); let cli = Cli::new().endpoint(&api); cli.run(&["auth", "login", "--key", "lin_bad", "--plaintext"]) - .failure(); + .auth_failure(); assert!(!cli.path("home/.config/linear/credentials.toml").exists()); } @@ -166,7 +166,7 @@ fn default_switches_the_default_workspace() { .success() .stdout_has("acme"); assert_eq!(token(&cli, &[]), "key-acme"); - cli.run(&["auth", "default", "nope"]).failure(); + cli.run(&["auth", "default", "nope"]).usage_error(); } #[test] @@ -265,7 +265,7 @@ fn login_reports_an_authentication_error_as_an_invalid_key() { ); let cli = Cli::new().endpoint(&api); cli.run(&["auth", "login", "--key", "lin_bad", "--plaintext"]) - .failure() + .auth_failure() .stderr_has("Invalid API key"); assert!(!cli.path(CREDENTIALS).exists()); } @@ -433,7 +433,7 @@ fn a_configured_workspace_without_credentials_never_falls_back_to_the_default() .credentials(INLINE) .file("cwd/.linear.toml", "workspace = \"ghost\"\n"); let run = cli.run(&["auth", "token"]); - run.failure() + run.auth_failure() .stderr_has("Workspace \"ghost\" (workspace set in project config") .stderr_has("not found in credentials"); assert!(!run.stdout.contains("key-beta"), "{run}"); @@ -441,7 +441,7 @@ fn a_configured_workspace_without_credentials_never_falls_back_to_the_default() .credentials(INLINE) .env("LINEAR_WORKSPACE", "ghost") .run(&["auth", "token"]); - run.failure() + run.auth_failure() .stderr_has("(workspace set in process environment)"); } @@ -452,7 +452,7 @@ fn a_dotenv_api_key_conflict_names_the_file() { .file("cwd/.env", "LINEAR_API_KEY=lin_env\n") .env_remove("LINEAR_IGNORE_ENV_FILE") .run(&["auth", "token", "--workspace", "acme"]) - .failure() + .usage_error() .stderr_has("Cannot use --workspace while LINEAR_API_KEY is set in ") .stderr_has(".env"); } diff --git a/crates/linear-cli/tests/cli/config.rs b/crates/linear-cli/tests/cli/config.rs index fdbe27e4..07e32c90 100644 --- a/crates/linear-cli/tests/cli/config.rs +++ b/crates/linear-cli/tests/cli/config.rs @@ -70,7 +70,7 @@ fn project_config_is_found_at_the_repository_root() { #[test] fn dotenv_supplies_linear_variables_unless_ignored() { let cli = Cli::new().file("cwd/.env", "LINEAR_API_KEY=key-dotenv\n"); - cli.run(&["auth", "token"]).failure(); + cli.run(&["auth", "token"]).auth_failure(); let cli = cli.env_remove("LINEAR_IGNORE_ENV_FILE"); assert_eq!(stdout(&cli, &["auth", "token"]), "key-dotenv"); let cli = cli.env("LINEAR_API_KEY", "key-env"); @@ -84,7 +84,7 @@ fn dotenv_never_sends_a_variable_reference_as_the_key() { .env_remove("LINEAR_IGNORE_ENV_FILE") .env("SECRET_KEY", "key-secret"); cli.run(&["auth", "token"]) - .failure() + .auth_failure() .stderr_has("Ignoring LINEAR_API_KEY") .stderr_has("${SECRET_KEY} would be used literally") .stderr_has("single-quote"); @@ -198,6 +198,6 @@ fn config_without_a_terminal_names_the_missing_flags() { fn config_without_credentials_points_to_login() { Cli::new() .run(&["config"]) - .failure() + .auth_failure() .stderr_has("linear auth login"); } diff --git a/crates/linear-cli/tests/cli/cycle.rs b/crates/linear-cli/tests/cli/cycle.rs index e0a6157f..ce39872b 100644 --- a/crates/linear-cli/tests/cli/cycle.rs +++ b/crates/linear-cli/tests/cli/cycle.rs @@ -158,7 +158,7 @@ fn view_unknown_cycle_fails_without_fetching_details() { .on("GetTeamCyclesForLookup", lookup(Value::Null)); Cli::for_api(&api) .run(&["cycle", "view", "42", "--team", "ENG"]) - .failure() + .not_found() .stderr_has("42"); } diff --git a/crates/linear-cli/tests/cli/document.rs b/crates/linear-cli/tests/cli/document.rs index 06aa8876..dc574844 100644 --- a/crates/linear-cli/tests/cli/document.rs +++ b/crates/linear-cli/tests/cli/document.rs @@ -179,7 +179,7 @@ fn view_missing_document_fails() { api.on("GetDocument", json!({ "document": null })); Cli::for_api(&api) .run(&["document", "view", "gone123", "--raw"]) - .failure(); + .not_found(); } #[test] @@ -636,7 +636,7 @@ fn comment_add_reports_a_missing_document() { api.on_error("GetDocumentCommentTarget", "Entity not found: Document"); Cli::for_api(&api) .run(&["document", "comment", "add", "gone", "--body", "Hi"]) - .failure() + .not_found() .stderr_has("Document not found: gone"); assert_eq!(api.operations(), ["GetDocumentCommentTarget"]); } @@ -1000,7 +1000,7 @@ fn delete_bulk_lists_the_documents_before_deleting_and_skips_missing_ones() { run.failure() .stderr_has("1 document to delete:\n Design notes\n") .stderr_has( - "Skipping 1 document that could not be found:\n https://linear.app/acme/issue/ENG-1: ", + "Skipping 1 document that could not be looked up:\n https://linear.app/acme/issue/ENG-1: ", ); assert_eq!(api.operations(), ["GetDocumentForDelete", "DeleteDocument"]); } @@ -1051,7 +1051,7 @@ fn create_on_a_terminal_checks_the_attachment_flag_before_the_editor() { &["document", "create", "-t", "Notes", "--project", "nope"], &[], ); - assert_eq!(run.code, 1, "{run}"); + assert_eq!(run.code, 3, "{run}"); assert!(run.stdout.contains("Project not found: nope"), "{run}"); assert!(cli.calls("editor").is_empty()); } diff --git a/crates/linear-cli/tests/cli/errors.rs b/crates/linear-cli/tests/cli/errors.rs index 34187298..d04c2628 100644 --- a/crates/linear-cli/tests/cli/errors.rs +++ b/crates/linear-cli/tests/cli/errors.rs @@ -40,7 +40,7 @@ fn failures_show_the_first_nonempty_graphql_message_or_the_http_status() { .stderr_has("boom"); assert_eq!(run.stdout, ""); let run = cli.run(&args); - run.failure() + run.unavailable() .stderr_has("✗ Failed to query issues: ") .stderr_has("500"); assert_eq!(run.stdout, ""); @@ -57,7 +57,7 @@ fn network_failures_show_a_cause_chain_without_secrets() { ) .env("LINEAR_DEBUG", "1") .run(&["auth", "whoami"]); - run.failure() + run.unavailable() .stderr_has("✗ ") .stderr_has("http://127.0.0.1:1") .stderr_has(" caused by: "); @@ -79,7 +79,7 @@ fn http_failures_show_a_sanitized_body_excerpt() { let plain = Cli::for_api(&api).run(&["auth", "whoami"]); plain - .failure() + .unavailable() .stderr_has("unexpected HTTP status 500 Internal Server Error: upstream [31mexploded[0m key= xxx") .stderr_has("x…"); assert!(!plain.stderr.contains('\x1b'), "{plain}"); @@ -90,13 +90,13 @@ fn http_failures_show_a_sanitized_body_excerpt() { .env("LINEAR_DEBUG", "1") .run(&["auth", "whoami"]); debug - .failure() + .unavailable() .stderr_has(" debug: HTTP 500 Internal Server Error body: upstream") .stderr_has(&"x".repeat(300)); assert!(!debug.stderr.contains(crate::support::API_KEY), "{debug}"); let html = Cli::for_api(&api).run(&["auth", "whoami"]); - html.failure() + html.unavailable() .stderr_has("unexpected HTTP status 502 Bad Gateway\n"); assert!(!html.stderr.contains(""), "{html}"); } @@ -107,7 +107,7 @@ fn missing_credentials_fail_with_a_login_hint() { Cli::new() .endpoint(&api) .run(&["team", "list"]) - .failure() + .auth_failure() .stderr_has("No API key configured") .stderr_has("linear auth login"); assert!(api.requests().is_empty()); @@ -148,3 +148,107 @@ fn a_closed_stdout_ends_the_command_quietly() { assert_eq!(output.status.code(), Some(0)); assert!(output.stderr.is_empty(), "{output:?}"); } + +/// Linear's answer to a revoked or mistyped API key. +const UNAUTHENTICATED: &str = r#"{"errors":[{"message":"Authentication required, not authenticated","extensions":{"type":"authentication error","code":"AUTHENTICATION_ERROR","statusCode":401,"userError":true,"userPresentableMessage":"You need to authenticate to access this operation.","meta":{},"http":{"status":401}}}]}"#; +const ISSUE_NOT_FOUND: &str = r#"{"errors":[{"message":"Entity not found: Issue","extensions":{"type":"invalid input","code":"INPUT_ERROR","statusCode":400,"userError":true,"userPresentableMessage":"Could not find referenced Issue."}}],"data":null}"#; +const RATE_LIMITED: &str = + r#"{"errors":[{"message":"Rate limit exceeded","extensions":{"code":"RATELIMITED"}}]}"#; + +const VIEW: [&str; 4] = ["issue", "view", "ENG-1", "--json"]; +const VIEW_OP: &str = "GetIssueDetailsWithComments"; + +#[test] +fn a_rejected_api_key_exits_4() { + let api = MockLinear::start(); + api.on_raw(VIEW_OP, 401, UNAUTHENTICATED) + .on_raw(VIEW_OP, 403, r#"{"errors":[{"message":"Forbidden"}]}"#) + .on_raw(VIEW_OP, 200, UNAUTHENTICATED); + let cli = Cli::for_api(&api); + cli.run(&VIEW) + .auth_failure() + .stderr_has("You need to authenticate"); + cli.run(&VIEW).auth_failure(); + cli.run(&VIEW).auth_failure(); +} + +#[test] +fn missing_credentials_exit_4() { + Cli::new() + .run(&VIEW) + .auth_failure() + .stderr_has("No API key configured"); + Cli::new() + .credentials("default = \"acme\"\nacme = \"lin_api_x\"\n") + .run(&["--workspace", "other", "issue", "view", "ENG-1"]) + .auth_failure() + .stderr_has("not found in credentials"); +} + +#[test] +fn a_missing_issue_exits_3() { + let api = MockLinear::start(); + api.on_raw(VIEW_OP, 200, ISSUE_NOT_FOUND); + Cli::for_api(&api) + .run(&VIEW) + .not_found() + .stderr_has("Issue not found: ENG-1"); +} + +#[test] +fn a_missing_entity_alongside_a_rejected_key_is_an_authentication_failure() { + let both = r#"{"errors":[{"message":"Entity not found: Issue"},{"message":"Authentication required","extensions":{"code":"AUTHENTICATION_ERROR"}}]}"#; + let api = MockLinear::start(); + api.on_raw(VIEW_OP, 200, both); + Cli::for_api(&api).run(&VIEW).auth_failure(); +} + +#[test] +fn an_unreachable_or_struggling_api_exits_5() { + // Nothing listens on port 1, so the connection is refused. + Cli::new() + .env("LINEAR_API_KEY", crate::support::API_KEY) + .env("LINEAR_GRAPHQL_ENDPOINT", "http://127.0.0.1:1/graphql") + .run(&VIEW) + .unavailable(); + let api = MockLinear::start(); + api.on_text(VIEW_OP, 503, "text/plain", "maintenance") + .on_text(VIEW_OP, 429, "text/plain", "slow down") + .on_raw(VIEW_OP, 400, RATE_LIMITED); + let cli = Cli::for_api(&api); + cli.run(&VIEW).unavailable(); + cli.run(&VIEW).unavailable(); + cli.run(&VIEW) + .unavailable() + .stderr_has("Rate limit exceeded"); +} + +#[test] +fn other_graphql_errors_and_unexpected_statuses_exit_1() { + let api = MockLinear::start(); + api.on_raw( + VIEW_OP, + 400, + r#"{"errors":[{"message":"Argument invalid","extensions":{"code":"INPUT_ERROR"}}]}"#, + ) + .on_text(VIEW_OP, 404, "text/plain", "no such route") + .on_raw( + VIEW_OP, + 200, + r#"{"errors":[{"message":"Entity not found: Issue"},{"message":"Something else broke"}]}"#, + ); + let cli = Cli::for_api(&api); + cli.run(&VIEW).failure(); + cli.run(&VIEW).failure(); + cli.run(&VIEW).failure(); +} + +#[test] +fn root_help_documents_the_exit_statuses() { + let run = Cli::new().run(&["--help"]); + run.success() + .stdout_has("Exit status:") + .stdout_has("3 ") + .stdout_has("4 ") + .stdout_has("5 "); +} diff --git a/crates/linear-cli/tests/cli/initiative.rs b/crates/linear-cli/tests/cli/initiative.rs index 2a80b26a..67d0cb46 100644 --- a/crates/linear-cli/tests/cli/initiative.rs +++ b/crates/linear-cli/tests/cli/initiative.rs @@ -338,7 +338,7 @@ fn view_of_a_missing_url_never_falls_back_to_a_name() { api.on("ResolveInitiativeBySlug", none()); Cli::for_api(&api) .run(&["initiative", "view", URL]) - .failure() + .not_found() .stderr_has("Initiative not found") .stderr_has("may have been deleted"); assert_eq!(api.operations(), ["ResolveInitiativeBySlug"]); @@ -351,7 +351,7 @@ fn view_unknown_initiative_fails() { .on("ResolveInitiativeByName", none()); Cli::for_api(&api) .run(&["initiative", "view", "nothing-here"]) - .failure() + .not_found() .stderr_has("nothing-here"); } diff --git a/crates/linear-cli/tests/cli/issue_attach.rs b/crates/linear-cli/tests/cli/issue_attach.rs index 6f583175..73cb5f31 100644 --- a/crates/linear-cli/tests/cli/issue_attach.rs +++ b/crates/linear-cli/tests/cli/issue_attach.rs @@ -171,7 +171,7 @@ fn link_to_a_missing_issue_fails() { api.on_error("GetIssueId", "Entity not found: Issue"); Cli::for_api(&api) .run(&["issue", "link", "ENG-1", "https://example.com/a"]) - .failure() + .not_found() .stderr_has("ENG-1"); } @@ -334,7 +334,7 @@ fn relation_delete_fails_when_no_relation_matches() { .run(&[ "issue", "relation", "delete", "ENG-1", "blocks", "ENG-2", "--yes", ]) - .failure() + .not_found() .stderr_has("not found"); } diff --git a/crates/linear-cli/tests/cli/issue_comment.rs b/crates/linear-cli/tests/cli/issue_comment.rs index 7f9fc1cb..c92f611e 100644 --- a/crates/linear-cli/tests/cli/issue_comment.rs +++ b/crates/linear-cli/tests/cli/issue_comment.rs @@ -364,7 +364,7 @@ fn delete_reports_an_unknown_comment_before_asking() { api.on_error("GetCommentForDelete", "Entity not found: Comment"); Cli::for_api(&api) .run(&["issue", "comment", "delete", COMMENT_ID, "--yes"]) - .failure() + .not_found() .stderr_has(&format!("Comment not found: {COMMENT_ID}")); assert_eq!(api.operations(), ["GetCommentForDelete"]); } @@ -611,7 +611,7 @@ fn add_on_a_terminal_does_not_open_the_editor_for_a_missing_issue() { .stub_bin("editor", APPENDING_EDITOR) .env("VISUAL", "editor"); cli.run_tty(&["issue", "comment", "add", "ENG-404"], &[]) - .failure() + .not_found() .stdout_has("Issue not found: ENG-404"); assert!(cli.calls("editor").is_empty()); } @@ -700,7 +700,7 @@ fn add_on_a_terminal_checks_the_parent_before_the_editor() { &["issue", "comment", "add", "ENG-1", "--reply-to", PARENT_ID], &[], ) - .failure() + .not_found() .stdout_has(&format!("Comment not found: {PARENT_ID}")); assert!(cli.calls("editor").is_empty()); } diff --git a/crates/linear-cli/tests/cli/issue_read.rs b/crates/linear-cli/tests/cli/issue_read.rs index 4e22ec88..9d61158c 100644 --- a/crates/linear-cli/tests/cli/issue_read.rs +++ b/crates/linear-cli/tests/cli/issue_read.rs @@ -194,10 +194,10 @@ fn view_and_title_name_a_missing_issue() { .on_raw("GetIssueDetails", 200, ISSUE_NOT_FOUND); let cli = Cli::for_api(&api); cli.run(&["issue", "view", "ENG-9999"]) - .failure() + .not_found() .stderr_has("Issue not found: ENG-9999"); let run = cli.run(&["issue", "title", "ENG-9999"]); - run.failure().stderr_has("Issue not found: ENG-9999"); + run.not_found().stderr_has("Issue not found: ENG-9999"); assert!(!run.stderr.contains("Could not find"), "{run}"); } @@ -489,14 +489,14 @@ fn list_and_query_reject_a_label_the_issues_cannot_have() { .on("GetLabelByName", json!({ "issueLabels": { "nodes": [] } })); let cli = Cli::for_api(&api).env("LINEAR_TEAM_ID", "ENG"); cli.run(&["issue", "list", "--label", "nope"]) - .failure() + .not_found() .stderr_has("Issue label not found: nope") .stderr_has("linear label list --team ENG"); cli.run(&["issue", "list", "--label", "Bug"]) - .failure() + .not_found() .stderr_has("Issue label not found: Bug"); cli.run(&["issue", "query", "--all-teams", "--label", "nope"]) - .failure() + .not_found() .stderr_has("Issue label not found: nope") .stderr_has("linear label list --all-teams"); assert_eq!( @@ -914,7 +914,7 @@ fn query_unknown_state_fails() { ); Cli::for_api(&api) .run(&["issue", "query", "--all-teams", "--state", "Absent"]) - .failure() + .not_found() .stderr_has("Absent"); assert_eq!( api.variables("GetWorkflowStatesInScope"), @@ -953,7 +953,7 @@ fn query_reports_api_errors() { ); Cli::for_api(&api) .run(&["issue", "query", "--all-teams", "--json"]) - .failure(); + .unavailable(); } /// An issue whose description embeds an image served by `api`. @@ -1109,7 +1109,7 @@ fn view_of_a_missing_issue_is_not_found() { api.on("GetIssueDetailsWithComments", json!({ "issue": null })); Cli::for_api(&api) .run(&["issue", "view", "ENG-404", "--json"]) - .failure() + .not_found() .stderr_has("Issue not found: ENG-404"); } @@ -1323,7 +1323,7 @@ fn missing_state_hints_quote_names_in_single_and_multiple_team_scopes() { argv.extend(scope); let run = Cli::for_api(&api).run(&argv); if command == "list" { - run.failure() + run.not_found() .stderr_has("Workflow state not found: 'Absent' in team ENG"); } let expected = if command == "list" { @@ -1331,7 +1331,7 @@ fn missing_state_hints_quote_names_in_single_and_multiple_team_scopes() { } else { r#"Valid states: "Bell\u0007" (unstarted, ENG), "Say \"hi\"" (started, OPS)."# }; - run.failure().stderr_has(expected); + run.not_found().stderr_has(expected); assert!(!run.stderr.contains('\u{7}')); assert!( api.operations() diff --git a/crates/linear-cli/tests/cli/issue_write.rs b/crates/linear-cli/tests/cli/issue_write.rs index 987ac7f3..7b1100a2 100644 --- a/crates/linear-cli/tests/cli/issue_write.rs +++ b/crates/linear-cli/tests/cli/issue_write.rs @@ -296,7 +296,7 @@ fn create_reports_an_unknown_template_id_as_not_found() { Cli::for_api(&api) .env("LINEAR_TEAM_ID", "ENG") .run(&["issue", "create", "--no-interactive", "--template", id]) - .failure() + .not_found() .stderr_has(&format!("Template not found: {id}")); } @@ -319,7 +319,7 @@ fn create_fails_on_an_unknown_label_without_creating() { "-l", "missing", ]) - .failure() + .not_found() .stderr_has("missing"); assert_eq!( api.variables("GetIssueLabelIdByNameForTeam"), @@ -580,7 +580,7 @@ fn update_of_a_missing_issue_names_it_and_prints_nothing_first() { r#"{"errors":[{"message":"Entity not found: Issue","extensions":{"userPresentableMessage":"Could not find referenced Issue."}}]}"#, ); let run = Cli::for_api(&api).run(&["issue", "update", "ENG-9999", "-t", "x"]); - run.failure().stderr_has("Issue not found: ENG-9999"); + run.not_found().stderr_has("Issue not found: ENG-9999"); assert_eq!(run.stdout, "", "{run}"); } @@ -705,7 +705,7 @@ fn delete_reports_a_missing_issue() { ); Cli::for_api(&api) .run(&["issue", "delete", "ENG-404", "-y"]) - .failure() + .not_found() .stderr_has("Issue not found: ENG-404"); } @@ -845,7 +845,7 @@ fn missing_state_hints_quote_names_before_issue_mutations() { vec!["issue", "update", "ENG-1", "--state", "Absent"] }; let run = Cli::for_api(&api).run(&argv); - run.failure() + run.not_found() .stderr_has("Workflow state not found: 'Absent' in team ENG") .stderr_has(r#"Valid states: "Bell\u0007" (unstarted), "Say \"hi\"" (started)."#); assert!(!run.stderr.contains('\u{7}')); @@ -942,7 +942,7 @@ fn create_reports_a_missing_parent_without_mutating() { "--parent", "ENG-404", ]) - .failure() + .not_found() .stderr_has("Parent issue not found: ENG-404"); assert_eq!(api.operations(), ["ResolveTeam", "GetIssueId"]); assert_eq!(api.variables("GetIssueId"), json!({"id": "ENG-404"})); @@ -956,7 +956,7 @@ fn update_reports_a_missing_parent_without_mutating() { Cli::for_api(&api) .env("LINEAR_TEAM_ID", "ENG") .run(&["issue", "update", "ENG-1", "--parent", "ENG-404"]) - .failure() + .not_found() .stderr_has("Parent issue not found: ENG-404"); assert_eq!(api.operations(), ["GetIssueId"]); assert_eq!(api.variables("GetIssueId"), json!({"id": "ENG-404"})); @@ -1278,7 +1278,7 @@ fn bulk_delete_lists_what_it_found_and_skips_before_asking() { ); assert_eq!(run.code, 1, "{run}"); let listed = "1 issue to delete:\n ENG-1: Fix login\n\ - Skipping 1 issue that could not be found:\n 3: Issue number 3 needs a team"; + Skipping 1 issue that could not be looked up:\n 3: Issue number 3 needs a team"; assert!(run.stdout.contains(listed), "{run}"); assert!( run.stdout.contains("Completed: 1/2 issues deleted"), @@ -1287,12 +1287,69 @@ fn bulk_delete_lists_what_it_found_and_skips_before_asking() { assert_eq!(api.operations(), ["GetIssueSummary", "DeleteIssue"]); } +fn summary(id: &str) -> Value { + json!({ "issue": { "identifier": id, "title": "t", "archivedAt": null } }) +} + +#[test] +fn a_bulk_run_with_a_missing_issue_exits_3_after_archiving_the_rest() { + let api = MockLinear::start(); + api.on("GetIssueSummary", summary("ENG-1")) + .on_raw("GetIssueSummary", 200, MISSING_ISSUE) + .on( + "ArchiveIssue", + json!({ "issueArchive": { "success": true } }), + ); + Cli::for_api(&api) + .run(&["issue", "archive", "--yes", "--bulk", "ENG-1", "ENG-404"]) + .not_found() + .stdout_has("Completed: 1/2 issues archived"); +} + +#[test] +fn a_bulk_run_exits_with_its_most_serious_failure() { + let api = MockLinear::start(); + api.on_raw("GetIssueSummary", 200, MISSING_ISSUE).on_text( + "GetIssueSummary", + 503, + "text/plain", + "maintenance", + ); + Cli::for_api(&api) + .run(&["issue", "archive", "--yes", "--bulk", "ENG-1", "ENG-2"]) + .unavailable() + .stderr_has("None of the listed issues could be looked up"); + + let api = MockLinear::start(); + let refused = r#"{"errors":[{"message":"Authentication required","extensions":{"code":"AUTHENTICATION_ERROR"}}]}"#; + api.on_raw("GetIssueSummary", 401, refused) + .on_raw("GetIssueSummary", 401, refused); + Cli::for_api(&api) + .run(&["issue", "archive", "--yes", "--bulk", "ENG-1", "ENG-2"]) + .auth_failure() + .stderr_has("Skipping 2 issues that could not be looked up:"); + assert_eq!(api.operations(), ["GetIssueSummary", "GetIssueSummary"]); + + let api = MockLinear::start(); + api.on("GetIssueSummary", summary("ENG-1")) + .on("GetIssueSummary", summary("ENG-2")) + .on( + "ArchiveIssue", + json!({ "issueArchive": { "success": true } }), + ) + .on_text("ArchiveIssue", 503, "text/plain", "maintenance"); + Cli::for_api(&api) + .run(&["issue", "archive", "--yes", "--bulk", "ENG-1", "ENG-2"]) + .unavailable() + .stdout_has("Completed: 1/2 issues archived"); +} + #[test] fn bulk_archive_with_nothing_found_asks_nothing() { let api = MockLinear::start(); api.on_raw("GetIssueSummary", 200, MISSING_ISSUE); let run = Cli::for_api(&api).run_tty(&["issue", "archive", "--bulk", "ENG-404"], &[]); - assert_eq!(run.code, 1, "{run}"); + assert_eq!(run.code, 3, "{run}"); assert!(run.stdout.contains("ENG-404: Issue not found"), "{run}"); assert!( run.stdout diff --git a/crates/linear-cli/tests/cli/label.rs b/crates/linear-cli/tests/cli/label.rs index b98a8700..24287968 100644 --- a/crates/linear-cli/tests/cli/label.rs +++ b/crates/linear-cli/tests/cli/label.rs @@ -320,7 +320,7 @@ fn delete_missing_label_is_not_found() { api.on("GetLabelByName", by_name(vec![])); Cli::for_api(&api) .run(&["label", "delete", "Nope", "--yes"]) - .failure() + .not_found() .stderr_has("Label not found: Nope"); } diff --git a/crates/linear-cli/tests/cli/milestone.rs b/crates/linear-cli/tests/cli/milestone.rs index 31bab009..c4f1342e 100644 --- a/crates/linear-cli/tests/cli/milestone.rs +++ b/crates/linear-cli/tests/cli/milestone.rs @@ -101,7 +101,7 @@ fn list_of_a_missing_project_is_not_found() { api.on("GetProjectMilestones", json!({ "project": null })); Cli::for_api(&api) .run(&["milestone", "list", "--project", PROJECT_ID]) - .failure() + .not_found() .stderr_has("Failed to list milestones: Project not found"); } @@ -458,7 +458,7 @@ fn create_with_unknown_project_fails_without_creating() { .on("GetProjectIdBySlugId", project_ids(&[])); Cli::for_api(&api) .run(&["milestone", "create", "--project", "Nope", "--name", "Beta"]) - .failure(); + .not_found(); } #[test] @@ -621,6 +621,6 @@ fn delete_reports_an_unknown_milestone_without_asking() { api.on("GetMilestoneName", json!({ "projectMilestone": null })); Cli::for_api(&api) .run(&["milestone", "delete", MILESTONE_ID]) - .failure() + .not_found() .stderr_has(&format!("Milestone not found: {MILESTONE_ID}")); } diff --git a/crates/linear-cli/tests/cli/misc.rs b/crates/linear-cli/tests/cli/misc.rs index 9e456e84..fefbee2f 100644 --- a/crates/linear-cli/tests/cli/misc.rs +++ b/crates/linear-cli/tests/cli/misc.rs @@ -121,7 +121,7 @@ fn config_without_credentials_fails_before_any_request() { Cli::new() .endpoint(&api) .run(&["config"]) - .failure() + .auth_failure() .stderr_has("linear auth login"); assert!(api.requests().is_empty()); } diff --git a/crates/linear-cli/tests/cli/network.rs b/crates/linear-cli/tests/cli/network.rs index 3f763185..b2e594a8 100644 --- a/crates/linear-cli/tests/cli/network.rs +++ b/crates/linear-cli/tests/cli/network.rs @@ -118,7 +118,7 @@ fn an_unrelated_ca_bundle_fails_the_handshake() { cli(&format!("https://localhost:{port}/graphql")) .env("SSL_CERT_FILE", &tls_fixture("wrong-ca.pem")) .run(&["api", QUERY]) - .failure() + .unavailable() .stderr_has("certificate"); assert!(!server.join().expect("TLS server").handshake); } @@ -155,7 +155,7 @@ fn a_redirect_from_https_to_plain_http_is_refused() { cli(&format!("https://localhost:{port}/graphql")) .env("SSL_CERT_FILE", &tls_fixture("test-ca.pem")) .run(&["api", QUERY]) - .failure() + .unavailable() .stderr_has("refusing to follow a redirect from HTTPS to plain HTTP"); assert!(server.join().expect("TLS server").handshake); assert!(api.requests().is_empty()); @@ -176,6 +176,6 @@ fn no_proxy_bypasses_the_proxy_for_listed_hosts() { Cli::for_api(&api) .env("HTTP_PROXY", dead_proxy) .run(&["api", QUERY]) - .failure(); + .unavailable(); assert_eq!(api.operations(), ["Probe"]); } diff --git a/crates/linear-cli/tests/cli/project.rs b/crates/linear-cli/tests/cli/project.rs index 4f92e936..470f00d7 100644 --- a/crates/linear-cli/tests/cli/project.rs +++ b/crates/linear-cli/tests/cli/project.rs @@ -257,7 +257,7 @@ fn view_unknown_project_fails() { .on("GetProjectIdBySlugId", no_ids()); Cli::for_api(&api) .run(&["project", "view", "nosuchproject", "--json"]) - .failure() + .not_found() .stderr_has("nosuchproject"); } @@ -547,7 +547,7 @@ fn delete_reports_an_unknown_project_without_asking() { .on("GetProjectIdBySlugId", no_ids()); Cli::for_api(&api) .run(&["project", "delete", "Nope"]) - .failure() + .not_found() .stderr_has("not found"); assert_eq!( api.operations(), @@ -770,6 +770,28 @@ fn update_without_changes_fails_before_any_request() { assert!(api.requests().is_empty()); } +#[test] +fn update_keeps_the_class_of_a_failed_initiative_link() { + let first = "00000000-0000-4000-9000-00000000000a"; + let api = MockLinear::start(); + api.on( + "GetInitiativeByIdForUpdate", + json!({ "initiatives": { "nodes": [{ "id": first, "name": "Alpha" }] } }), + ) + .on( + "GetProjectInitiativeLinksForUpdate", + json!({ "project": { + "id": ID, "name": "Mobile App", "url": "https://linear.app/acme/project/mobile", + "initiativeToProjects": page(json!([])) + } }), + ) + .on_text("AddProjectToInitiative", 503, "text/plain", "maintenance"); + Cli::for_api(&api) + .run(&["project", "update", ID, "--add-initiative", first]) + .unavailable() + .stderr_has("503"); +} + #[test] fn update_reports_partially_applied_initiative_links() { let first = "00000000-0000-4000-9000-00000000000a"; @@ -831,7 +853,7 @@ fn create_fails_before_creating_when_the_initiative_is_unknown() { "--initiative", "Nope", ]) - .failure() + .not_found() .stderr_has("Nope"); assert!(!api.operations().contains(&"CreateProject".to_owned())); } @@ -860,6 +882,28 @@ fn create_reports_a_failed_initiative_link_after_creating() { .stderr_has("--add-initiative"); } +#[test] +fn an_unavailable_initiative_link_exits_5_after_creating() { + let initiative = "00000000-0000-4000-9000-000000002509"; + let api = MockLinear::start(); + api.on("ResolveTeam", team("SRC", TEAM_ID)) + .on("CreateProject", created()) + .on_text("AddProjectToInitiative", 503, "text/plain", "maintenance"); + Cli::for_api(&api) + .run(&[ + "project", + "create", + "-n", + "X", + "-t", + "SRC", + "--initiative", + initiative, + ]) + .unavailable() + .stdout_has("✓ Created project Fixture project"); +} + #[test] fn comment_add_without_a_body_needs_a_terminal_before_any_request() { let api = MockLinear::start(); @@ -953,7 +997,7 @@ fn comment_add_on_a_terminal_resolves_the_project_before_the_editor() { .stub_bin("editor", "printf 'Hi' > \"$1\"") .env("VISUAL", "editor"); cli.run_tty(&["project", "comment", "add", "Nope"], &[]) - .failure() + .not_found() .stdout_has("Project not found: Nope"); assert!(cli.calls("editor").is_empty()); } diff --git a/crates/linear-cli/tests/cli/status_update.rs b/crates/linear-cli/tests/cli/status_update.rs index e14d7a7c..17b4d628 100644 --- a/crates/linear-cli/tests/cli/status_update.rs +++ b/crates/linear-cli/tests/cli/status_update.rs @@ -337,7 +337,7 @@ fn project_list_unknown_project_fails() { api.on("ListProjectUpdates", json!({ "project": null })); Cli::for_api(&api) .run(&["project-update", "list", PROJECT_ID]) - .failure() + .not_found() .stderr_has(PROJECT_ID); } diff --git a/crates/linear-cli/tests/cli/support/sandbox.rs b/crates/linear-cli/tests/cli/support/sandbox.rs index 95208d1e..73038cab 100644 --- a/crates/linear-cli/tests/cli/support/sandbox.rs +++ b/crates/linear-cli/tests/cli/support/sandbox.rs @@ -374,6 +374,27 @@ impl Run { self } + /// Something the command looked up in Linear does not exist: status 3. + #[track_caller] + pub fn not_found(&self) -> &Self { + assert_eq!(self.code, 3, "expected a not-found failure\n{self}"); + self + } + + /// No usable credentials, or Linear rejected them: status 4. + #[track_caller] + pub fn auth_failure(&self) -> &Self { + assert_eq!(self.code, 4, "expected an authentication failure\n{self}"); + self + } + + /// Linear could not be reached or could not serve the request: status 5. + #[track_caller] + pub fn unavailable(&self) -> &Self { + assert_eq!(self.code, 5, "expected an unavailable failure\n{self}"); + self + } + /// A usage error: from clap, or a value the command rejected after /// parsing, such as a missing required value: status 2. #[track_caller] diff --git a/crates/linear-cli/tests/cli/team.rs b/crates/linear-cli/tests/cli/team.rs index b7af90e6..8a2c0733 100644 --- a/crates/linear-cli/tests/cli/team.rs +++ b/crates/linear-cli/tests/cli/team.rs @@ -212,7 +212,7 @@ fn an_unknown_team_lists_every_team_key() { ); Cli::for_api(&api) .run(&["team", "members", "nope"]) - .failure() + .not_found() .stderr_has("Team not found: nope") .stderr_has("Valid team keys: ABC (Alpha), ZED (Zed)."); } @@ -566,6 +566,24 @@ fn delete_keeps_the_team_when_some_issues_fail_to_move() { assert!(!api.operations().contains(&"DeleteTeam".to_owned())); } +#[test] +fn delete_exits_5_when_linear_is_unavailable_for_a_move() { + let api = MockLinear::start(); + api.on("ResolveTeam", resolved("t-src", "SRC", "Source")) + .on("ResolveTeam", resolved("t-dest", "DEST", "Destination")) + .on( + "GetTeamIssuesForMove", + issue_page(&["SRC-1", "SRC-2"], Value::Null, false), + ) + .on("MoveIssueToTeam", moved(true)) + .on_text("MoveIssueToTeam", 503, "text/plain", "maintenance"); + Cli::for_api(&api) + .run(&["team", "delete", "SRC", "--yes", "--move-issues", "DEST"]) + .unavailable() + .stderr_has("1 issue(s) could not be moved, so team SRC was not deleted"); + assert!(!api.operations().contains(&"DeleteTeam".to_owned())); +} + #[test] fn delete_fails_before_moving_when_issue_pages_have_no_cursor() { let api = MockLinear::start(); diff --git a/crates/linear-cli/tests/cli/template.rs b/crates/linear-cli/tests/cli/template.rs index a3c5498e..62784343 100644 --- a/crates/linear-cli/tests/cli/template.rs +++ b/crates/linear-cli/tests/cli/template.rs @@ -146,7 +146,7 @@ fn view_unknown_name_fails() { api.on("GetTemplates", all_templates()); Cli::for_api(&api) .run(&["template", "view", "Nonexistent"]) - .failure() + .not_found() .stderr_has("Nonexistent"); } @@ -213,7 +213,7 @@ fn view_unknown_id_is_not_found() { .on("GetTemplates", json!({ "templates": [] })); Cli::for_api(&api) .run(&["template", "view", BUG_ID]) - .failure() + .not_found() .stderr_has(&format!("Template not found: {BUG_ID}")) .stderr_has("linear template list"); } diff --git a/docs/usage.md b/docs/usage.md index 4c7e524e..990a7ef8 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -485,6 +485,20 @@ every command accepts: `issue list`, `issue query`, `issue view`, and `project view` page long output; pass `--no-pager` to disable it. color is used only on a terminal; set `NO_COLOR=1` to turn it off. +### exit status + +| status | meaning | +| ------ | ------- | +| `0` | success | +| `1` | any other failure, including GraphQL errors such as an invalid query in `linear api` | +| `2` | usage error: bad flags or values, rejected before anything is sent to Linear | +| `3` | not found: an issue, team, project, or other entity the command looked up does not exist | +| `4` | authentication: no usable API key, or Linear rejected it (revoked, mistyped, or lacking access) | +| `5` | unavailable: Linear could not be reached, timed out, rate limited the request, or failed with a server error. Retrying later may work, but a create or update may still have taken effect | +| `130` | cancelled at a prompt or in the editor | + +a bulk command that fails for several reasons exits with the first of `4`, `5`, `1`, `3` among them. + ### examples common workflows: diff --git a/skills/linear-cli/SKILL.md b/skills/linear-cli/SKILL.md index 74730b72..03d925f1 100644 --- a/skills/linear-cli/SKILL.md +++ b/skills/linear-cli/SKILL.md @@ -180,6 +180,18 @@ Markdown content that is initially hidden. The square brackets around the title and the closing `+++` are required. +## Exit Status + +The exit status says why a command failed, so scripts need not parse stderr: + +- `0` success +- `1` any other failure, including GraphQL errors from `linear api` +- `2` usage error: fix the flags or values; nothing was sent +- `3` not found: the issue, team, or other entity does not exist +- `4` authentication: no usable API key, or Linear rejected it; retrying will not help +- `5` unavailable: network failure, timeout, rate limit, or server error; retry later, but a create or update may already have happened +- `130` cancelled + ## Available Commands Compact command list, generated from `linear --help`: diff --git a/skills/linear-cli/SKILL.template.md b/skills/linear-cli/SKILL.template.md index 95a10ee8..b84a4517 100644 --- a/skills/linear-cli/SKILL.template.md +++ b/skills/linear-cli/SKILL.template.md @@ -180,6 +180,18 @@ Markdown content that is initially hidden. The square brackets around the title and the closing `+++` are required. +## Exit Status + +The exit status says why a command failed, so scripts need not parse stderr: + +- `0` success +- `1` any other failure, including GraphQL errors from `linear api` +- `2` usage error: fix the flags or values; nothing was sent +- `3` not found: the issue, team, or other entity does not exist +- `4` authentication: no usable API key, or Linear rejected it; retrying will not help +- `5` unavailable: network failure, timeout, rate limit, or server error; retry later, but a create or update may already have happened +- `130` cancelled + ## Available Commands Compact command list, generated from `linear --help`: diff --git a/skills/linear-cli/references/api.md b/skills/linear-cli/references/api.md index 4d388c06..c566b530 100644 --- a/skills/linear-cli/references/api.md +++ b/skills/linear-cli/references/api.md @@ -30,7 +30,7 @@ Options: Follow the cursor of the one connection in the response and print every page --silent - Print nothing; the exit status still reports errors + Print nothing; the exit status still says whether and why it failed (see `linear --help`) -h, --help Print help (see a summary with '-h')