From 9e2d219f769d7119f97d0bc0591c8f17bbb5ae63 Mon Sep 17 00:00:00 2001 From: Ahmad Farhat Date: Thu, 5 Dec 2024 10:41:22 -0500 Subject: [PATCH 1/3] Add a retry mechanism for recording ready notifier (#1129) * Add a retry mechanism for recording ready notifier * rubo * Rubo again --- .../recording_ready_notifier_service.rb | 50 +++++++++++-------- .../recording_ready_notifier_service_spec.rb | 22 ++++++++ 2 files changed, 52 insertions(+), 20 deletions(-) diff --git a/app/services/recording_ready_notifier_service.rb b/app/services/recording_ready_notifier_service.rb index 8a97a2d2..75695359 100644 --- a/app/services/recording_ready_notifier_service.rb +++ b/app/services/recording_ready_notifier_service.rb @@ -21,8 +21,7 @@ def execute(recording_id) notify(callback_url, meeting_id, recording.record_id, tenant_name) if callback_url end - def encoded_payload(meeting_id, record_id, tenant_name) - secret = fetch_secrets(tenant_name: tenant_name)[0] + def encoded_payload(meeting_id, record_id, secret) payload = { meeting_id: meeting_id, record_id: record_id } JWT.encode(payload, secret) end @@ -31,27 +30,38 @@ def notify(callback_url, meeting_id, record_id, tenant_name) logger.info("Recording Ready Notify for [#{meeting_id}] starts") logger.info('Making callback for recording ready notification') - payload = encoded_payload(meeting_id, record_id, tenant_name) - uri = URI.parse(callback_url) - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = (uri.scheme == 'https') - logger.info("Sending request to #{uri.scheme}://#{uri.host}#{uri.request_uri}") - request = Net::HTTP::Post.new(uri.request_uri) - request.set_form_data(signed_parameters: payload) + secrets = fetch_secrets(tenant_name: tenant_name) + success = false - response = http.request(request) - code = response.code.to_i + secrets.each do |secret| + payload = encoded_payload(meeting_id, record_id, secret) - if code == 410 - logger.info("Notified for deleted meeting: #{meeting_id}") - elsif code == 404 - logger.info("404 error when notifying for recording: #{meeting_id}, ignoring") - elsif code < 200 || code >= 300 - logger.info("Callback HTTP request failed: #{response.code} #{response.message} (code #{code})") - else - logger.info("Recording notifier successful: #{meeting_id} (code #{code})") + uri = URI.parse(callback_url) + http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = (uri.scheme == 'https') + logger.info("Sending request to #{uri.scheme}://#{uri.host}#{uri.request_uri}") + request = Net::HTTP::Post.new(uri.request_uri) + request.set_form_data(signed_parameters: payload) + + response = http.request(request) + code = response.code.to_i + + if code == 410 + logger.info("Notified for deleted meeting: #{meeting_id}") + break + elsif code == 404 + logger.info("404 error when notifying for recording: #{meeting_id}, ignoring") + break + elsif code < 200 || code >= 300 + logger.info("Callback HTTP request failed: #{response.code} #{response.message} (code #{code})") + else + logger.info("Recording notifier successful: #{meeting_id} (code #{code})") + success = true + break + end end - true + + success rescue StandardError => e logger.info('Rescued') logger.info(e.to_s) diff --git a/spec/services/recording_ready_notifier_service_spec.rb b/spec/services/recording_ready_notifier_service_spec.rb index d0430eff..7abb106c 100644 --- a/spec/services/recording_ready_notifier_service_spec.rb +++ b/spec/services/recording_ready_notifier_service_spec.rb @@ -26,6 +26,28 @@ expect(return_val).to be true end + it 'retries with different secrets if multiple secrets are set' do + stub_request(:post, url) + .to_return( + { status: 401, body: '', headers: {} }, # First secret fails + { status: 401, body: '', headers: {} }, # Second secret fails + { status: 200, body: '', headers: {} } # Third secret succeeds + ) + + allow_any_instance_of(ApiHelper).to receive(:fetch_secrets).and_return(%w[secret1 secret2 secret3]) + + allow(JWT).to receive(:encode).and_return('eyJhbGciOiJIUzI1NiJ9.eyJtZWV0aW5nX2lkIjoibWVldGluZzE5In0') + + allow(Rails.logger).to receive(:info).and_call_original # Allow all other logger calls to pass through + + expect(Rails.logger).to receive(:info).with("Callback HTTP request failed: 401 (code 401)").twice + expect(Rails.logger).to receive(:info).with("Recording notifier successful: #{recording.meeting_id} (code #{200})").once + + return_val = described_class.execute(recording.id) + + expect(return_val).to be true + end + it 'returns false if recording ready notification fails' do stub_request(:post, url).to_timeout From ca13e629fc737f1650e94a6bda7dd0de695590f1 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Thu, 5 Dec 2024 16:41:51 +0100 Subject: [PATCH 2/3] add workaround for voice_bridge=99999 (test_voice_bridge) issue (#1128) --- app/models/meeting.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/meeting.rb b/app/models/meeting.rb index 0fd6f83f..0ff0ea33 100644 --- a/app/models/meeting.rb +++ b/app/models/meeting.rb @@ -203,6 +203,7 @@ def self.find_or_create_with_server!(id, server, moderator_pw, voice_bridge = ni def self.allocate_voice_bridge(meeting_id, voice_bridge = nil) voice_bridge_len = Rails.configuration.x.voice_bridge_len use_external_voice_bridge = Rails.configuration.x.use_external_voice_bridge + test_voice_bridge = "99999" # default value in BBB, must be avoided # In order to make consistent random pin numbers, use the provided meeting as the seed. Ruby's 'Random' PRNG takes a 128bit # integer as seed. Create one from a truncated hash of the meeting id. @@ -225,6 +226,7 @@ def self.allocate_voice_bridge(meeting_id, voice_bridge = nil) end tries += 1 logger.debug { "Trying to allocate voice bridge number #{voice_bridge}, try #{tries}" } + next if voice_bridge == test_voice_bridge # avoid special voice bridge number _created, allocated_meeting_id = redis.multi do |transaction| transaction.hsetnx('voice_bridges', voice_bridge, meeting_id) From bb671b17ed11eb690048504124281c4df03c28ec Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 9 Dec 2024 22:45:48 +0100 Subject: [PATCH 3/3] remove duplicate block in config/puma.rb (#1131) --- config/puma.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/config/puma.rb b/config/puma.rb index 28113d29..d2a334e3 100644 --- a/config/puma.rb +++ b/config/puma.rb @@ -46,14 +46,6 @@ # workers ENV.fetch("WEB_CONCURRENCY", 0) -# Specifies the number of `workers` to boot in clustered mode. -# Workers are forked web server processes. If using threads and workers together -# the concurrency of the application would be max `threads` * `workers`. -# Workers do not work on JRuby or Windows (both of which do not support -# processes). -# -workers ENV.fetch("WEB_CONCURRENCY", 0) - # Use the `preload_app!` method when specifying a `workers` number. # This directive tells Puma to first boot the application and load code # before forking the application. This takes advantage of Copy On Write