Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rambulance doesn't rescue request with invalid format #41

Closed
willnet opened this issue Jan 7, 2018 · 19 comments
Closed

rambulance doesn't rescue request with invalid format #41

willnet opened this issue Jan 7, 2018 · 19 comments

Comments

@willnet
Copy link
Contributor

willnet commented Jan 7, 2018

a request like /users.text raise ActionController::UnknownFormat when the routes exists but the template with the format doesn't exist.

rambulance doesn't rescue the error because not_acceptable.text template also doesn't exist.

I think that rambulance should render not_acceptable.html if client request without formats rambulance handles. but I can not think of a good method to create PR. Please tell me if anyone has a good way.

sample app for reproduce the behavior

willnet/rambulance-format-issue-sample

log sample

Started GET "/users.text" for 127.0.0.1 at 2018-01-07 22:14:53 +0900
Processing by UsersController#index as TEXT
Completed 406 Not Acceptable in 55ms (ActiveRecord: 0.0ms)


  
ActionController::UnknownFormat (UsersController#index is missing a template for this request format and variant.

request.formats: ["text/plain"]
request.variant: []):
  
actionpack (5.1.4) lib/action_controller/metal/implicit_render.rb:40:in `default_render'
actionpack (5.1.4) lib/action_controller/metal/basic_implicit_render.rb:4:in `block in send_action'
actionpack (5.1.4) lib/action_controller/metal/basic_implicit_render.rb:4:in `tap'
actionpack (5.1.4) lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'
actionpack (5.1.4) lib/abstract_controller/base.rb:186:in `process_action'
actionpack (5.1.4) lib/action_controller/metal/rendering.rb:30:in `process_action'
actionpack (5.1.4) lib/abstract_controller/callbacks.rb:20:in `block in process_action'
activesupport (5.1.4) lib/active_support/callbacks.rb:131:in `run_callbacks'
actionpack (5.1.4) lib/abstract_controller/callbacks.rb:19:in `process_action'
actionpack (5.1.4) lib/action_controller/metal/rescue.rb:20:in `process_action'
actionpack (5.1.4) lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
activesupport (5.1.4) lib/active_support/notifications.rb:166:in `block in instrument'
activesupport (5.1.4) lib/active_support/notifications/instrumenter.rb:21:in `instrument'
activesupport (5.1.4) lib/active_support/notifications.rb:166:in `instrument'
actionpack (5.1.4) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
actionpack (5.1.4) lib/action_controller/metal/params_wrapper.rb:252:in `process_action'
activerecord (5.1.4) lib/active_record/railties/controller_runtime.rb:22:in `process_action'
actionpack (5.1.4) lib/abstract_controller/base.rb:124:in `process'
actionview (5.1.4) lib/action_view/rendering.rb:30:in `process'
actionpack (5.1.4) lib/action_controller/metal.rb:189:in `dispatch'
actionpack (5.1.4) lib/action_controller/metal.rb:253:in `dispatch'
actionpack (5.1.4) lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
actionpack (5.1.4) lib/action_dispatch/routing/route_set.rb:31:in `serve'
actionpack (5.1.4) lib/action_dispatch/journey/router.rb:50:in `block in serve'
actionpack (5.1.4) lib/action_dispatch/journey/router.rb:33:in `each'
actionpack (5.1.4) lib/action_dispatch/journey/router.rb:33:in `serve'
actionpack (5.1.4) lib/action_dispatch/routing/route_set.rb:834:in `call'
rack (2.0.3) lib/rack/etag.rb:25:in `call'
rack (2.0.3) lib/rack/conditional_get.rb:25:in `call'
rack (2.0.3) lib/rack/head.rb:12:in `call'
rack (2.0.3) lib/rack/session/abstract/id.rb:232:in `context'
rack (2.0.3) lib/rack/session/abstract/id.rb:226:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/cookies.rb:613:in `call'
activerecord (5.1.4) lib/active_record/migration.rb:556:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/callbacks.rb:26:in `block in call'
activesupport (5.1.4) lib/active_support/callbacks.rb:97:in `run_callbacks'
actionpack (5.1.4) lib/action_dispatch/middleware/callbacks.rb:24:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/executor.rb:12:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/debug_exceptions.rb:59:in `call'
web-console (3.5.1) lib/web_console/middleware.rb:135:in `call_app'
web-console (3.5.1) lib/web_console/middleware.rb:28:in `block in call'
web-console (3.5.1) lib/web_console/middleware.rb:18:in `catch'
web-console (3.5.1) lib/web_console/middleware.rb:18:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
railties (5.1.4) lib/rails/rack/logger.rb:36:in `call_app'
railties (5.1.4) lib/rails/rack/logger.rb:24:in `block in call'
activesupport (5.1.4) lib/active_support/tagged_logging.rb:69:in `block in tagged'
activesupport (5.1.4) lib/active_support/tagged_logging.rb:26:in `tagged'
activesupport (5.1.4) lib/active_support/tagged_logging.rb:69:in `tagged'
railties (5.1.4) lib/rails/rack/logger.rb:24:in `call'
sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/request_id.rb:25:in `call'
rack (2.0.3) lib/rack/method_override.rb:22:in `call'
rack (2.0.3) lib/rack/runtime.rb:22:in `call'
activesupport (5.1.4) lib/active_support/cache/strategy/local_cache_middleware.rb:27:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/executor.rb:12:in `call'
actionpack (5.1.4) lib/action_dispatch/middleware/static.rb:125:in `call'
rack (2.0.3) lib/rack/sendfile.rb:111:in `call'
railties (5.1.4) lib/rails/engine.rb:522:in `call'
puma (3.11.0) lib/puma/configuration.rb:225:in `call'
puma (3.11.0) lib/puma/server.rb:624:in `handle_request'
puma (3.11.0) lib/puma/server.rb:438:in `process_client'
puma (3.11.0) lib/puma/server.rb:302:in `block in run'
puma (3.11.0) lib/puma/thread_pool.rb:120:in `block in spawn_thread'
Processing by Rambulance::ExceptionsApp#not_acceptable as TEXT
Completed 500 Internal Server Error in 7ms (ActiveRecord: 0.0ms)


Error during failsafe response: Missing template errors/internal_server_error with {:locale=>[:en], :formats=>[:text], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :coffee, :jbuilder]}. Searched in:
  * "/Users/willnet/tmp/rambulance-format-issue-sample/app/views"

  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/path_set.rb:46:in `find'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/lookup_context.rb:116:in `find'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/renderer/abstract_renderer.rb:18:in `find_template'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/renderer/template_renderer.rb:38:in `determine_template'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/renderer/template_renderer.rb:8:in `render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/renderer/renderer.rb:42:in `render_template'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/renderer/renderer.rb:23:in `render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/rendering.rb:103:in `_render_template'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/streaming.rb:217:in `_render_template'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/rendering.rb:83:in `render_to_body'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/rendering.rb:52:in `render_to_body'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/renderers.rb:141:in `render_to_body'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/abstract_controller/rendering.rb:24:in `render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/rendering.rb:36:in `render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:44:in `block (2 levels) in render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/core_ext/benchmark.rb:12:in `block in ms'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/2.5.0/benchmark.rb:308:in `realtime'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/core_ext/benchmark.rb:12:in `ms'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:44:in `block in render'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:87:in `cleanup_view_runtime'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activerecord-5.1.4/lib/active_record/railties/controller_runtime.rb:29:in `cleanup_view_runtime'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:43:in `render'
  (eval):2:in `not_acceptable'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/basic_implicit_render.rb:4:in `send_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rambulance-0.5.0/lib/rambulance/exceptions_app.rb:77:in `send_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/abstract_controller/base.rb:186:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/rendering.rb:30:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/abstract_controller/callbacks.rb:20:in `block in process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/callbacks.rb:131:in `run_callbacks'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/abstract_controller/callbacks.rb:19:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/rescue.rb:20:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/notifications.rb:166:in `block in instrument'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/notifications.rb:166:in `instrument'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/instrumentation.rb:30:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal/params_wrapper.rb:252:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activerecord-5.1.4/lib/active_record/railties/controller_runtime.rb:22:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rambulance-0.5.0/lib/rambulance/exceptions_app.rb:69:in `process_action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/abstract_controller/base.rb:124:in `process'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionview-5.1.4/lib/action_view/rendering.rb:30:in `process'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rambulance-0.5.0/lib/rambulance/exceptions_app.rb:51:in `process'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal.rb:189:in `dispatch'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_controller/metal.rb:242:in `block in action'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rambulance-0.5.0/lib/rambulance/exceptions_app.rb:29:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/show_exceptions.rb:49:in `render_exception'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/show_exceptions.rb:34:in `rescue in call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/show_exceptions.rb:29:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/railties-5.1.4/lib/rails/rack/logger.rb:36:in `call_app'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/railties-5.1.4/lib/rails/rack/logger.rb:24:in `block in call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/tagged_logging.rb:69:in `block in tagged'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/tagged_logging.rb:26:in `tagged'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/tagged_logging.rb:69:in `tagged'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/railties-5.1.4/lib/rails/rack/logger.rb:24:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/sprockets-rails-3.2.1/lib/sprockets/rails/quiet_assets.rb:13:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/request_id.rb:25:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-2.0.3/lib/rack/method_override.rb:22:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-2.0.3/lib/rack/runtime.rb:22:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/activesupport-5.1.4/lib/active_support/cache/strategy/local_cache_middleware.rb:27:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/executor.rb:12:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/actionpack-5.1.4/lib/action_dispatch/middleware/static.rb:125:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/rack-2.0.3/lib/rack/sendfile.rb:111:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/railties-5.1.4/lib/rails/engine.rb:522:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/puma-3.11.0/lib/puma/configuration.rb:225:in `call'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/puma-3.11.0/lib/puma/server.rb:624:in `handle_request'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/puma-3.11.0/lib/puma/server.rb:438:in `process_client'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/puma-3.11.0/lib/puma/server.rb:302:in `block in run'
  /Users/willnet/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/puma-3.11.0/lib/puma/thread_pool.rb:120:in `block in spawn_thread'
