When should Discourse upgrade to Rails 5?

david · September 27, 2017, 7:39pm

@tgxworld I think the rails 5 upgrade may have introduced an issue with error handling in the application controller.

I think I’ve tracked it down to any call to rescue_discourse_actions which has include_ember: true. I use this in my category-lockdown plugin (hence why I noticed), but it is also used in core.

One example of this is the recovery from an InvalidAccess exception:

https://github.com/discourse/discourse/blob/master/app/controllers/application_controller.rb#L143-L150

If you try and access a topic you do not have access to, the server throws a 500 error. This is the stack trace I get when reproducing on my dev instance:

Stack Trace

ActionView::MissingTemplate - Missing template topics/show, application/show with {:locale=>[:en], :formats=>[:json], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby]}. Searched in:
  * "/Users/David/Web-Projects/discourse/app/views"
  * "/Users/David/Web-Projects/discourse/plugins/docker_manager/app/views"
  * "/Users/David/.rvm/gems/ruby-2.3.4/gems/discourse-qunit-rails-0.0.11/app/views"
:
  actionview (5.1.3) lib/action_view/path_set.rb:46:in `find'
  actionview (5.1.3) lib/action_view/lookup_context.rb:116:in `find'
  actionview (5.1.3) lib/action_view/renderer/abstract_renderer.rb:18:in `find_template'
  actionview (5.1.3) lib/action_view/renderer/template_renderer.rb:38:in `determine_template'
  actionview (5.1.3) lib/action_view/renderer/template_renderer.rb:8:in `render'
  actionview (5.1.3) lib/action_view/renderer/renderer.rb:42:in `render_template'
  actionview (5.1.3) lib/action_view/renderer/renderer.rb:23:in `render'
  actionview (5.1.3) lib/action_view/rendering.rb:103:in `_render_template'
  actionpack (5.1.3) lib/action_controller/metal/streaming.rb:217:in `_render_template'
  actionview (5.1.3) lib/action_view/rendering.rb:83:in `render_to_body'
  actionpack (5.1.3) lib/action_controller/metal/rendering.rb:52:in `render_to_body'
  actionpack (5.1.3) lib/action_controller/metal/renderers.rb:141:in `render_to_body'
  actionpack (5.1.3) lib/abstract_controller/rendering.rb:24:in `render'
  actionpack (5.1.3) lib/action_controller/metal/rendering.rb:36:in `render'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:44:in `block (2 levels) in render'
  activesupport (5.1.3) lib/active_support/core_ext/benchmark.rb:12:in `block in ms'
  /Users/David/.rvm/rubies/ruby-2.3.4/lib/ruby/2.3.0/benchmark.rb:308:in `realtime'
  activesupport (5.1.3) lib/active_support/core_ext/benchmark.rb:12:in `ms'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:44:in `block in render'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:87:in `cleanup_view_runtime'
  activerecord (5.1.3) lib/active_record/railties/controller_runtime.rb:29:in `cleanup_view_runtime'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:43:in `render'
  app/controllers/application_controller.rb:165:in `rescue_discourse_actions'
  app/controllers/application_controller.rb:144:in `block in <class:ApplicationController>'
  activesupport (5.1.3) lib/active_support/rescuable.rb:115:in `block in handler_for_rescue'
  activesupport (5.1.3) lib/active_support/rescuable.rb:91:in `rescue_with_handler'
  activesupport (5.1.3) lib/active_support/rescuable.rb:164:in `rescue_with_handler'
  actionpack (5.1.3) lib/action_controller/metal/rescue.rb:23:in `rescue in process_action'
  actionpack (5.1.3) lib/action_controller/metal/rescue.rb:20:in `process_action'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:32:in `block in process_action'
  activesupport (5.1.3) lib/active_support/notifications.rb:166:in `block in instrument'
  activesupport (5.1.3) lib/active_support/notifications/instrumenter.rb:21:in `instrument'
  activesupport (5.1.3) lib/active_support/notifications.rb:166:in `instrument'
  actionpack (5.1.3) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
  actionpack (5.1.3) lib/action_controller/metal/params_wrapper.rb:252:in `process_action'
  activerecord (5.1.3) lib/active_record/railties/controller_runtime.rb:22:in `process_action'
  actionpack (5.1.3) lib/abstract_controller/base.rb:124:in `process'
  actionview (5.1.3) lib/action_view/rendering.rb:30:in `process'
  rack-mini-profiler (0.10.5) lib/mini_profiler/profiling_methods.rb:102:in `block in profile_method'
  actionpack (5.1.3) lib/action_controller/metal.rb:189:in `dispatch'
  actionpack (5.1.3) lib/action_controller/metal.rb:253:in `dispatch'
  actionpack (5.1.3) lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
  actionpack (5.1.3) lib/action_dispatch/routing/route_set.rb:31:in `serve'
  actionpack (5.1.3) lib/action_dispatch/journey/router.rb:46:in `block in serve'
  actionpack (5.1.3) lib/action_dispatch/journey/router.rb:33:in `serve'
  actionpack (5.1.3) lib/action_dispatch/routing/route_set.rb:834:in `call'
  rack-protection (2.0.0) lib/rack/protection/frame_options.rb:31:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/strategy.rb:189:in `call!'
  omniauth (1.6.1) lib/omniauth/strategy.rb:167:in `call'
  omniauth (1.6.1) lib/omniauth/builder.rb:63: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.3) lib/action_dispatch/middleware/cookies.rb:613:in `call'
  activerecord (5.1.3) lib/active_record/migration.rb:556:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/callbacks.rb:26:in `block in call'
  activesupport (5.1.3) lib/active_support/callbacks.rb:97:in `run_callbacks'
  actionpack (5.1.3) lib/action_dispatch/middleware/callbacks.rb:24:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/executor.rb:12:in `call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:84:in `protected_app_call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:79:in `better_errors_call'
  better_errors (2.1.1) lib/better_errors/middleware.rb:57:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/debug_exceptions.rb:59:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
  logster (1.2.7) lib/logster/middleware/reporter.rb:31:in `call'
  railties (5.1.3) lib/rails/rack/logger.rb:36:in `call_app'
  railties (5.1.3) lib/rails/rack/logger.rb:26:in `call'
  config/initializers/100-quiet_logger.rb:16:in `call'
  config/initializers/100-silence_logger.rb:29:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/remote_ip.rb:79:in `call'
  actionpack (5.1.3) 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'
  actionpack (5.1.3) lib/action_dispatch/middleware/executor.rb:12:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/static.rb:125:in `call'
  actionpack (5.1.3) lib/action_dispatch/middleware/static.rb:125:in `call'
  rack (2.0.3) lib/rack/sendfile.rb:111:in `call'
  lib/middleware/missing_avatars.rb:21:in `call'
  lib/middleware/turbo_dev.rb:34:in `call'
  rack-mini-profiler (0.10.5) lib/mini_profiler/profiler.rb:282:in `call'
  message_bus (2.0.5) lib/message_bus/rack/middleware.rb:63:in `call'
  railties (5.1.3) lib/rails/engine.rb:522:in `call'
  railties (5.1.3) lib/rails/railtie.rb:185:in `method_missing'
  rack (2.0.3) lib/rack/urlmap.rb:68:in `block in call'
  rack (2.0.3) lib/rack/urlmap.rb:53:in `call'
  puma (3.9.1) lib/puma/configuration.rb:224:in `call'
  puma (3.9.1) lib/puma/server.rb:602:in `handle_request'
  puma (3.9.1) lib/puma/server.rb:435:in `process_client'
  puma (3.9.1) lib/puma/server.rb:299:in `block in run'
  puma (3.9.1) lib/puma/thread_pool.rb:120:in `block in spawn_thread'

For a live example, try accessing a topic you do not have access to on meta (e.g. this one in incognito mode). Browser console shows a 500 error, and I would assume you have a sea of errors in the server logs.

Edit: it also looks like the handling of topic redirects is broken. For example, a link to

https://meta.discourse.org/t/automatic-second-post-wiki/70791/7

works fine

but if I omit the title:

https://meta.discourse.org/t/70791/7

bad things happen

tgxworld · September 28, 2017, 3:07am

Thanks @david, fixed in

Hmm can you still reproduce this? It isn’t broken for me anymore.

sam · September 28, 2017, 7:17am

2 posts were split to a new topic: Guessing the keyboard height on iOS

david · September 28, 2017, 8:04am

Great

Yes I can. It only errors when I click the link in the topic - if I open in a new tab then it works fine. So I guess something going wrong with the routing logic on the ember side.

If it’s not happening to anyone else then I’ll see if I can isolate what’s causing it for me.

cpradio · September 28, 2017, 10:17am

It doesn’t seem to be just you, as your post showcasing it causes it here on Meta (at least for me). Normal click - Page not found error, Right Click open new window/tab - loads fine.

codinghorror · September 28, 2017, 5:44pm

@tgxworld can you repro the above?

tgxworld · September 29, 2017, 4:05am

Seems like the following links do not work as well. This isn’t a Rails 5 bug but rather a mismatch between how the topic routes are handled by Ember.

https://meta.discourse.org/t/70791/7
https://meta.discourse.org/t/70791

tgxworld · September 29, 2017, 6:07am

PR submitted in

https://github.com/discourse/discourse/pull/5212

@eviltrout Can you review this for me? Thank you!

eviltrout · September 29, 2017, 2:35pm

Sure, looks good to me. Merged.

saiqulhaq · September 30, 2017, 12:23pm

discourse_admin_statistics_digest plugin doesn’t use before_filter method
it’s not tied to controller

so it should be OK

Topic		Replies	Views
Discourse as Your First Rails App Blog	128	41416	January 18, 2018
Rails plugin generator Announcements	50	12513	August 20, 2021
Edits not displayed and timing out Bug fixed	12	164	November 9, 2024
Preparing for Discourse's upgrade to Ember 5 Announcements	56	6933	April 8, 2024
Problem with post revisions after updating to 2.8.6 stable or 2.9.0.beta7 Bug	41	2621	October 1, 2022

When should Discourse upgrade to Rails 5?

Related topics