Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Messenger] messenger:consume offers sync transports to consume from and then immediately throws an exception #51556

Open
dkarlovi opened this issue Sep 4, 2023 · 5 comments
Open

[Messenger] messenger:consume offers sync transports to consume from and then immediately throws an exception #51556

dkarlovi opened this issue Sep 4, 2023 · 5 comments
Labels
Bug Messenger Status: Needs Review

Comments

@dkarlovi
Copy link
Contributor

dkarlovi commented Sep 4, 2023 •
edited
Loading

Symfony version(s) affected

6.3.3

Description

When running messenger:consume without a transport, it asks to select which transport to consume from.

$ bin/console messenger:consume

                                                                                                                        
 Which transports/receivers do you want to consume?                                                                     
                                                                                                                        

Choose which receivers you want to consume messages from in order of priority.
Hint: to consume from multiple, use a list of their names, e.g. async, default

 Select receivers to consume: [async]:
  [0] async
  [1] default

But, if I select default (which is sync), it immediately throws

In SyncTransport.php line 37:
                                                                 
  You cannot receive messages from the Messenger SyncTransport.

making this choice superfluous because the only possible selection is async.

How to reproduce

See above.

Possible Solution

Filter out sync transports and, if only one item remains, consume that immediately.

Additional Context

I guess this could be considered an UX bug.
@dkarlovi dkarlovi added the Bug label Sep 4, 2023
fabpot added a commit that referenced this issue Dec 4, 2023
@fabpot
feature #52411 [Messenger] Add --all option to messenger:consume …
dc330b0 
…(javaDeveloperKid)

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

Discussion
----------

[Messenger] Add `--all` option to `messenger:consume`

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

When implementing this feature the problem with sync transports came out so the `if` statement for this was needed. I can see someone reported this 2 months ago in #51556. I think this be can fixed properly in a dedicated PR because it requires to dig into MessengerPass I guess.

Commits
-------

ae454e0 [Messenger] Add `--all` option to `messenger:consume`
symfony-splitter pushed a commit to symfony/messenger that referenced this issue Dec 4, 2023
@fabpot
feature #52411 [Messenger] Add --all option to messenger:consume …
4a47223 
…(javaDeveloperKid)

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

Discussion
----------

[Messenger] Add `--all` option to `messenger:consume`

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

When implementing this feature the problem with sync transports came out so the `if` statement for this was needed. I can see someone reported this 2 months ago in symfony/symfony#51556. I think this be can fixed properly in a dedicated PR because it requires to dig into MessengerPass I guess.

Commits
-------

ae454e0648 [Messenger] Add `--all` option to `messenger:consume`
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@dkarlovi
Copy link
Contributor Author

dkarlovi commented Mar 6, 2024

Yup.

@carsonbot carsonbot removed the Stalled label Mar 6, 2024
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

@dkarlovi
Copy link
Contributor Author

AFAIK this wasn't fixed in other related PRs but was mentioned as a nice to have.

@carsonbot carsonbot removed the Stalled label Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Messenger Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dkarlovi @carsonbot