Skip to content

Set minimum required version for framework-bundle to 5.3 #41545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

jordisala1991
Copy link
Contributor

@jordisala1991 jordisala1991 commented Jun 4, 2021

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

When trying to update symfony/messenger to 5.3 while still using symfony/framework-bundle 4.4 I've got this error:

The service "console.command.messenger_failed_messages_retry" has a dependency on a non-existent service "messenger.failure_transports.default".
Full Stack trace:

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException:
The service "console.command.messenger_failed_messages_retry" has a dependency on a non-existent service "messenger.failure_transports.default".

  at /usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:86
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:87)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:91)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:82)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:49)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->processValue()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/AbstractRecursivePass.php:46)
  at Symfony\Component\DependencyInjection\Compiler\AbstractRecursivePass->process()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php:40)
  at Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass->process()
     (/usr/app/vendor/symfony/dependency-injection/Compiler/Compiler.php:94)
  at Symfony\Component\DependencyInjection\Compiler\Compiler->compile()
     (/usr/app/vendor/symfony/dependency-injection/ContainerBuilder.php:762)
  at Symfony\Component\DependencyInjection\ContainerBuilder->compile()
     (/usr/app/vendor/symfony/http-kernel/Kernel.php:596)
  at Symfony\Component\HttpKernel\Kernel->initializeContainer()
     (/usr/app/vendor/symfony/http-kernel/Kernel.php:136)
  at Symfony\Component\HttpKernel\Kernel->boot()
     (/usr/app/vendor/symfony/http-kernel/Kernel.php:196)
  at Symfony\Component\HttpKernel\Kernel->handle()
     (/usr/app/public/index.php:30)                

Here you can see the issue reproduced: Runroom/archetype-symfony#1913

This is caused by this PR: #38468

This is the piece of code from framework-bundle: https://github.com/symfony/symfony/blob/5.3/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L1969-L1976

This is the usage of that service on messenger:
https://github.com/symfony/symfony/blob/5.3/src/Symfony/Component/Messenger/DependencyInjection/MessengerPass.php#L260-L264

It adds some logic to the framework bundle to declare a new alias: messenger.failure_transports.default which is used on a CompilerPass on the messenger component. This service is not present on 4.4 of the symfony/framework-bundle.

Maybe there is another option to deal with this cases, but I wasn't sure.

@jordisala1991 jordisala1991 requested a review from sroze as a code owner June 4, 2021 14:27
@carsonbot carsonbot added this to the 5.3 milestone Jun 4, 2021
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 5.4 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@jordisala1991 jordisala1991 changed the title Set minimum required version for framework-bundle to 5.3 [Messenger] Set minimum required version for framework-bundle to 5.3 Jun 4, 2021
@jordisala1991
Copy link
Contributor Author

jordisala1991 commented Jun 4, 2021

Another option could be doing something like this:

if (null !== $globalReceiverName && $container->hasDefinition('messenger.failure_transports.default')) {
    $failureTransportsMap[$commandDefinition->getArgument(0)] = new Reference('messenger.failure_transports.default');
}

To add a hasDefinition before creating the reference. It stops throwing an error on my end.

@OskarStark
Copy link
Contributor

cc @monteiro

@derrabus
Copy link
Member

derrabus commented Jun 5, 2021

At first glace, this looks like a regression to me. If FrameworkBundle was able to integrate Messenger 5.2, an upgrade to 5.3 must not break that integration. Would it be very difficult to restore compatibility?

@monteiro
Copy link
Contributor

monteiro commented Jun 5, 2021

Let me create a PR to fix this. Thanks @jordisala1991 for finding it! 😄

@derrabus
Copy link
Member

derrabus commented Jun 5, 2021

@jordisala1991 Does #41553 work for you?

@carsonbot carsonbot changed the title [Messenger] Set minimum required version for framework-bundle to 5.3 Set minimum required version for framework-bundle to 5.3 Jun 6, 2021
@jordisala1991
Copy link
Contributor Author

Works like a charm @derrabus , Thanks @monteiro

@jordisala1991 jordisala1991 deleted the hotfix/messenger-framework-requirement branch June 7, 2021 10:50
nicolas-grekas added a commit that referenced this pull request Jun 8, 2021
…tence alias being used (monteiro)

This PR was squashed before being merged into the 5.3 branch.

Discussion
----------

[Messenger] fix BC for FrameworkBundle 4.4 with a non-existence alias being used

| Q             | A
| ------------- | ---
| Branch?       | 5.3
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #41545 ... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

When trying to update symfony/messenger to 5.3 while still using symfony/framework-bundle 4.4 there is an error:

The service "console.command.messenger_failed_messages_retry" has a dependency on a non-existent service "messenger.failure_transports.default".

This is because in Symfony Messenger 5.3, on the PR #38468 to support multiple failure transports we have created an alias, that is only available on the FrameworkBundle 5.3.

This fix will use an already existence alias.

Sample project to test this behavior: https://github.com/monteiro/PR-41553-symfony

Commits
-------

aa0e166 [Messenger] fix BC for FrameworkBundle 4.4 with a non-existence alias being used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants