search

heartcombo / devise

Flexible authentication solution for Rails with Warden.
http://blog.plataformatec.com.br/tag/devise/
MIT License
23.89k stars 5.54k forks source link

NoMethodError in Devise::SessionsController#create after upgrade to 4.9 from 4.8.1 #5558

Closed vincemilo closed 1 year ago

vincemilo commented 1 year ago

Environment

Current behavior

When trying to login with a current user or create a new user:

undefined method empty?' for #<User id: 9, email: "xxxx", created_at: "2023-02-15 21:53:06.157323000 +0000", updated_at: "2023-02-15 21:53:06.157323000 +0000", notice_seen: nil, provider: nil, uid: nil, name: "xxxx", image: nil>

Extracted source (around line #458):

      else
        match = matched_attribute_method(method.to_s)
        match ? attribute_missing(match, *args, &block) : super
      end
    end
    ruby2_keywords(:method_missing)

activemodel (7.0.3.1) lib/active_model/attribute_methods.rb:458:in method_missing' ...

Expected behavior

Did not have this issue before in version 4.8.1 and if I rollback to earlier version it works again.

carlosantoniodasilva commented 1 year ago

Hm interesting, I don't remember hitting anything like these in my tests but I'll try to repro with a similar setup.

Any chance you can share more of the stack trace? Since that's inside Rails Active Model, it'd be nice to see where Devise is calling it for example.

Also if you have any way to repro this in a sample app, that'd be great too. Thanks.

vincemilo commented 1 year ago

Full stack trace:

activemodel (7.0.3.1) lib/active_model/attribute_methods.rb:458:in `method_missing'
rack (2.2.4) lib/rack/etag.rb:71:in `block in digest_body'
actionpack (7.0.3.1) lib/action_dispatch/http/response.rb:146:in `each'
actionpack (7.0.3.1) lib/action_dispatch/http/response.rb:146:in `each_chunk'
actionpack (7.0.3.1) lib/action_dispatch/http/response.rb:128:in `each'
actionpack (7.0.3.1) lib/action_dispatch/http/response.rb:76:in `each'
actionpack (7.0.3.1) lib/action_dispatch/http/response.rb:481:in `each'
rack (2.2.4) lib/rack/body_proxy.rb:41:in `method_missing'
rack (2.2.4) lib/rack/etag.rb:69:in `digest_body'
rack (2.2.4) lib/rack/etag.rb:33:in `call'
rack (2.2.4) lib/rack/conditional_get.rb:40:in `call'
rack (2.2.4) lib/rack/head.rb:12:in `call'
actionpack (7.0.3.1) lib/action_dispatch/http/permissions_policy.rb:38:in `call'
actionpack (7.0.3.1) lib/action_dispatch/http/content_security_policy.rb:36:in `call'
rack (2.2.4) lib/rack/session/abstract/id.rb:266:in `context'
rack (2.2.4) lib/rack/session/abstract/id.rb:260:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/cookies.rb:697:in `call'
activerecord (7.0.3.1) lib/active_record/migration.rb:603:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
activesupport (7.0.3.1) lib/active_support/callbacks.rb:99:in `run_callbacks'
actionpack (7.0.3.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/actionable_exceptions.rb:17:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/debug_exceptions.rb:28:in `call'
web-console (4.2.0) lib/web_console/middleware.rb:132:in `call_app'
web-console (4.2.0) lib/web_console/middleware.rb:28:in `block in call'
web-console (4.2.0) lib/web_console/middleware.rb:17:in `catch'
web-console (4.2.0) lib/web_console/middleware.rb:17:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/show_exceptions.rb:26:in `call'
railties (7.0.3.1) lib/rails/rack/logger.rb:40:in `call_app'
railties (7.0.3.1) lib/rails/rack/logger.rb:25:in `block in call'
activesupport (7.0.3.1) lib/active_support/tagged_logging.rb:114:in `block in tagged'
activesupport (7.0.3.1) lib/active_support/tagged_logging.rb:38:in `tagged'
activesupport (7.0.3.1) lib/active_support/tagged_logging.rb:114:in `tagged'
railties (7.0.3.1) lib/rails/rack/logger.rb:25:in `call'
sprockets-rails (3.4.2) lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/remote_ip.rb:93:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/request_id.rb:26:in `call'
rack (2.2.4) lib/rack/method_override.rb:24:in `call'
rack (2.2.4) lib/rack/runtime.rb:22:in `call'
activesupport (7.0.3.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/server_timing.rb:20:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/static.rb:23:in `call'
rack (2.2.4) lib/rack/sendfile.rb:110:in `call'
actionpack (7.0.3.1) lib/action_dispatch/middleware/host_authorization.rb:137:in `call'
railties (7.0.3.1) lib/rails/engine.rb:530:in `call'
puma (5.6.5) lib/puma/configuration.rb:252:in `call'
puma (5.6.5) lib/puma/request.rb:77:in `block in handle_request'
puma (5.6.5) lib/puma/thread_pool.rb:340:in `with_force_shutdown'
puma (5.6.5) lib/puma/request.rb:76:in `handle_request'
puma (5.6.5) lib/puma/server.rb:443:in `process_client'
puma (5.6.5) lib/puma/thread_pool.rb:147:in `block in spawn_thread'
Started GET "/users/sign_in" for 127.0.0.1 at 2023-02-15 14:42:07 -0700
Processing by Devise::SessionsController#new as HTML
  Rendering layout layouts/application.html.erb
  Rendering devise/sessions/new.html.erb within layouts/application
  Rendered devise/shared/_links.html.erb (Duration: 0.7ms | Allocations: 815)
  Rendered devise/sessions/new.html.erb within layouts/application (Duration: 2.5ms | Allocations: 2874)

I actually wasn't able to reproduce it on a smaller scale app with just omniauth and devise installed but if helpful the repo for my (very messy) school project where I'm getting the error is here: https://github.com/vincemilo/odin-facebook

carlosantoniodasilva commented 1 year ago

Thanks. Nothing jumps out at me so I might have to try and run the app locally, I'll try in the next few days, but let me know if you figure it out before.

One thing I noticed is that the app is using Devise 4.8.1, not 4.9 (which is the main branch that supports Turbo), so it's nothing related to the newer changes there (that I thought could be the culprit at first).

Have you hit the issue while trying to upgrade? Have you removed some of the workarounds like this and this while doing so? If you can push a branch against Devise main / 4.9 alpha, where the error actually occurs, that'd be cool too.

vincemilo commented 1 year ago

Right the version on the repo is currently running 4.8.1 for it to function properly but if you upgrade it to 4.9 you will get the error which is where I'm currently stuck.

Not sure what you mean by workarounds?

On Thu, Feb 16, 2023, 5:35 AM Carlos Antonio da Silva < @.***> wrote:

Thanks. Nothing jumps out at me so I might have to try and run the app locally, I'll try in the next few days, but let me know if you figure it out before.

One thing I noticed is that the app is using Devise 4.8.1, not 4.9 (which is the main branch that supports Turbo), so it's nothing related to the newer changes there (that I thought could be the culprit at first).

— Reply to this email directly, view it on GitHub https://github.com/heartcombo/devise/issues/5558#issuecomment-1433022619, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAL3UN4CAY5NVDTU5YKPJALWXYNKZANCNFSM6AAAAAAU5MXBWI . You are receiving this because you authored the thread.Message ID: @.***>

carlosantoniodasilva commented 1 year ago

Sorry, I updated the comment right after sending it with this bit:

Have you hit the issue while trying to upgrade? Have you removed some of the workarounds like this and this while doing so? If you can push a branch against Devise main / 4.9 alpha, where the error actually occurs, that'd be cool too.

Those additional pieces of code related to supporting Turbo are no longer necessary on the main Devise branch.

I'll try to take a look at the app in the next few days and see if I can learn something.

carlosantoniodasilva commented 1 year ago

@vincemilo I got your app working with Devise main and sent you a PR with the diff so you can take a look there: https://github.com/vincemilo/odin-facebook/pull/1.

Everything seems to be working with those changes as I expected, but please let me know if you run into any issues.

ricvillagrana commented 1 year ago

@carlosantoniodasilva thanks for this example, unfortunately for me, it doesn't work for sessions (specifically the login). It just re-renders, any advice on what should be looking for?

Thanks in advance.

carlosantoniodasilva commented 1 year ago

@ricvillagrana when you say it just re-renders, you mean it just show the form again without the error flash message? This could be because it's not actually re-calling the controller to render the session form again, in my previous tests it was because the failure app was customized to redirect instead, which loses the flash message context, or maybe because the error status code wasn't configured.

If you've upgraded your app and had custom failure app / parent controller set for Turbo specifically, those have to be removed. Also make sure you're using responders 3.1.0+ and have customized the error responses as shown in the changelog/wiki and in the sample upgrade above. Let me know if none of those work.

vincemilo commented 1 year ago

@vincemilo I got your app working with Devise main and sent you a PR with the diff so you can take a look there: vincemilo/odin-facebook#1.

Everything seems to be working with those changes as I expected, but please let me know if you run into any issues.

This did the trick, thanks so much! I guess app/controllers/turbo_devise_user_controller.rb was from an earlier workaround of devise with turbo?

carlosantoniodasilva commented 1 year ago

@vincemilo awesome!

I guess app/controllers/turbo_devise_user_controller.rb was from an earlier workaround of devise with turbo?

That is not an "official" way to integrate / make it work, but it was widely used as a custom implementation while Devise didn't have proper integration with Turbo built-in. Now that it has, that custom code can be safely removed.