Giter VIP home page Giter VIP logo

Comments (14)

amatsuda avatar amatsuda commented on May 29, 2024

Awww, that is a horrible regression. πŸ’£πŸ”₯ Thanks for reporting!

I firstly added a test that reproducing the problem, then made a quick fix against the test (I guess you could see the commits displayed above this comment).

@idesaku @joker1007 I'm not totally confident with the fix though. Could you please review whether I'm doing it right?

from active_decorator.

idesaku avatar idesaku commented on May 29, 2024

after_action handlers, I think, aren't called when something an error raises. What about around_action and ensure?

from active_decorator.

amatsuda avatar amatsuda commented on May 29, 2024

Good point! Yeah, around_action would be safer, I was indeed aware of that, but I really didn't want to pollute the framework stack trace...

before_action:

app/controllers/users_controller.rb:8:in `index'
actionpack (4.2.0) lib/action_controller/metal/implicit_render.rb:4:in `send_action'
actionpack (4.2.0) lib/abstract_controller/base.rb:198:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/rendering.rb:10:in `process_action'
actionpack (4.2.0) lib/abstract_controller/callbacks.rb:20:in `block in process_action'
activesupport (4.2.0) lib/active_support/callbacks.rb:117:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:117:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:151:in `block in halting_and_conditional'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:92:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:92:in `_run_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:734:in `_run_process_action_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:81:in `run_callbacks'
actionpack (4.2.0) lib/abstract_controller/callbacks.rb:19:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/rescue.rb:29:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/instrumentation.rb:31:in `block in process_action'
activesupport (4.2.0) lib/active_support/notifications.rb:164:in `block in instrument'
activesupport (4.2.0) lib/active_support/notifications/instrumenter.rb:20:in `instrument'
activesupport (4.2.0) lib/active_support/notifications.rb:164:in `instrument'
actionpack (4.2.0) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/params_wrapper.rb:250:in `process_action'
activerecord (4.2.0) lib/active_record/railties/controller_runtime.rb:18:in `process_action'
actionpack (4.2.0) lib/abstract_controller/base.rb:137:in `process'
actionview (4.2.0) lib/action_view/rendering.rb:30:in `process'
actionpack (4.2.0) lib/action_controller/metal.rb:195:in `dispatch'
actionpack (4.2.0) lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'
actionpack (4.2.0) lib/action_controller/metal.rb:236:in `block in action'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:73:in `call'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:73:in `dispatch'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:42:in `serve'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:43:in `block in serve'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:30:in `each'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:30:in `serve'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:802:in `call'
rack (1.6.0) lib/rack/etag.rb:24:in `call'

around_action:

app/controllers/users_controller.rb:8:in `index'
actionpack (4.2.0) lib/action_controller/metal/implicit_render.rb:4:in `send_action'
actionpack (4.2.0) lib/abstract_controller/base.rb:198:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/rendering.rb:10:in `process_action'
actionpack (4.2.0) lib/abstract_controller/callbacks.rb:20:in `block in process_action'
activesupport (4.2.0) lib/active_support/callbacks.rb:117:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:117:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:151:in `block in halting_and_conditional'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:308:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:308:in `block (2 levels) in halting'
/Users/a_matsuda/src/active_decorator/lib/active_decorator/view_context.rb:26:in `call'
/Users/a_matsuda/src/active_decorator/lib/active_decorator/view_context.rb:26:in `block (2 levels) in <module:Filter>'
activesupport (4.2.0) lib/active_support/callbacks.rb:436:in `instance_exec'
activesupport (4.2.0) lib/active_support/callbacks.rb:436:in `block in make_lambda'
activesupport (4.2.0) lib/active_support/callbacks.rb:307:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:307:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:234:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:169:in `block in halting'
activesupport (4.2.0) lib/active_support/callbacks.rb:92:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:92:in `_run_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:734:in `_run_process_action_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:81:in `run_callbacks'
actionpack (4.2.0) lib/abstract_controller/callbacks.rb:19:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/rescue.rb:29:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/instrumentation.rb:31:in `block in process_action'
activesupport (4.2.0) lib/active_support/notifications.rb:164:in `block in instrument'
activesupport (4.2.0) lib/active_support/notifications/instrumenter.rb:20:in `instrument'
activesupport (4.2.0) lib/active_support/notifications.rb:164:in `instrument'
actionpack (4.2.0) lib/action_controller/metal/instrumentation.rb:30:in `process_action'
actionpack (4.2.0) lib/action_controller/metal/params_wrapper.rb:250:in `process_action'
activerecord (4.2.0) lib/active_record/railties/controller_runtime.rb:18:in `process_action'
actionpack (4.2.0) lib/abstract_controller/base.rb:137:in `process'
actionview (4.2.0) lib/action_view/rendering.rb:30:in `process'
actionpack (4.2.0) lib/action_controller/metal.rb:195:in `dispatch'
actionpack (4.2.0) lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'
actionpack (4.2.0) lib/action_controller/metal.rb:236:in `block in action'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:73:in `call'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:73:in `dispatch'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:42:in `serve'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:43:in `block in serve'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:30:in `each'
actionpack (4.2.0) lib/action_dispatch/journey/router.rb:30:in `serve'
actionpack (4.2.0) lib/action_dispatch/routing/route_set.rb:802:in `call'
rack (1.6.0) lib/rack/etag.rb:24:in `call'
rack (1.6.0) lib/rack/conditionalget.rb:25:in `call'
rack (1.6.0) lib/rack/head.rb:13:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/params_parser.rb:27:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/flash.rb:260:in `call'
rack (1.6.0) lib/rack/session/abstract/id.rb:225:in `context'
rack (1.6.0) lib/rack/session/abstract/id.rb:220:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/cookies.rb:560:in `call'
activerecord (4.2.0) lib/active_record/query_cache.rb:36:in `call'
activerecord (4.2.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:647:in `call'
activerecord (4.2.0) lib/active_record/migration.rb:378:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
activesupport (4.2.0) lib/active_support/callbacks.rb:88:in `call'
activesupport (4.2.0) lib/active_support/callbacks.rb:88:in `_run_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:734:in `_run_call_callbacks'
activesupport (4.2.0) lib/active_support/callbacks.rb:81:in `run_callbacks'
actionpack (4.2.0) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/reloader.rb:73:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/remote_ip.rb:78:in `call'
web-console (2.0.0) lib/action_dispatch/debug_exceptions.rb:18:in `middleware_call'
web-console (2.0.0) lib/action_dispatch/debug_exceptions.rb:13:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
railties (4.2.0) lib/rails/rack/logger.rb:38:in `call_app'
railties (4.2.0) lib/rails/rack/logger.rb:20:in `block in call'
activesupport (4.2.0) lib/active_support/tagged_logging.rb:68:in `block in tagged'
activesupport (4.2.0) lib/active_support/tagged_logging.rb:26:in `tagged'
activesupport (4.2.0) lib/active_support/tagged_logging.rb:68:in `tagged'
railties (4.2.0) lib/rails/rack/logger.rb:20:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/request_id.rb:21:in `call'
rack (1.6.0) lib/rack/methodoverride.rb:22:in `call'
rack (1.6.0) lib/rack/runtime.rb:18:in `call'
activesupport (4.2.0) lib/active_support/cache/strategy/local_cache_middleware.rb:28:in `call'
rack (1.6.0) lib/rack/lock.rb:17:in `call'
actionpack (4.2.0) lib/action_dispatch/middleware/static.rb:113:in `call'
rack (1.6.0) lib/rack/sendfile.rb:113:in `call'
railties (4.2.0) lib/rails/engine.rb:518:in `call'
railties (4.2.0) lib/rails/application.rb:164:in `call'
rack (1.6.0) lib/rack/lock.rb:17:in `call'

Let me consider a little bit more...

from active_decorator.

joker1007 avatar joker1007 commented on May 29, 2024

Ah..... sorry for my mistake. πŸ™‡ 😒
I was not aware of that ActionMailer view_context differents from ActionController one.

I have one worry about the fix.
If an exception is raised, and pop method is not called, Thread.current[:active_decorator_view_contexts] keeps view_context object.
And next push method adds view_context to Thread.current.
these methods increase Thread.current size gradually ?

I think that stacktrace pollution is not a big probrem.

from active_decorator.

amatsuda avatar amatsuda commented on May 29, 2024

Convinced. πŸ’€

from active_decorator.

amatsuda avatar amatsuda commented on May 29, 2024

Published 0.5.0 gem that includes this fix. #railstokyo

from active_decorator.

idesaku avatar idesaku commented on May 29, 2024

I tried out v0.5.0, but, unfortunately, came across another case due to push/pop strategy.

def show
  raise RuntimeError
end

rescue_from RuntimeError do
  ::ActiveDecorator::ViewContext.current #=> nil
  render "500" #=> Using a decorated instance only to NoMethodError
end

I have no idea how to fix it...

from active_decorator.

joker1007 avatar joker1007 commented on May 29, 2024

In rescue_from, is ViewContext already poped ?
it is better to divide controller view_context and mailer view_context, isn't it?

from active_decorator.

amatsuda avatar amatsuda commented on May 29, 2024

In order to switch controller view_context and mailer view_context, decorators should be aware of the caller inside method_missing.
I mean, when the ViewContext has both controller VC and mailer VC, how could the decorator choose one here?

ActiveDecorator::ViewContext.current.send method, *args, &block

I don't think we can do that without an evil black magic such as binding_of_caller. πŸ‘Ώ

from active_decorator.

joker1007 avatar joker1007 commented on May 29, 2024

I wrote rescue_from override way as spike.
here, joker1007@9819fff

this patch can fix the probrem.

from active_decorator.

amatsuda avatar amatsuda commented on May 29, 2024

Hm, not very bad.
How do you like it? > @idesaku

from active_decorator.

idesaku avatar idesaku commented on May 29, 2024

Would it be better to avoid Module.prepend to continue Ruby 1.9 support?

Anyway, it looks good. Actually my tests got to all green at last thanks to that patch.

from active_decorator.

joker1007 avatar joker1007 commented on May 29, 2024

OK, I use alias_method_chain, but I want not to use it actually... 😭

I created pull request #39.
If better solution is found, please comment it.

from active_decorator.

idesaku avatar idesaku commented on May 29, 2024

Thanks a lot! ❀️ v0.5.1, including the fix, works perfectly!

from active_decorator.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.