Move api error handling and timeouts to parent class

Fixes openstreetmap#4861 Since the around_action is defined before authorize_resource is called, the handler needs to pass on the CanCan::AccessDenied exception. I've added the timeouts where I think they were missing (e.g. UserPreferencesController) but I've kept the exception for changeset#upload and traces#create
gravitystorm · Oct 2, 2024 · 83425ed · 83425ed
1 parent e8da505
commit 83425ed
Show file tree

Hide file tree

Showing 18 changed files with 6 additions and 27 deletions.
diff --git a/app/controllers/api/capabilities_controller.rb b/app/controllers/api/capabilities_controller.rb
@@ -5,7 +5,6 @@ class CapabilitiesController < ApiController
     authorize_resource :class => false
 
     before_action :set_request_formats
-    around_action :api_call_handle_error, :api_call_timeout
 
     # External apps that use the api are able to query the api to find out some
     # parameters of the API. It currently returns:

diff --git a/app/controllers/api/changeset_comments_controller.rb b/app/controllers/api/changeset_comments_controller.rb
@@ -6,9 +6,8 @@ class ChangesetCommentsController < ApiController
     authorize_resource
 
     before_action :require_public_data, :only => [:create]
+
     before_action :set_request_formats
-    around_action :api_call_handle_error
-    around_action :api_call_timeout
 
     ##
     # Add a comment to a changeset

diff --git a/app/controllers/api/changesets_controller.rb b/app/controllers/api/changesets_controller.rb
@@ -11,8 +11,7 @@ class ChangesetsController < ApiController
     before_action :require_public_data, :only => [:create, :update, :upload, :close, :subscribe, :unsubscribe]
     before_action :set_request_formats, :except => [:create, :close, :upload]
 
-    around_action :api_call_handle_error
-    around_action :api_call_timeout, :except => [:upload]
+    skip_around_action :api_call_timeout, :only => [:upload]
 
     # Helper methods for checking consistency
     include ConsistencyValidations

diff --git a/app/controllers/api/map_controller.rb b/app/controllers/api/map_controller.rb
@@ -2,8 +2,6 @@ module Api
   class MapController < ApiController
     authorize_resource :class => false
 
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats
 
     # This is probably the most common call of all. It is used for getting the

diff --git a/app/controllers/api/messages_controller.rb b/app/controllers/api/messages_controller.rb
@@ -9,8 +9,6 @@ class MessagesController < ApiController
 
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats
 
     def inbox

diff --git a/app/controllers/api/nodes_controller.rb b/app/controllers/api/nodes_controller.rb
@@ -8,8 +8,6 @@ class NodesController < ApiController
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 

diff --git a/app/controllers/api/notes_controller.rb b/app/controllers/api/notes_controller.rb
@@ -7,7 +7,6 @@ class NotesController < ApiController
     authorize_resource
 
     before_action :set_locale
-    around_action :api_call_handle_error, :api_call_timeout
     before_action :set_request_formats, :except => [:feed]
 
     ##

diff --git a/app/controllers/api/old_elements_controller.rb b/app/controllers/api/old_elements_controller.rb
@@ -9,7 +9,6 @@ class OldElementsController < ApiController
 
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
     before_action :lookup_old_element, :except => [:history]
     before_action :lookup_old_element_versions, :only => [:history]
 

diff --git a/app/controllers/api/permissions_controller.rb b/app/controllers/api/permissions_controller.rb
@@ -4,7 +4,6 @@ class PermissionsController < ApiController
 
     before_action :setup_user_auth
     before_action :set_request_formats
-    around_action :api_call_handle_error, :api_call_timeout
 
     # External apps that use the api are able to query which permissions
     # they have. This currently returns a list of permissions granted to the current user:

diff --git a/app/controllers/api/relations_controller.rb b/app/controllers/api/relations_controller.rb
@@ -6,8 +6,6 @@ class RelationsController < ApiController
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 

diff --git a/app/controllers/api/tracepoints_controller.rb b/app/controllers/api/tracepoints_controller.rb
@@ -2,8 +2,6 @@ module Api
   class TracepointsController < ApiController
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
-
     # Get an XML response containing a list of tracepoints that have been uploaded
     # within the specified bounding box, and in the specified page.
     def index

diff --git a/app/controllers/api/traces_controller.rb b/app/controllers/api/traces_controller.rb
@@ -7,7 +7,7 @@ class TracesController < ApiController
     authorize_resource
 
     before_action :offline_error, :only => [:create, :destroy, :data]
-    around_action :api_call_handle_error
+    skip_around_action :api_call_timeout, :only => :create
 
     def show
       @trace = Trace.visible.find(params[:id])

diff --git a/app/controllers/api/user_blocks_controller.rb b/app/controllers/api/user_blocks_controller.rb
@@ -2,7 +2,6 @@ module Api
   class UserBlocksController < ApiController
     authorize_resource
 
-    around_action :api_call_handle_error, :api_call_timeout
     before_action :set_request_formats
 
     def show

diff --git a/app/controllers/api/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb
@@ -6,8 +6,6 @@ class UserPreferencesController < ApiController
 
     authorize_resource
 
-    around_action :api_call_handle_error
-
     before_action :set_request_formats
 
     ##

diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb
@@ -6,7 +6,6 @@ class UsersController < ApiController
 
     authorize_resource
 
-    around_action :api_call_handle_error
     load_resource :only => :show
 
     before_action :set_request_formats, :except => [:gpx_files]

diff --git a/app/controllers/api/versions_controller.rb b/app/controllers/api/versions_controller.rb
@@ -4,7 +4,6 @@ class VersionsController < ApiController
     authorize_resource :class => false
 
     before_action :set_request_formats
-    around_action :api_call_handle_error, :api_call_timeout
 
     # Show the list of available API versions. This will replace the global
     # unversioned capabilities call in due course.

diff --git a/app/controllers/api/ways_controller.rb b/app/controllers/api/ways_controller.rb
@@ -6,8 +6,6 @@ class WaysController < ApiController
     authorize_resource
 
     before_action :require_public_data, :only => [:create, :update, :delete]
-    around_action :api_call_handle_error, :api_call_timeout
-
     before_action :set_request_formats, :except => [:create, :update, :delete]
     before_action :check_rate_limit, :only => [:create, :update, :delete]
 

diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb
@@ -3,6 +3,8 @@ class ApiController < ApplicationController
 
   before_action :check_api_readable
 
+  around_action :api_call_handle_error, :api_call_timeout
+
   private
 
   ##
@@ -132,7 +134,7 @@ def api_call_handle_error
     report_error message, :bad_request
   rescue OSM::APIError => e
     report_error e.message, e.status
-  rescue AbstractController::ActionNotFound => e
+  rescue AbstractController::ActionNotFound, CanCan::AccessDenied => e
     raise
   rescue StandardError => e
     logger.info("API threw unexpected #{e.class} exception: #{e.message}")
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,8 +6,6 @@ class UserPreferencesController < ApiController @@
         authorize_resource
-        around_action :api_call_handle_error
         before_action :set_request_formats
         ##
@@ Expand Down @@