Closed zgid123 closed 11 months ago
It seems the parameters of controller's constructor before 7.1 is different with the 7.1 one.
When using before 7.1, the one klass.instance_method(:initialize).parameters
will return [[:keyrest, :kwargs], [:block, :block]]
, but 7.1 will return [[:rest, :args], [:keyrest, :kwargs], [:block, :block]]
.
Because of this one, this @splat = parameters.any? { |type, _| type == :rest }
will return true, and then the problem above occurs.
This is new for me, so I don't think I can help to fix this one 😂
I still cannot find out how to make this case work, don't you think we should create a strategy and force it to use define_initialize_with_keywords(super_parameters)
instead?
Can you show the stacktrace of the error? In general, we cannot predict how a base class will handle its arguments, we do our best but it cannot work in all cases. I think you can trick dry-auto_inject into using the required path by something like
module Dummy
def initialize(**kw)
super(**kw)
end
end
class TestController < ActionController::API
include Dummy
include Inject[...]
end
I don't have a ready rails project on hands to test it
I create this test project, so you can have a look at the issue
https://github.com/zgid123/test-auto-inject-gem.
Full trace
ArgumentError (wrong number of arguments (given 1, expected 0)):
actionpack (7.1.2) lib/action_controller/metal.rb:185:in `initialize'
actionpack (7.1.2) lib/action_dispatch/routing/url_for.rb:110:in `initialize'
actionpack (7.1.2) lib/action_controller/metal/url_for.rb:31:in `initialize'
actionpack (7.1.2) lib/action_controller/metal/instrumentation.rb:22:in `initialize'
activerecord (7.1.2) lib/active_record/railties/controller_runtime.rb:20:in `initialize'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:68:in `block (2 levels) in define_initialize_with_splat'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:19:in `new'
dry-auto_inject (1.0.1) lib/dry/auto_inject/strategies/kwargs.rb:19:in `block (2 levels) in define_new'
actionpack (7.1.2) lib/action_controller/metal.rb:309:in `dispatch'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:32:in `serve'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:51:in `block in serve'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:131:in `block in find_routes'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:124:in `each'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:124:in `find_routes'
actionpack (7.1.2) lib/action_dispatch/journey/router.rb:32:in `serve'
actionpack (7.1.2) lib/action_dispatch/routing/route_set.rb:882:in `call'
rack (3.0.8) lib/rack/etag.rb:29:in `call'
rack (3.0.8) lib/rack/conditional_get.rb:31:in `call'
rack (3.0.8) lib/rack/head.rb:15:in `call'
activerecord (7.1.2) lib/active_record/migration.rb:654:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
activesupport (7.1.2) lib/active_support/callbacks.rb:101:in `run_callbacks'
actionpack (7.1.2) lib/action_dispatch/middleware/callbacks.rb:28:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/actionable_exceptions.rb:16:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/debug_exceptions.rb:29:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/show_exceptions.rb:31:in `call'
railties (7.1.2) lib/rails/rack/logger.rb:37:in `call_app'
railties (7.1.2) lib/rails/rack/logger.rb:24:in `block in call'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:135:in `block in tagged'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:39:in `tagged'
activesupport (7.1.2) lib/active_support/tagged_logging.rb:135:in `tagged'
activesupport (7.1.2) lib/active_support/broadcast_logger.rb:240:in `method_missing'
railties (7.1.2) lib/rails/rack/logger.rb:24:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/remote_ip.rb:92:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/request_id.rb:28:in `call'
rack (3.0.8) lib/rack/runtime.rb:24:in `call'
activesupport (7.1.2) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:59:in `block in call'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:24:in `collect_events'
actionpack (7.1.2) lib/action_dispatch/middleware/server_timing.rb:58:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/static.rb:25:in `call'
rack (3.0.8) lib/rack/sendfile.rb:114:in `call'
actionpack (7.1.2) lib/action_dispatch/middleware/host_authorization.rb:141:in `call'
railties (7.1.2) lib/rails/engine.rb:529:in `call'
puma (6.4.0) lib/puma/configuration.rb:272:in `call'
puma (6.4.0) lib/puma/request.rb:100:in `block in handle_request'
puma (6.4.0) lib/puma/thread_pool.rb:378:in `with_force_shutdown'
puma (6.4.0) lib/puma/request.rb:99:in `handle_request'
puma (6.4.0) lib/puma/server.rb:443:in `process_client'
puma (6.4.0) lib/puma/server.rb:241:in `block in run'
puma (6.4.0) lib/puma/thread_pool.rb:155:in `block in spawn_thread'
Thanks, so, I had a look and now I don't think it's a bug. Rails now uses def initialize(...)
in its countless mixins, dry-auto_inject gets confused about it. Currently, the kwargs strategy expects if there's (...)
then it's some kind of delegation so pass all dependencies to this constructor. We even have tests for it: https://github.com/dry-rb/dry-auto_inject/blob/49516a53e5f66e150387d6bf8d49acc33f579a8d/spec/integration/kwargs/inheritance/parent_class_injections_spec.rb#L70-L98 Rails, on the other hand, rails doesn't use constructor parameters at all, the base class doesn't accept any arguments. There's no reliable way to distinguish between these cases.
I suggest having a base class as a workaround:
class BaseController < ApplicationController
def initialize = super
end
class TestsController < ApplicationController
include Inject[
test_service: 'test_service'
]
def index
test_service.call(test: 'test')
p 'test'
render json: {
message: 'ok'
}
end
end
or simply this:
class ApplicationController < ActionController::API
def initialize = super
end
you are correct. When I first upgraded to ruby 3, I changed the parameters to ...
. Your fix above is working, but I don't understand why. Hope we can fix this one soon in the next version.
Thank you so much. You saved my day
Actually, I don't think we can/should fix it, it's not a bug. Instead, we could describe how to set up ApplicationController
in docs. I'm going to close this one since I don't see what else can be done right now.
Hi @flash-gordon , I recently upgraded a Rails application to Rails 7 and encountered the same issue. The fix above works.
I also troubleshoot and found the issue of Rails changing the method signature in multiple Modules included in ActionController (commit), which causes this issue.
I see the PR https://github.com/dry-rb/dry-auto_inject/pull/92 is needed going forward. Because the parameters information of ruby 3 (...)
is still not recognized.
Instead of detecting all pass_through patterns, how about detecting required/optional args/kwargs? Combining this with method arity, I hope we can easily solve the problem of passing on parameters to super_method.
If you think that's a good idea, I can make a PR and test it with my application.
@tiev since 3.1 can be declared as the minimal version now, I think it's possible to move forward with #92. I'll take a look
Describe the bug
After upgrade from rails 7.0.8 to 7.1, the
Inject
will raise errorArgumentError (wrong number of arguments (given 1, expected 0))
.After debugging, at this code block
For rails 7.0.8's controller, the
super_parameters
is notsplat
orsequential_arguments
, but rails 7.1 will be.I am still not understand what the problem is. I will try to debug and find out problem.
To Reproduce
Use the code above using rails 7.1
Expected behavior
Can inject the service
My environment