Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:6.4fromkbond:exception-handler-name
Oct 25, 2023

Conversation

@kbond
Copy link
Member

QA
Branch?6.4
Bug fix?no
New feature?yes
Deprecations?no
Ticketsn/a
LicenseMIT
Doc PRn/a

Currently, when looking atHandlerFailedException to see what exceptions were thrown for a message, you can't see what handler caused the exception.

Copy link
Member

@nicolas-grekasnicolas-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 this be tested somehow?

@kbond
Copy link
MemberAuthor

can this be tested somehow?

Test added.

@fabpot
Copy link
Member

Should be rebased (see#51653)

@kbondkbondforce-pushed theexception-handler-name branch from61a5a06 to84abc0dCompareOctober 2, 2023 12:40
@kbond
Copy link
MemberAuthor

Rebased.

@kbondkbondforce-pushed theexception-handler-name branch 2 times, most recently from3e34ef0 to7c7fb54CompareOctober 2, 2023 13:09
@kbond
Copy link
MemberAuthor

Psalm errors fixed.

@kbondkbondforce-pushed theexception-handler-name branch from7c7fb54 toc2650c1CompareOctober 2, 2023 13:46
@chalasr
Copy link
Member

What about reverting the changes made to the now deprecatedgetNestedExceptions() to update the newly introduced API only?

@kbond
Copy link
MemberAuthor

What about reverting the changes made to the now deprecated getNestedExceptions() to update the newly introduced API only?

You mean throw anarray_values() intogetNestedExceptions() before returning?


Another thing, it would be nice ifDelayedMessageHandlingException (which implements this new interface) could provide handlers names as array keys as well but.. I don't think this is possible?

@chalasr
Copy link
Member

chalasr commentedOct 2, 2023
edited
Loading

You mean throw an array_values() into getNestedExceptions() before returning?

It's probably not a big deal here but yes, we usually avoid changing deprecated code paths.

Another thing, it would be nice if DelayedMessageHandlingException (which implements this new interface) could provide handlers names as array keys as well but.. I don't think this is possible?

I think it would be possible to useexception->getTrace()[0]['class'] as keys inDispatchAfterCurrentBusMiddleware.
I may be missing some edge cases though e.g. if the exception wasn't thrown by the handler or if the handler callable has no class (if that's even possible?)

@kbond
Copy link
MemberAuthor

It's probably not a big deal here but yes, we usually avoid changing deprecated code paths.

@chalasr, I've made this change.

I think it would be possible to use exception->getTrace()[0]['class'] as keys in DispatchAfterCurrentBusMiddleware.
I may be missing some edge cases though e.g. if the exception wasn't thrown by the handler or if the handler callable has no class (if that's even possible?)

I'm going to leave this off of this PR because of the possible edge cases you describe. This can be revisited later if desired.

@kbondkbondforce-pushed theexception-handler-name branch from5e7bb97 to06a697cCompareOctober 23, 2023 16:34
@kbondkbondforce-pushed theexception-handler-name branch from06a697c todcc788dCompareOctober 23, 2023 16:57
@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 23, 2023
@kbondkbondforce-pushed theexception-handler-name branch fromdcc788d to9e67f29CompareOctober 24, 2023 14:37
@nicolas-grekasnicolas-grekas changed the title[Messenger] add handler description as array key toHandlerFailedException::getNestedExceptions()[Messenger] add handler description as array key toHandlerFailedException::getWrappedExceptions()Oct 24, 2023
@nicolas-grekas
Copy link
Member

Thank you@kbond.

@nicolas-grekasnicolas-grekas merged commiteb7d88d intosymfony:6.4Oct 25, 2023
This was referencedOct 29, 2023
@kbondkbond deleted the exception-handler-name branchNovember 13, 2024 14:55
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

@chalasrchalasrchalasr left review comments

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Labels

FeatureMessenger❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

6 participants

@kbond@fabpot@chalasr@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp