fix(api): handle GraphQL partial-success responses cleanly (CRY-61)

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

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)}")

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})

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

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 ->