[#162105] Handle mime type errors #4819
Conversation
diebas
force-pushed
the
162105-handle-mime-type-error
branch
6 times, most recently
from
29d7a11 to
15e7da3
Compare
config/environments/development.rb
Outdated
| @@ -12,7 +12,7 @@ | |||
| config.eager_load = true | |||
|
|
|||
| # Show full error reports. | |||
| config.consider_all_requests_local = true | |||
| config.consider_all_requests_local = false | |||
There was a problem hiding this comment.
can we leave this flag on? it's useful to have this in development
| def render_acting_error | ||
| render "/acting_error", status: 403, layout: "application", formats: formats_with_html_fallback | ||
| rescue_from NUCore::PermissionDenied, CanCan::AccessDenied, with: :render_403 | ||
| rescue_from NUCore::NotPermittedWhileActingAs, with: :render_acting_error |
There was a problem hiding this comment.
We should leave this rescue_from for all environments
diebas
force-pushed
the
162105-handle-mime-type-error
branch
from
7e06dd0 to
b88c6c1
Compare
| @@ -208,4 +193,23 @@ def formats_with_html_fallback | |||
| request.formats.map(&:ref) + [:html] | |||
| end | |||
|
|
|||
| def render_403(_exception) | |||
| byebug | |||
There was a problem hiding this comment.
I don't think this should be here - do we have a spec that would catch something like this?
| rescue_from NUCore::NotPermittedWhileActingAs, with: :render_acting_error | ||
|
|
||
| # Test exception handlers | ||
| if Rails.env.test? |
There was a problem hiding this comment.
Is there a way to test without this? I usually try to avoid changing the application code to make testing work
diebas
force-pushed
the
162105-handle-mime-type-error
branch
from
b88c6c1 to
e7af96c
Compare
| end | ||
|
|
||
| def call(env) | ||
| env["HTTP_ACCEPT"] = "*/*" if env["HTTP_ACCEPT"] =~ /(\.\.|{|})/ |
There was a problem hiding this comment.
I think we should let rails decide which types are invalid and set the mime type to text/html as fallback, like in the fallback_to_html_format_if_invalid_mime_type function here: https://guides.rubyonrails.org/configuring.html#config-exceptions-app.
config/application.rb
Outdated
| config.autoload_paths << Rails.root.join("lib/middlewares") | ||
| config.eager_load_paths << Rails.root.join("lib/middlewares") |
There was a problem hiding this comment.
I'm not sure if this is needed since you're requiring the file on line 8
Release Notes
Created an errors controller to handle the errors in the app and set the routes to redirect the raised exceptions to the proper errors controller method and handle scenarios where requests with invalid format are made.
Additional Context
Exceptions weren't being handled properly while running the tests, so I kept the error handling in the application controller for the tests. Any suggestion on how to capture exceptions through the routes in the test environment is appreciated.