@yuki24
Copy link
Owner

yuki24 commented Jan 7, 2018

Thanks for reporting. The exceptions app already responds to all the methods that correspond to all the error HTTP status codes. What this means is that if there's a template for the status code, it will just work out of the box. Try creating a file called not_acceptable.text.erb in app/views/errors:

$ echo "Custom error page for 406 Not Acceptable" > app/views/errors/not_acceptable.text.erb

You can see all the predefined actions by looking at the Rambulance::ERROR_HTTP_STATUSES constant:

irb(main):001:0> y Rambulance::ERROR_HTTP_STATUSES
---
400: :bad_request
401: :unauthorized
402: :payment_required
403: :forbidden
404: :not_found
405: :method_not_allowed
406: :not_acceptable
407: :proxy_authentication_required
408: :request_timeout
409: :conflict
410: :gone
411: :length_required
412: :precondition_failed
413: :payload_too_large
414: :uri_too_long
415: :unsupported_media_type
416: :range_not_satisfiable
417: :expectation_failed
421: :misdirected_request
422: :unprocessable_entity
423: :locked
424: :failed_dependency
426: :upgrade_required
428: :precondition_required
429: :too_many_requests
431: :request_header_fields_too_large
451: :unavailable_for_legal_reasons
500: :internal_server_error
501: :not_implemented
502: :bad_gateway
503: :service_unavailable
504: :gateway_timeout
505: :http_version_not_supported
506: :variant_also_negotiates
507: :insufficient_storage
508: :loop_detected
510: :not_extended
511: :network_authentication_required

What's actually unexpected, however, is that the exceptions app allowed an internal exception to bubble up when it shouldn't, causing the page to show an almost blank error page. There are a few improvements that could be made:

  • The backtrace should have a proper file path and lineno for the action for easier debugging
  • The exceptions app shouldn't leak internal exceptions
  • There should be a documentation on how to add a new template for each status code. Or, if the exceptions app receives an error but the template is missing, it should inform the developer of creating a new template with an example command to do so.

@willnet
Copy link
Contributor Author

willnet commented Jan 9, 2018

Thanks for the reply.

Try creating a file called not_acceptable.text.erb in app/views/errors

I'm sorry I didn't make it clear enough. What I think as a problem is that a user can request with any format, like/users.xml, /users.pdf, /users.csv and so on, but we can't create all error templates in advance.

If a user request with unknown format, web service with rambulance should respond 4xx(maybe 406), but respond 500 now.

What's actually unexpected, however, is that the exceptions app allowed an internal exception to bubble up when it shouldn't, causing the page to show an almost blank error page

I totally agree.

The exceptions app shouldn't leak internal exceptions

I think this is the best out all of them.

I think it's better that we can set default format to rambulance. If a user request with a format and a template with the format doesn't exist, rambulance should search template with default format and display it.

