Metric & logging tools often want to measure / log individual exceptions from the messenger. There are currently two exception classes that hold a bunch of collected exceptions from different messages or handlers. It would be nice if there was a single interface to check and call upon when extracting these nested exceptions.

Example usecase:https://github.com/getsentry/sentry-symfony/pull/760/files#diff-da0fb4498178e4866e794b813999618022c327dea59f9277b86a7abf784aeafaR98

carsonbot added Status: Needs Review Deprecation Feature Messenger labels

Sep 14, 2023

carsonbot added this to the6.4 milestone

Sep 14, 2023

Jeroeny force-pushed themsg-nested-xcp branch 2 times, most recently fromfaf2883 to4422f6bCompare

September 14, 2023 12:25

kbond approved these changes

Sep 15, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Sep 15, 2023

nicolas-grekas reviewed

Sep 19, 2023

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

In order to reduce the cost of upgrading, I'd prefer not deprecating anything and instead addNestedExceptionsInterface::getExceptions()

This would mean removing the$recursive argument, but I think this would be legit: flattening the exception hierarchy isn't needed for the interface to be sufficient.

WDYT?

nicolas-grekas reviewed

Sep 19, 2023

View reviewed changes

src/Symfony/Component/Messenger/Exception/HandlerFailedException.phpShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/NestedExceptionsTrait.php OutdatedShow resolvedHide resolved

Jeroeny force-pushed themsg-nested-xcp branch from4422f6b to30c69c5Compare

September 19, 2023 14:27

Jeroeny requested a review fromnicolas-grekas

September 19, 2023 14:28

Copy link

Member

kbond commentedSep 19, 2023•
edited
Loading

On the topic of nested exceptions. I'm looking for feedback on#51331 - getting the context of these exceptions. In the case ofHandlerFailedException: what handler threw the exception. Is there a way currently?

nicolas-grekas reviewed

Sep 20, 2023

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Let's replace the "nested" vocabulary by "wrapped" everywhere.
Can you please also account for#51331 and preserve non-numeric keys (using array_merge(_recursive) should do it).
Last but not least, please also add test covering the $class argument.

UPGRADE-6.4.md OutdatedShow resolvedHide resolved

src/Symfony/Component/Messenger/CHANGELOG.md OutdatedShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/DelayedMessageHandlingException.php OutdatedShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/HandlerFailedException.phpShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/NestedExceptionsTrait.php OutdatedShow resolvedHide resolved

src/Symfony/Component/Messenger/Tests/Exception/HandlerFailedExceptionTest.php OutdatedShow resolvedHide resolved

Jeroeny force-pushed themsg-nested-xcp branch from74ddb36 tofe56af8Compare

September 20, 2023 15:31

Jeroeny requested a review fromnicolas-grekas

September 20, 2023 15:32

nicolas-grekas changed the title~~[Messenger] Add NestedExceptionsInterface for nested exceptions~~[Messenger] Add WrappedExceptionsInterface for nested exceptions

Sep 29, 2023

nicolas-grekas approved these changes

Sep 29, 2023

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can you please rebase? This LGTM after some minor tweaks on my side.

src/Symfony/Component/Messenger/CHANGELOG.mdShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/WrappedExceptionsInterface.php OutdatedShow resolvedHide resolved

src/Symfony/Component/Messenger/Exception/WrappedExceptionsTrait.phpShow resolvedHide resolved

Jeroeny force-pushed themsg-nested-xcp branch from1a667ec to325164aCompare

September 30, 2023 13:14

Jeroeny requested a review fromOskarStark as acode owner

September 30, 2023 13:18

carsonbot added Status: Needs Review and removed Status: Reviewed labels

Sep 30, 2023

Copy link

ContributorAuthor

Jeroeny commentedSep 30, 2023

@nicolas-grekas Done. I also changed usages ofHandlerFailedException::getNestedExceptions() togetWrappedExceptions().

fabpot approved these changes

Oct 1, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Oct 1, 2023

[Messenger] Add WrappedExceptionsInterface for nested exceptions

a3fe850

fabpot force-pushed themsg-nested-xcp branch from3645641 toa3fe850Compare

October 1, 2023 08:43

Copy link

Member

fabpot commentedOct 1, 2023

Thank you@Jeroeny.

fabpot merged commit363e217 intosymfony:6.4

Oct 1, 2023

This was referencedOct 1, 2023

[Messenger] Add WrappedExceptionsInterface for nested exceptionssymfony/symfony-docs#18952

Closed

[Messenger] add handler description as array key toHandlerFailedException::getWrappedExceptions()#51331

Merged

chalasr mentioned this pull request

Oct 2, 2023

[Messenger] Fix WrappedExceptionsTrait#51801

Merged

fabpot added a commit that referenced this pull request

Oct 2, 2023

minor#51801[Messenger] Fix WrappedExceptionsTrait (chalasr)

a257327

This PR was merged into the 6.4 branch.Discussion----------[Messenger] Fix WrappedExceptionsTrait| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | -| License       | MITFixes some minor flaws introduced in#51653Commits-------f7242ca [Messenger] Fix WrappedExceptionsTrait

This was referencedOct 21, 2023

Release v6.4.0-BETA1#52220

Merged

Release v7.0.0-BETA1#52224

Merged