feat: error metadata handling (#33)

* Serialize & deserialize gRPC errors * add metadata support and tests * add unit tests for error metadata Co-authored-by: Natalie <[email protected]>
Gusto · Aug 11, 2020 · ddeec69 · ddeec69
1 parent 22b65d1
commit ddeec69
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 23 deletions.
diff --git a/lib/grpc_web/client/client_executor.rb b/lib/grpc_web/client/client_executor.rb
@@ -14,6 +14,7 @@ class << self
 
     GRPC_STATUS_HEADER = 'grpc-status'
     GRPC_MESSAGE_HEADER = 'grpc-message'
+    GRPC_HEADERS = %W[x-grpc-web #{GRPC_STATUS_HEADER} #{GRPC_MESSAGE_HEADER}].freeze
 
     def request(uri, rpc_desc, params = {})
       req_proto = rpc_desc.input.new(params)
@@ -82,32 +83,37 @@ def parse_headers(header_str)
     end
 
     def raise_on_response_errors(resp, headers, error_unpacking_frames)
+      metadata = headers.reject { |key, _| GRPC_HEADERS.include?(key) }
       status_str = headers[GRPC_STATUS_HEADER]
       status_code = status_str.to_i if status_str && status_str == status_str.to_i.to_s
 
       # see https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md
       if status_code && status_code != 0
-        raise ::GRPC::BadStatus.new_status_exception(status_code, headers[GRPC_MESSAGE_HEADER])
+        raise ::GRPC::BadStatus.new_status_exception(
+          status_code,
+          headers[GRPC_MESSAGE_HEADER],
+          metadata,
+        )
       end
 
       case resp
       when Net::HTTPBadRequest # 400
-        raise ::GRPC::Internal, resp.message
+        raise ::GRPC::Internal.new(resp.message, metadata)
       when Net::HTTPUnauthorized # 401
-        raise ::GRPC::Unauthenticated, resp.message
+        raise ::GRPC::Unauthenticated.new(resp.message, metadata)
       when Net::HTTPForbidden # 403
-        raise ::GRPC::PermissionDenied, resp.message
+        raise ::GRPC::PermissionDenied.new(resp.message, metadata)
       when Net::HTTPNotFound # 404
-        raise ::GRPC::Unimplemented, resp.message
+        raise ::GRPC::Unimplemented.new(resp.message, metadata)
       when Net::HTTPTooManyRequests, # 429
           Net::HTTPBadGateway, # 502
           Net::HTTPServiceUnavailable, # 503
           Net::HTTPGatewayTimeOut # 504
-        raise ::GRPC::Unavailable, resp.message
+        raise ::GRPC::Unavailable.new(resp.message, metadata)
       else
-        raise ::GRPC::Unknown, resp.message unless resp.is_a?(Net::HTTPSuccess) # 200
-        raise ::GRPC::Internal, resp.message if error_unpacking_frames
-        raise ::GRPC::Unknown, resp.message if status_code.nil?
+        raise ::GRPC::Unknown.new(resp.message, metadata) unless resp.is_a?(Net::HTTPSuccess) # 200
+        raise ::GRPC::Internal.new(resp.message, metadata) if error_unpacking_frames
+        raise ::GRPC::Unknown.new(resp.message, metadata) if status_code.nil?
       end
     end
   end

diff --git a/lib/grpc_web/server/message_serialization.rb b/lib/grpc_web/server/message_serialization.rb
@@ -59,7 +59,7 @@ def serialize_success_response(response)
     def serialize_error_response(response)
       ex = response.body
       if ex.is_a?(::GRPC::BadStatus)
-        header_str = generate_headers(ex.code, ex.details)
+        header_str = generate_headers(ex.code, ex.details, ex.metadata)
       else
         header_str = generate_headers(
           ::GRPC::Core::StatusCodes::UNKNOWN,
@@ -72,13 +72,19 @@ def serialize_error_response(response)
 
     # If needed, trailers can be appended to the response as a 2nd
     # base64 encoded string with independent framing.
-    def generate_headers(status, message)
-      [
+    def generate_headers(status, message, metadata = {})
+      headers = [
         "grpc-status:#{status}",
         "grpc-message:#{message}",
         'x-grpc-web:1',
-        nil, # for trailing newline
-      ].join("\r\n")
+      ]
+      metadata.each do |name, value|
+        next if %w[grpc-status grpc-message x-grpc-web].include?(name)
+
+        headers << "#{name}:#{value}"
+      end
+      headers << nil # for trailing newline
+      headers.join("\r\n")
     end
   end
 end
diff --git a/spec/integration/envoy_server_ruby_client_spec.rb b/spec/integration/envoy_server_ruby_client_spec.rb
@@ -33,13 +33,19 @@
     let(:service) do
       Class.new(TestHelloService) do
         def say_hello(_request, _metadata = nil)
-          raise ::GRPC::InvalidArgument, 'Test message'
+          raise ::GRPC::InvalidArgument.new(
+            'Test message',
+            'metadata' => 'more info',
+            'envoy' => 'more info',
+          )
         end
       end
     end
 
     it 'raises an error' do
-      expect { subject }.to raise_error(GRPC::InvalidArgument, '3:Test message')
+      expect { subject }.to raise_error(GRPC::InvalidArgument, '3:Test message') do |e|
+        expect(e.metadata).to eq('metadata' => 'more info', 'envoy' => 'more info')
+      end
     end
   end
 

diff --git a/spec/integration/ruby_server_ruby_client_spec.rb b/spec/integration/ruby_server_ruby_client_spec.rb
@@ -38,13 +38,19 @@
     let(:service) do
       Class.new(TestHelloService) do
         def say_hello(_request, _metadata = nil)
-          raise ::GRPC::InvalidArgument, 'Test message'
+          raise ::GRPC::InvalidArgument.new(
+            'Test message',
+            'metadata' => 'more info',
+            'envoy' => 'more info',
+          )
         end
       end
     end
 
     it 'raises an error' do
-      expect { subject }.to raise_error(GRPC::InvalidArgument, '3:Test message')
+      expect { subject }.to raise_error(GRPC::InvalidArgument, '3:Test message') do |e|
+        expect(e.metadata).to eq('metadata' => 'more info', 'envoy' => 'more info')
+      end
     end
   end
 

diff --git a/spec/unit/grpc_web/client/client_executor_spec.rb b/spec/unit/grpc_web/client/client_executor_spec.rb
@@ -134,13 +134,16 @@
           {
             status: 200,
             body:
-              "\x80\x00\x00\x008grpc-status:#{GRPC::Core::StatusCodes::INVALID_ARGUMENT}"\
-              "\r\ngrpc-message:Test message\r\nx-grpc-web:1\r\n",
+              "\x80\x00\x00\x00Wgrpc-status:#{GRPC::Core::StatusCodes::INVALID_ARGUMENT}"\
+              "\r\ngrpc-message:Test message\r\nx-grpc-web:1"\
+              "\r\nuser-role-id:123\r\nuser-id:456\r\n",
           }
         end
 
         it 'raises an error' do
-          expect { response }.to raise_error(GRPC::InvalidArgument)
+          expect { response }.to raise_error(GRPC::InvalidArgument) do |exception|
+            expect(exception.metadata).to eq('user-role-id' => '123', 'user-id' => '456')
+          end
         end
       end
     end

diff --git a/spec/unit/grpc_web/server/message_serialization_spec.rb b/spec/unit/grpc_web/server/message_serialization_spec.rb
@@ -115,14 +115,14 @@
       end
 
       context 'when response is a GRPC::BadStatus' do
-        let(:body) { ::GRPC::NotFound.new('Where am I?') }
+        let(:body) { ::GRPC::NotFound.new('Where am I?', 'user-role-id' => '123') }
 
         it_behaves_like 'generates a body without a payload frame'
 
         it_behaves_like(
           'generates a body with a header frame',
           expected_header_frame_body:
-            "grpc-status:5\r\ngrpc-message:Where am I?\r\nx-grpc-web:1\r\n",
+            "grpc-status:5\r\ngrpc-message:Where am I?\r\nx-grpc-web:1\r\nuser-role-id:123\r\n",
         )
       end
     end