When should Discourse upgrade to Rails 5?

mcwumbly · September 26, 2017, 1:24am

Hopefully this is all that’s needed:

https://github.com/mcwumbly/discourse-slackdoor/commit/26ec8b4bf84aca08596d85c5f2fc8c8f1efa5d12

(Wish I had CI set up for this thing).

angus · September 26, 2017, 1:56am

Nescis quid serus vesper vehat

Thanks David.

Note that, if you have written specs, all

xhr :method, :action, params

has to be re-written as

method :action, params: params, format: :json

gdpelican · September 27, 2017, 5:50am

Thanks for the heads up @david, I’ve my things.

ChrisBeach · September 27, 2017, 6:56am

Is it safe to do an update of my forum at the moment? Are people finding any instability / upgrade issues post-Rails-upgrade?

HAWK · September 27, 2017, 7:07am

Hang fire for now. @tgxworld is investigating a possible instability.

jomaxro · September 27, 2017, 12:29pm

Looks like @tgxworld has concluded his investigation. Should be safe to update, but it couldn’t hurt to wait a day or two more while we finish ironing out any additional unexpected issues.

pfaffman · September 27, 2017, 1:42pm

And if you use any non official plugins you’ll want to be especially careful.

joebuhlig · September 27, 2017, 2:19pm

I’m with @pfaffman here. I just finished pushing updates to the majority of my plugins to cover this but I’m sure there are others still working on it.

rriemann · September 27, 2017, 4:14pm

Thank you for your efforts! Does this mean that the username search related PR can finally be merged in?

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

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

https://github.com/discourse/discourse/commit/4319d8a142ea23bdfc9025d137662aa17ac37bd0

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

codinghorror · October 15, 2017, 1:29am