Merge pull request #7261 from opf/fix/30092/better-api-error-logging

[30092] Fix duplicated join alias name in attachment filters

[ci skip]

app/models/queries/work_packages/filter/attachment_base_filter.rb

@@ -32,6 +32,16 @@ class Queries::WorkPackages::Filter::AttachmentBaseFilter < Queries::WorkPackage
   include Queries::WorkPackages::Filter::FilterOnTsvMixin
   include Queries::WorkPackages::Filter::TextFilterOnJoinMixin
 
+  attr_reader :join_table_suffix
+
+  def initialize(name, options = {})
+    super name, options
+
+    # Generate a uniq suffix to add to the join table
+    # because attachment filters may be used multiple times
+    @join_table_suffix = SecureRandom.hex(4)
+  end
+
   def type
     :text
   end
@@ -44,7 +54,11 @@ class Queries::WorkPackages::Filter::AttachmentBaseFilter < Queries::WorkPackage
     Queries::Operators::All.sql_for_field(values, join_table_alias, 'id')
   end
 
-  private
+  protected
+
+  def join_table_alias
+    "#{self.class.key}_#{join_table}_#{join_table_suffix}"
+  end
 
   def join_table
     Attachment.table_name

lib/api/root.rb

@@ -220,8 +220,8 @@ module API
       e.error_response status: 401, message: representer.to_json, headers: warden.headers, log: false
     end
 
-    error_response ActiveRecord::RecordNotFound, ::API::Errors::NotFound
-    error_response ActiveRecord::StaleObjectError, ::API::Errors::Conflict
+    error_response ActiveRecord::RecordNotFound, ::API::Errors::NotFound, log: false
+    error_response ActiveRecord::StaleObjectError, ::API::Errors::Conflict, log: false
 
     error_response MultiJson::ParseError, ::API::Errors::ParseError
 

lib/api/utilities/grape_helper.rb

@@ -66,6 +66,7 @@ module API
 
       def default_error_response(headers, log)
         lambda { |e|
+          original_exception = $!
           representer = ::API::V3::Errors::ErrorRepresenter.new e
           resp_headers = instance_exec &headers
           env['api.format'] = 'hal+json'
@@ -74,16 +75,20 @@ module API
             message = <<-MESSAGE
   Grape rescuing from error: #{e}
 
-  Original error: #{$!.inspect}
+  Original error: #{original_exception.inspect}
 
   Stacktrace:
             MESSAGE
 
-            $@.each do |line|
+            OpenProject.logger.clean_backtrace(original_exception).each do |line|
               message << "\n    #{line}"
             end
 
-            Rails.logger.error message
+            OpenProject.logger.error(
+              message,
+              exception: original_exception,
+              reference: :APIv3
+            )
           end
 
           error_response status: e.code, message: representer.to_json, headers: resp_headers

lib/open_project/logging/log_delegator.rb

@@ -26,9 +26,13 @@ module OpenProject
 
           registered_handlers.values.each do |handler|
             handler.call message, context
+          rescue StandardError => e
+            Rails.logger.error "Failed to delegate log to #{handler.inspect}: #{e.inspect}"
           end
 
           nil
+        rescue StandardError => e
+          Rails.logger.error "Failed to process log message #{exception.inspect}: #{e.inspect}"
         end
 
         %i(debug info warn error fatal unknown).each do |level|