Merge pull request #14882 from opf/code-maintenance/52921-configure-h…

…ttpx-timeouts

[#52921] Configure HTTPX timeouts. Merge to release 13.3.

config/constants/settings/definition.rb

@@ -716,6 +716,34 @@ class Definition
         format: :string,
         default: nil
       },
+      httpx_connect_timeout: {
+        description: '',
+        format: :float,
+        writable: false,
+        allowed: (0..),
+        default: 3
+      },
+      httpx_read_timeout: {
+        description: '',
+        format: :float,
+        writable: false,
+        allowed: (0..),
+        default: 3
+      },
+      httpx_write_timeout: {
+        description: '',
+        format: :float,
+        writable: false,
+        allowed: (0..),
+        default: 3
+      },
+      httpx_keep_alive_timeout: {
+        description: '',
+        format: :float,
+        writable: false,
+        allowed: (0..),
+        default: 20
+      },
       rate_limiting: {
         default: {},
         description: 'Configure rate limiting for various endpoint rules. See configuration documentation for details.'
@@ -833,7 +861,7 @@ class Definition
       sendmail_arguments: {
         description: 'Arguments to call sendmail with in case it is configured as outgoing email setup',
         format: :string,
-        default: "-i",
+        default: "-i"
       },
       sendmail_location: {
         description: 'Location of sendmail to call if it is configured as outgoing email setup',

lib/open_project.rb

@@ -33,6 +33,7 @@
 require 'open_project/patches'
 require 'open_project/mime_type'
 require 'open_project/custom_styles/design'
+require 'open_project/httpx_appsignal'
 require 'redmine/plugin'
 
 require 'csv'
@@ -46,8 +47,23 @@ def self.logger
   end
 
   def self.httpx
-    HTTPX
-      .plugin(:basic_auth)
-      .plugin(:webdav)
+    # It is essential to reuse HTTPX session if persistent connections are enabled.
+    # Otherwise for every request there will be a new connections.
+    # And old connections will not be closed properly which could lead to EMFILE error.
+    Thread.current[:httpx_session] ||= begin
+      session = HTTPX
+        .plugin(:persistent) # persistent plugin enables retries plugin under the hood
+        .plugin(:basic_auth)
+        .plugin(:webdav)
+        .with(
+          timeout: {
+            connect_timeout: OpenProject::Configuration.httpx_connect_timeout,
+            write_timeout: OpenProject::Configuration.httpx_write_timeout,
+            read_timeout: OpenProject::Configuration.httpx_read_timeout,
+            keep_alive_timeout: OpenProject::Configuration.httpx_keep_alive_timeout
+          }
+        )
+      OpenProject::Appsignal.enabled? ? session.plugin(HttpxAppsignal) : session
+    end
   end
 end

lib/open_project/httpx_appsignal.rb

@@ -0,0 +1,50 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module OpenProject
+  module HttpxAppsignal
+    module ConnectionMethods
+      def send(request)
+        request.on(:response) do
+          # Attention. Errors in this block are suppressed.
+          # When modifying make sure it works.
+          # Otherwise there will be no information sent to AppSignal
+          event = [
+            "request.httpx", # event_group.event_name
+            "#{request.verb.upcase} #{request.uri}", # title
+            nil, # body
+            ::Appsignal::EventFormatter::DEFAULT # formatter
+          ]
+          ::Appsignal::Transaction.current.finish_event(*event)
+        end
+        ::Appsignal::Transaction.current.start_event
+        super
+      end
+    end
+  end
+end

modules/storages/app/workers/storages/manage_nextcloud_integration_cron_job.rb

@@ -34,7 +34,7 @@ class ManageNextcloudIntegrationCronJob < Cron::CronJob
 
     queue_with_priority :low
 
-    self.cron_expression = '*/5 * * * *'
+    self.cron_expression = '1 * * * *'
 
     def self.ensure_scheduled!
       if ::Storages::ProjectStorage.active_automatically_managed.exists?

modules/storages/spec/contracts/storages/storages/nextcloud_contract_spec.rb

@@ -93,8 +93,8 @@
           expect(subject.errors.to_hash)
             .to eq({ password: ["could not be validated. Please check your storage connection and try again."] })
 
-          # will be twice when httpx persistent plugin enabled.
-          expect(credentials_request).to have_been_made.once
+          # twice due to HTTPX retry plugin being enabled.
+          expect(credentials_request).to have_been_made.twice
         end
       end
 

modules/storages/spec/contracts/storages/storages/shared_contract_examples.rb

@@ -192,8 +192,8 @@
 
             it 'retries failed request once' do
               contract.validate
-              # will be twice when httpx persistent plugin enabled.
-              expect(stub_server_capabilities).to have_been_made.once
+              # twice due to HTTPX retry plugin being enabled.
+              expect(stub_server_capabilities).to have_been_made.twice
             end
           end
 
@@ -205,8 +205,8 @@
             it 'retries failed request once' do
               contract.validate
 
-              # will be twice when httpx persistent plugin enabled.
-              expect(stub_config_check).to have_been_made.once
+              # twice due to HTTPX retry plugin being enabled.
+              expect(stub_config_check).to have_been_made.twice
             end
           end
         end

modules/storages/spec/workers/storages/manage_nextcloud_integration_cron_job_spec.rb

@@ -33,7 +33,7 @@
 
 RSpec.describe Storages::ManageNextcloudIntegrationCronJob, :webmock, type: :job do
   it 'has a schedule set' do
-    expect(described_class.cron_expression).to eq('*/5 * * * *')
+    expect(described_class.cron_expression).to eq('1 * * * *')
   end
 
   describe '.ensure_scheduled!' do