@yuki24 yuki24 closed this as completed in b7c60d5 Jan 9, 2018
@yuki24
Copy link
Owner

yuki24 commented Jan 9, 2018

re-opening as this isn't quite fixed yet: https://travis-ci.org/yuki24/rambulance/builds/326901626

@yuki24 yuki24 reopened this Jan 9, 2018
@joker-777
Copy link

Hi, is this now fixed? We are experiencing the same issue.

@yuki24
Copy link
Owner

yuki24 commented Jan 31, 2018

The master branch should work fine with Rails 5.0 and 5.1. Would you mind using it for now?

@joker-777
Copy link

Hi, we use Rails 5.1. I'm using now master and it shows now a different page but doesn't use the correct layout. Any ideas why? Unfortunately, the error isn't still caught. We can still see it in our logs.

@yuki24
Copy link
Owner

yuki24 commented Feb 1, 2018

@joker-777 Thanks for getting back, but I'm not sure I understand your issue. Could you post example code or a test that fails?

@joker-777
Copy link

OK, when I use the url https://staging.kenhub.com/en/dashboa which doesn't exist I get

screenshot_20180201_131339

But when I use https://staging.kenhub.com/en/dashboard.json where the file format doesn't exist I get

screenshot_20180201_131416

Without the layout.

Here is the config file we are using.
https://raw.githubusercontent.com/kenhub/kenhub/master/config/initializers/rambulance.rb?token=AAXH69e_LkZ4YHCezFaqjLtVDDe2-L2bks5afJRywA%3D%3D

In addition, we still see this ActionController::UnknownFormat error in our logs. It seems it isn't caught properly.

@yuki24
Copy link
Owner

yuki24 commented Feb 1, 2018

@joker-777 do you have app/views/errors/not_acceptable.json.erb in your project?

@joker-777
Copy link

@yuki24 No, we don't.

@yuki24
Copy link
Owner

yuki24 commented Feb 1, 2018

Assuming you are using master, the expected behaviour here is your app returns 406 and falls back to app/views/errors/not_acceptable.html.erb instead of returning 500. Is that what you observed?

Also, what would you expect to observe? As @willnet mentioned above, there's no good way to cover every single mime type as there's a million of them and falling back to a text/html response is all the framework could do.

@joker-777
Copy link

Yes, it returns the correct status number and also uses the correct template but without any layout although it is defined correctly. It also logs an error too.

@yuki24
Copy link
Owner

yuki24 commented Feb 2, 2018

@joker-777 It'd be much appreciated if you could provide me with a test case that fails.

@joker-777
Copy link

@y-yagi Sorry for my late raply. I finally managed to create this test app. https://github.com/joker-777/rambulance_test_app

After starting the server call http://localhost:3001/test.json. You wil see FORMAT ERROR but you actually should see

LAYOUT
FORMAT ERROR

Because app/views/layouts/error.html.erb displays LAYOUT. But it isn't used. You can also see in the server output that it still logs an error.

@yuki24
Copy link
Owner

yuki24 commented Feb 9, 2018

Thanks @joker-777 for the example. I haven't been able to work on my open source stuff lately but will be able to look into this as well as other issues in early March. Thanks again for bearing with me!

@joker-777
Copy link

@yuki24 Never mind, I think I found out why it didn't use the correct layout. You set the layout to false when the request is a json request. Also please don't mind my comment about logging the error. It seems like this is normal.

@joker-777
Copy link

joker-777 commented Feb 14, 2018

@yuki24 There is actually another problem. It is possible that a format exists with the ref */*. When you do request.formats = request.formats.map(&:ref) << :html this specific format */* translates to nil. Which then raises later in the call chain an error because there request.formats.map(&:ref) is called again which brakes when one of the format objects is nil.
I think it is better to write request.formats << Mime::Type.register("text/html", :html).
I will try to update my example app tomorrow.

@joker-777
Copy link

I created a PR for it. #42
This format */* appears when using an ajax json request with jQuery.

joker-777 added a commit to joker-777/rambulance that referenced this issue Feb 22, 2018
@yuki24 yuki24 closed this as completed in 1b824e6 Mar 8, 2018
@yuki24
Copy link
Owner

yuki24 commented Mar 8, 2018

released as 0.6.0. Thanks for bearing with me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants