From 2c982c41b3ec73addfbd1f5bf7348d32b3c1ddab Mon Sep 17 00:00:00 2001 From: Stan Hu <stanhu@gmail.com> Date: Thu, 6 Jul 2023 22:10:43 +0000 Subject: [PATCH] Update grpc to v1.55.0 and add monkey patch grpc v1.42 and up introduced a regression where a non-standard exception, `GRPC::Core::CallError`, is raised instead of DeadlineExceeded: https://github.com/grpc/grpc/issues/33283. The monkey patch applies https://github.com/grpc/grpc/pull/33565. This commit restores the changes in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121899 and introduces a monkey patch to fix the regression. Changelog: changed --- Gemfile | 2 +- Gemfile.checksum | 13 ++++---- Gemfile.lock | 6 ++-- config/initializers/grpc_patch.rb | 31 +++++++++++++++++++ .../processor/grpc_error_processor.rb | 3 +- spec/initializers/grpc_patch_spec.rb | 31 +++++++++++++++++++ 6 files changed, 75 insertions(+), 11 deletions(-) create mode 100644 config/initializers/grpc_patch.rb create mode 100644 spec/initializers/grpc_patch_spec.rb diff --git a/Gemfile b/Gemfile index 082d158071b42..aa8d9af7cdf8d 100644 --- a/Gemfile +++ b/Gemfile @@ -529,7 +529,7 @@ gem 'gitaly', '~> 16.2.0-rc2' # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.2.0' -gem 'grpc', '~> 1.42.0' +gem 'grpc', '~> 1.55.0' gem 'google-protobuf', '~> 3.23', '>= 3.23.3' diff --git a/Gemfile.checksum b/Gemfile.checksum index c7f776d4c11e3..4ac1d5b0cabae 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -267,12 +267,13 @@ {"name":"graphql","version":"1.13.12","platform":"ruby","checksum":"1d82666cf201193a8d0cb54cea38576b820418db4869b549f61a35f3a2d97ac3"}, {"name":"graphql-client","version":"0.17.0","platform":"ruby","checksum":"5aaf02ce8f2dbc8e3ba05a7eaeb3ad9336762c4424c6093f4438fbb9490eeb5d"}, {"name":"graphql-docs","version":"2.1.0","platform":"ruby","checksum":"7eb82402f8fda455104b2b60364e9ada145d79d3121a8f915790d49da38bb576"}, -{"name":"grpc","version":"1.42.0","platform":"ruby","checksum":"b3d2649e67c6a636544996843d9ec191699c54c1aca797dbfea4dff36c14584a"}, -{"name":"grpc","version":"1.42.0","platform":"x64-mingw32","checksum":"6aac1b6576134b0a83e000b1269f60d502eb24aee96c64e2658c3f24f8e32ac0"}, -{"name":"grpc","version":"1.42.0","platform":"x86-linux","checksum":"4aa50538aa929f1f3bcefb11c65ee1a1606b5aef838ea4d4e93c100b5f4263a5"}, -{"name":"grpc","version":"1.42.0","platform":"x86-mingw32","checksum":"eeb2a9381bea43fafe879b6ddaa011351a44d0894d48bdc965a07bcb67c6eb56"}, -{"name":"grpc","version":"1.42.0","platform":"x86_64-darwin","checksum":"20fa202d46d8a055628260622e98fb6439529fbac283f0552af620b909f78535"}, -{"name":"grpc","version":"1.42.0","platform":"x86_64-linux","checksum":"92e2ceb2aca335d5755163dd8030082091d5b0e63c117b1ca07051b66c53eb2e"}, +{"name":"grpc","version":"1.55.0","platform":"ruby","checksum":"529332f8e5e98f5b138afd5c4a9c7bdc9e247f4c10c84c1adbf1a114eba161ae"}, +{"name":"grpc","version":"1.55.0","platform":"x64-mingw-ucrt","checksum":"6b5c7b7358476469c5ecb46f35e1eff6983efc9395d9db8db0a2eb4207c82ffb"}, +{"name":"grpc","version":"1.55.0","platform":"x64-mingw32","checksum":"73755c256fc0fe5361a979cd609414ebdaa5862f5821fba20ea31110f1d87405"}, +{"name":"grpc","version":"1.55.0","platform":"x86-linux","checksum":"37c20569a17b1cff91155f193b0df41eb42fd0aed9051fa91ccca273a259e393"}, +{"name":"grpc","version":"1.55.0","platform":"x86-mingw32","checksum":"6b4144b5af8086b46b2e62b5fbda50fc19105a4efefafaca63e15b0384c42274"}, +{"name":"grpc","version":"1.55.0","platform":"x86_64-darwin","checksum":"d7f57eb84811d7ea2a9464ec88d9296a92801f643a4d7cf76cf4896edf12a25c"}, +{"name":"grpc","version":"1.55.0","platform":"x86_64-linux","checksum":"4ee73555759774db22ba23ff79c332cce7ae08b0ba4d4b33ab4747e83e0a8518"}, {"name":"gssapi","version":"1.3.1","platform":"ruby","checksum":"c51cf30842ee39bd93ce7fc33e20405ff8a04cda9dec6092071b61258284aee1"}, {"name":"guard","version":"2.16.2","platform":"ruby","checksum":"71ba7abaddecc8be91ab77bbaf78f767246603652ebbc7b976fda497ebdc8fbb"}, {"name":"guard-compat","version":"1.2.1","platform":"ruby","checksum":"3ad21ab0070107f92edfd82610b5cdc2fb8e368851e72362ada9703443d646fe"}, diff --git a/Gemfile.lock b/Gemfile.lock index 984bceae521ca..bb8c4bb994759 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -789,8 +789,8 @@ GEM graphql (~> 1.12) html-pipeline (~> 2.9) sass (~> 3.4) - grpc (1.42.0) - google-protobuf (~> 3.18) + grpc (1.55.0) + google-protobuf (~> 3.23) googleapis-common-protos-types (~> 1.0) gssapi (1.3.1) ffi (>= 1.0.1) @@ -1836,7 +1836,7 @@ DEPENDENCIES graphlyte (~> 1.0.0) graphql (~> 1.13.12) graphql-docs (~> 2.1.0) - grpc (~> 1.42.0) + grpc (~> 1.55.0) gssapi (~> 1.3.1) guard-rspec haml_lint (~> 0.40.0) diff --git a/config/initializers/grpc_patch.rb b/config/initializers/grpc_patch.rb new file mode 100644 index 0000000000000..a6094f85c942c --- /dev/null +++ b/config/initializers/grpc_patch.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'grpc' + +# grpc v1.42 and up introduced a regression where a non-standard +# exception, `GRPC::Core::CallError`, is raised instead of +# DeadlineExceeded: https://github.com/grpc/grpc/issues/33283. +# This patch applies https://github.com/grpc/grpc/pull/33565. +module Gitlab + module GRPCPatch + module ActiveCall + def remote_read + super + ensure + # Ensure we don't attempt to request the initial metadata again + # in case an exception occurs. + @metadata_received = true # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def receive_and_check_status + super + ensure + # Ensure we don't attempt to request the initial metadata again + # in case an exception occurs. + @metadata_received = true # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + end + end +end + +GRPC::ActiveCall.prepend Gitlab::GRPCPatch::ActiveCall diff --git a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb index ab0df39e5128c..c141398bee0d4 100644 --- a/lib/gitlab/error_tracking/processor/grpc_error_processor.rb +++ b/lib/gitlab/error_tracking/processor/grpc_error_processor.rb @@ -6,7 +6,8 @@ module Processor module GrpcErrorProcessor extend Gitlab::ErrorTracking::Processor::Concerns::ProcessesExceptions - DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:(.*)') + # Braces added by gRPC Ruby code: https://github.com/grpc/grpc/blob/0e38b075ffff72ab2ad5326e3f60ba6dcc234f46/src/ruby/lib/grpc/errors.rb#L46 + DEBUG_ERROR_STRING_REGEX = RE2('(.*) debug_error_string:\{(.*)\}') class << self def call(event) diff --git a/spec/initializers/grpc_patch_spec.rb b/spec/initializers/grpc_patch_spec.rb new file mode 100644 index 0000000000000..90d1269e4c105 --- /dev/null +++ b/spec/initializers/grpc_patch_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'GRPC monkey patch', feature_category: :shared do + let(:server) { GRPC::RpcServer.new } + let(:stub) do + Class.new(Gitaly::CommitService::Service) do + def find_all_commits(_request, _call) + sleep 1 + + nil + end + end + end + + it 'raises DeadlineExceeded on a late server streaming response' do + server_port = server.add_http2_port('0.0.0.0:0', :this_port_is_insecure) + host = "localhost:#{server_port}" + server.handle(stub) + thr = Thread.new { server.run } + + stub = Gitaly::CommitService::Stub.new(host, :this_channel_is_insecure) + request = Gitaly::FindAllCommitsRequest.new + response = stub.find_all_commits(request, deadline: Time.now + 0.1) + expect { response.to_a }.to raise_error(GRPC::DeadlineExceeded) + + server.stop + thr.join + end +end -- GitLab