diff --git a/app/lib/linear_cli/api.ex b/app/lib/linear_cli/api.ex index b81754f..e1f778e 100644 --- a/app/lib/linear_cli/api.ex +++ b/app/lib/linear_cli/api.ex @@ -17,10 +17,15 @@ defmodule LinearCli.Api do where `reason` is one of: * `:missing_api_key` - `LINEAR_API_KEY` is not set - * `{:graphql_errors, errors}` - the response body had a non-empty `"errors"` list + * `{:graphql_errors, errors}` - the response body has errors but no `"data"` field * `{:unexpected_response, body}` - a 200 response with neither `"data"` nor `"errors"` * `{:http_error, status, body}` - a non-200 response * `{:transport_error, exception}` - the request itself failed (timeout, DNS, etc.) + + GraphQL partial-success responses (both `"data"` and `"errors"` present) return + `{:ok, data}` — the `"errors"` are field-level annotations on an otherwise valid + result. Callers inspect nil fields (e.g. `{:ok, %{"issue" => nil}}`) to detect + entity-not-found, which is how Linear signals a missing entity when the query ran. """ def call(document, variables \\ %{}) do with {:ok, api_key} <- fetch_api_key() do @@ -38,12 +43,20 @@ defmodule LinearCli.Api do end end - defp handle_response({:ok, %Req.Response{status: 200, body: %{"errors" => [_ | _] = errors}}}) do - {:error, {:graphql_errors, errors}} + # "data" takes precedence: a response with both "data" and "errors" is a + # GraphQL partial-success - the query ran and produced a result (possibly + # with nil fields); callers handle nil fields themselves. Only fall back to + # {:error, {:graphql_errors, ...}} when there is no "data" at all. + # Guard: only match when data is a map (the expected shape). A null top-level + # "data" means the entire operation failed; in that case the errors clause + # below provides the more informative result. + defp handle_response({:ok, %Req.Response{status: 200, body: %{"data" => data}}}) + when is_map(data) do + {:ok, data} end - defp handle_response({:ok, %Req.Response{status: 200, body: %{"data" => data}}}) do - {:ok, data} + defp handle_response({:ok, %Req.Response{status: 200, body: %{"errors" => [_ | _] = errors}}}) do + {:error, {:graphql_errors, errors}} end defp handle_response({:ok, %Req.Response{status: 200, body: body}}) do diff --git a/app/lib/linear_cli/cli.ex b/app/lib/linear_cli/cli.ex index 961cd98..cadef5a 100644 --- a/app/lib/linear_cli/cli.ex +++ b/app/lib/linear_cli/cli.ex @@ -334,6 +334,25 @@ defmodule LinearCli.CLI do halt.(22) end + # Safety net for any LinearCli.Api.call/2 site whose {:error, {:graphql_errors, + # errors}} return was not already converted to a domain-specific error tuple + # (e.g. by fetch_one/1's ENTITY_NOT_FOUND clause in issue.ex). Ash wraps the + # raw tuple in %Ash.Error.Unknown.UnknownError{error: {:graphql_errors, ...}}. + # Extracting the first error's "message" gives a human-readable error rather + # than falling through to the opaque "What the heck is this?" catch-all. + defp handle_error( + %Ash.Error.Unknown{ + errors: [%{value: [{:graphql_errors, [%{"message" => message} | _]}]} | _] + }, + debug, + halt + ) do + IO.puts(:stderr, "Linear API error: #{message}") + IO.puts(:stderr, "** API Error, Cannot Continue **") + maybe_print_backtrace(debug) + halt.(88) + end + # Ported from CLI::Caller#call's catch-all `rescue StandardError` clause. defp handle_error(error, debug, halt) do IO.puts(:stderr, "What the heck is this? #{Exception.format_banner(:error, error)}") diff --git a/app/test/linear_cli/api_test.exs b/app/test/linear_cli/api_test.exs index 1312c35..eaf68c4 100644 --- a/app/test/linear_cli/api_test.exs +++ b/app/test/linear_cli/api_test.exs @@ -12,7 +12,7 @@ defmodule LinearCli.ApiTest do assert LinearCli.Api.call("{ viewer { id } }") == {:ok, %{"viewer" => %{"id" => "123"}}} end - test "returns {:error, {:graphql_errors, errors}} when the response has errors" do + test "returns {:error, {:graphql_errors, errors}} when the response has errors but no data" do Req.Test.stub(LinearCli.Api, fn conn -> Req.Test.json(conn, %{"errors" => [%{"message" => "boom"}]}) end) @@ -21,6 +21,21 @@ defmodule LinearCli.ApiTest do {:error, {:graphql_errors, [%{"message" => "boom"}]}} end + test "returns {:ok, data} when the response has both data and errors (partial success)" do + # Linear returns HTTP 200 with both "data": {"issue": null} and "errors" + # when the requested entity doesn't exist. Returning {:ok, data} lets callers + # handle the nil field themselves (e.g. fetch_one/1's {:not_found, id} clause) + # rather than discarding the data and surfacing an opaque graphql_errors tuple. + Req.Test.stub(LinearCli.Api, fn conn -> + Req.Test.json(conn, %{ + "data" => %{"issue" => nil}, + "errors" => [%{"message" => "Entity not found: Issue", "path" => ["issue"]}] + }) + end) + + assert LinearCli.Api.call("{ issue(id: $id) { id } }") == {:ok, %{"issue" => nil}} + end + test "returns {:error, {:unexpected_response, body}} when there's neither data nor errors" do Req.Test.stub(LinearCli.Api, fn conn -> Req.Test.json(conn, %{"wat" => true}) diff --git a/app/test/linear_cli/cli_test.exs b/app/test/linear_cli/cli_test.exs index 928ea57..36e7c43 100644 --- a/app/test/linear_cli/cli_test.exs +++ b/app/test/linear_cli/cli_test.exs @@ -413,6 +413,77 @@ defmodule LinearCli.CLITest do "Roadmap" end + test "a non-entity-not-found GraphQL error prints the error message, not WTH" do + # GraphQL errors that are NOT ENTITY_NOT_FOUND (e.g. rate limits, auth errors) + # should print the API's own message rather than the opaque "What the heck" + # catch-all. The safety-net handle_error/3 clause in cli.ex handles this. + Req.Test.stub(LinearCli.Api, fn conn -> + Req.Test.json(conn, %{ + "errors" => [ + %{"message" => "Rate limit exceeded", "extensions" => %{"type" => "RATE_LIMITED"}} + ] + }) + end) + + test_pid = self() + halt = fn code -> send(test_pid, {:halted, code}) end + + output = + capture_io(:stderr, fn -> + LinearCli.CLI.main(["whoami"], halt) + end) + + assert_received {:halted, 88} + assert output =~ "Rate limit exceeded" + refute output =~ "What the heck is this?" + end + + test "issue develop with a GraphQL entity-not-found error gives a clear not-found message, not WTH" do + # Linear returns 200 with both "errors" and "data": {"issue": null} for + # missing issues. Previously this hit the "What the heck is this?" catch-all; + # now fetch_one/1 converts the ENTITY_NOT_FOUND graphql error to {:not_found, + # id} so the existing handle_error/3 clause fires with exit 66. + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + %{"query" => query} = Jason.decode!(body) + + if query =~ "viewer" do + Req.Test.json(conn, %{ + "data" => %{ + "viewer" => %{ + "id" => "u1", + "name" => "Ada", + "email" => "ada@example.com", + "teams" => %{"nodes" => []} + } + } + }) + else + Req.Test.json(conn, %{ + "errors" => [ + %{ + "message" => "Entity not found", + "extensions" => %{"type" => "ENTITY_NOT_FOUND"} + } + ], + "data" => %{"issue" => nil} + }) + end + end) + + test_pid = self() + halt = fn code -> send(test_pid, {:halted, code}) end + + output = + capture_io(:stderr, fn -> + LinearCli.CLI.main(["issue", "develop", "CRY-999"], halt) + end) + + assert_received {:halted, 66} + assert output =~ "No issue found with id" + refute output =~ "What the heck is this?" + end + test "project list --team with an unknown team key gives a clear not-found message, not WTH" do # find_team/1's get?: true action returns Ash's own built-in # %Ash.Error.Query.NotFound{} when the API responds with a nil team - a diff --git a/app/test/linear_cli/linear/issue_test.exs b/app/test/linear_cli/linear/issue_test.exs index d8128c8..075d888 100644 --- a/app/test/linear_cli/linear/issue_test.exs +++ b/app/test/linear_cli/linear/issue_test.exs @@ -132,6 +132,29 @@ defmodule LinearCli.Linear.IssueTest do assert {:error, %Ash.Error.Unknown{}} = Linear.issues(%{ids: ["nope"]}) end + test "issues/1 with a GraphQL entity-not-found response normalises to not_found" do + # Linear returns HTTP 200 with both "data": {"issue": null} and "errors" when + # an issue does not exist. Api.handle_response/1 now checks "data" before + # "errors", so it returns {:ok, %{"issue" => nil}}; fetch_one/1's nil-data + # clause converts that to {:error, {:not_found, id}}, which handle_error/3 + # in cli.ex maps to a clean "No issue found" message with exit 66. + Req.Test.stub(LinearCli.Api, fn conn -> + Req.Test.json(conn, %{ + "data" => %{"issue" => nil}, + "errors" => [ + %{ + "message" => "Entity not found: Issue", + "path" => ["issue"], + "extensions" => %{"type" => "invalid input", "userError" => true} + } + ] + }) + end) + + assert {:error, %Ash.Error.Unknown{errors: [%{value: [not_found: _id]}]}} = + Linear.issues(%{ids: ["nope"]}) + end + describe "create_issue/3+" do test "sends title/description/teamId and returns the created issue via base_fields" do Req.Test.stub(LinearCli.Api, fn conn ->