From 808a149c692b7985a55612981b9628dc1117db72 Mon Sep 17 00:00:00 2001
From: QQ <me@quang.be>
Date: Wed, 25 May 2022 07:13:00 +1000
Subject: [PATCH] fix: raise `ExecutionError` with partial error response

When the GraphQL response has nested errors, `graphlient` does not raise
the `ExcutionError` which is not expected. This commit will check all
errors (including nested one) by using `errors.all` and make sure the
exception is raised.

Some considerations:
- Should we use the `Graphlient::Errors::ExecutionError` for partial
  success response or create a new type of exception to maintain
  backward compatibility? I tend to think having one type of exception
  could make things simpler for usage.
- Should `graphlient` ignore nested errors and not raise exception?
  Apparently, the whole point of this commit is about it. I'm working on
  a sensitive GraphQL client app, and any type of errors should be well
  aware to proceed to the appropriate next step.

References:
- https://github.com/github/graphql-client/pull/132#issuecomment-386111906
---
 CHANGELOG.md                         |  1 +
 lib/graphlient/client.rb             |  2 +-
 spec/graphlient/client_query_spec.rb | 20 ++++++++++++++++++++
 spec/support/queries/query.rb        | 12 ++++++++++++
 spec/support/types/invoice_type.rb   |  6 ++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 48b8afb..306b2c0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ### 0.6.0 (Next)
 * Your contribution here.
 
+* [#87](https://github.com/ashkan18/graphlient/pull/87): Raised `ExecutionError` with partial error response - [@QQism](https://github.com/QQism).
 * [#90](https://github.com/ashkan18/graphlient/pull/90): Added support for Ruby 3.1 - [@QQism](https://github.com/QQism).
 * [#90](https://github.com/ashkan18/graphlient/pull/90): Dropped support for Ruby 2.5 - [@QQism](https://github.com/QQism).
 * [#91](https://github.com/ashkan18/graphlient/pull/91): Update GHA for `danger` with right permissions - [@QQism](https://github.com/QQism).
diff --git a/lib/graphlient/client.rb b/lib/graphlient/client.rb
index b9801a9..a7e7bc4 100644
--- a/lib/graphlient/client.rb
+++ b/lib/graphlient/client.rb
@@ -67,7 +67,7 @@ def client
     end
 
     def errors_in_result?(response)
-      response.data && response.data.errors && response.data.errors.any?
+      response.data && response.data.errors && response.data.errors.all.any?
     end
   end
 end
diff --git a/spec/graphlient/client_query_spec.rb b/spec/graphlient/client_query_spec.rb
index 7436675..8fb8a63 100644
--- a/spec/graphlient/client_query_spec.rb
+++ b/spec/graphlient/client_query_spec.rb
@@ -91,6 +91,18 @@
         GRAPHQL
       end
 
+      let(:partial_success_query) do
+        <<-GRAPHQL
+          query {
+            someInvoices {
+              id
+              feeInCents
+              createdAt
+            }
+          }
+        GRAPHQL
+      end
+
       it '#execute' do
         response = client.execute(query, id: 42)
         invoice = response.data.invoice
@@ -129,6 +141,14 @@
           expect(e.response).to be_a GraphQL::Client::Response
         end
       end
+
+      it 'fails with a partial error response' do
+        expect do
+          client.execute(partial_success_query)
+        end.to raise_error Graphlient::Errors::ExecutionError do |e|
+          expect(e.response).to be_a GraphQL::Client::Response
+        end
+      end
     end
 
     context 'non-parameterized query' do
diff --git a/spec/support/queries/query.rb b/spec/support/queries/query.rb
index 76a6380..a714c8e 100644
--- a/spec/support/queries/query.rb
+++ b/spec/support/queries/query.rb
@@ -15,6 +15,10 @@ class Query < GraphQL::Schema::Object
     argument :id, Integer, required: false
   end
 
+  field :some_invoices, [InvoiceType], null: true do
+    description 'List of invoices'
+  end
+
   def invoice(id: nil)
     return nil if id.nil?
     OpenStruct.new(
@@ -32,4 +36,12 @@ def execution_error_invoice(id: nil, execution_errors:)
 
     invoice(id: id)
   end
+
+  def some_invoices
+    [
+      OpenStruct.new(id: 0, fee_in_cents: 20_000),
+      OpenStruct.new(id: 1, fee_in_cents: 20_000),
+      OpenStruct.new(id: 2, fee_in_cents: 20_000)
+    ]
+  end
 end
diff --git a/spec/support/types/invoice_type.rb b/spec/support/types/invoice_type.rb
index fef4fe8..6ed6821 100644
--- a/spec/support/types/invoice_type.rb
+++ b/spec/support/types/invoice_type.rb
@@ -4,4 +4,10 @@ class InvoiceType < GraphQL::Schema::Object
 
   field :id, ID, null: false
   field :fee_in_cents, Integer, null: true
+  field :created_at, String, null: true, extras: [:execution_errors]
+
+  def created_at(execution_errors:)
+    execution_errors.add(GraphQL::ExecutionError.new('This is a partial error'))
+    Time.now.iso8601
+  end
 end