From a05d67e833a658bf3ddd9b68f06fb27c60cd3952 Mon Sep 17 00:00:00 2001
From: Nelson <nelsonwittwer@users.noreply.github.com>
Date: Wed, 12 Apr 2023 15:17:56 -0400
Subject: [PATCH] JSON parseable error messages (#1140)

* JSON parseable error messages

* changelog
---
 CHANGELOG.md                           |  2 ++
 lib/shopify_api/clients/http_client.rb | 22 +++++++++++++---------
 test/clients/http_client_test.rb       | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7c8b7fc7..9983a41ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ Note: For changes to the API, see https://shopify.dev/changelog?filter=api
 
 ## Unreleased
 
+- [#1140](https://github.com/Shopify/shopify-api-ruby/pull/1140) ⚠️ [Breaking] Reformat Http error messages to be JSON parsable.
+
 ## 12.5.0
 
 - [#1113](https://github.com/Shopify/shopify-api-ruby/pull/1113) Handle JSON::ParserError when http response is HTML and raise ShopifyAPI::Errors::HttpResponseError
diff --git a/lib/shopify_api/clients/http_client.rb b/lib/shopify_api/clients/http_client.rb
index 4f18c94a0..9084bf70e 100644
--- a/lib/shopify_api/clients/http_client.rb
+++ b/lib/shopify_api/clients/http_client.rb
@@ -65,15 +65,7 @@ def request(request)
 
           break if response.ok?
 
-          error_messages = []
-          error_messages << response.body["errors"] if response.body["errors"]
-
-          if response.headers["x-request-id"]
-            id = T.must(response.headers["x-request-id"])[0]
-            error_messages << "If you report this error, please include this id: #{id}."
-          end
-
-          error_message = error_messages.join("\n")
+          error_message = serialized_error(response)
 
           unless [429, 500].include?(response.code)
             raise ShopifyAPI::Errors::HttpResponseError.new(response: response), error_message
@@ -102,6 +94,18 @@ def request(request)
       def request_url(request)
         "#{@base_uri_and_path}/#{request.path}"
       end
+
+      sig { params(response: HttpResponse).returns(String) }
+      def serialized_error(response)
+        body = {}
+        body["errors"] = response.body["errors"] if response.body["errors"]
+
+        if response.headers["x-request-id"]
+          id = T.must(response.headers["x-request-id"])[0]
+          body["error_reference"] = "If you report this error, please include this id: #{id}."
+        end
+        body.to_json
+      end
     end
   end
 end
diff --git a/test/clients/http_client_test.rb b/test/clients/http_client_test.rb
index 7e1f2c8b8..b692d5450 100644
--- a/test/clients/http_client_test.rb
+++ b/test/clients/http_client_test.rb
@@ -123,6 +123,25 @@ def test_request_with_invalid_request
         assert_raises(ShopifyAPI::Errors::InvalidHttpRequestError) { @client.request(@request) }
       end
 
+      def test_error_message_structure
+        error_response_body = {
+          "errors": { "line_items" => ["must have at least one line item"] },
+        }.to_json
+        response_headers = {
+          "x-request-id": 1234,
+        }
+        stub_request(@request.http_method, "https://#{@shop}#{@base_path}/#{@request.path}")
+          .with(body: @request.body.to_json, query: @request.query, headers: @expected_headers)
+          .to_return(body: error_response_body, status: 422, headers: response_headers)
+
+        response = assert_raises(ShopifyAPI::Errors::HttpResponseError) do
+          @client.request(@request)
+        end
+        parsed_error = JSON.parse(response.message)
+        assert(parsed_error["errors"].present?)
+        assert(parsed_error["error_reference"].present?)
+      end
+
       def test_non_retriable_error_code
         stub_request(@request.http_method, "https://#{@shop}#{@base_path}/#{@request.path}")
           .with(body: @request.body.to_json, query: @request.query, headers: @expected_headers)