Movatterモバイル変換

carsonbot added this to the6.4 milestone

VincentLanglet mentioned this pull request

[HttpKernel] ErrorListener::onKernelException should ignore exception thrown afterkernel.terminate#51912

Closed

fabpot modified the milestones:6.4,7.1

nicolas-grekas reviewed

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.

I likely misunderstood your proposal but I was expecting the new method on classExceptionEvent.

VincentLanglet changed the title~~Introduce isTerminated method to skip error rendering when kernel is terminated~~Introduce ExceptionEvent::isKernelTerminated method to skip error rendering when kernel is terminated

Copy link

ContributorAuthor

VincentLanglet commentedOct 18, 2023

I likely misunderstood your proposal but I was expecting the new method on classExceptionEvent.

Indeed I thought about it first, then I saw the kernel was accessible in the ExceptionEvent, this would have avoid an extra param. Also, it would have avoid having to think about passing both the kernel and the terminated state.

I updated the PR to follow this strategy if it's preferred@nicolas-grekas

VincentLanglet force-pushed theterminated branch froma892f94 toc9dfb2dCompare

October 18, 2023 09:24

nicolas-grekas reviewed

src/Symfony/Component/HttpKernel/EventListener/ErrorListener.php OutdatedShow resolvedHide resolved

src/Symfony/Component/HttpKernel/HttpKernel.php OutdatedShow resolvedHide resolved

nicolas-grekas added HttpKernel and removed Bug labels

carsonbot changed the title~~Introduce ExceptionEvent::isKernelTerminated method to skip error rendering when kernel is terminated~~[HttpKernel] Introduce ExceptionEvent::isKernelTerminated method to skip error rendering when kernel is terminated

Copy link

Member

nicolas-grekas commentedOct 18, 2023

Don't miss adding a line to the changelog of the component.

Copy link

ContributorAuthor

VincentLanglet commentedOct 18, 2023

Don't miss adding a line to the changelog of the component.

Should it be in the 6.4 changelog or should I create a 7.1 one ?

Copy link

Member

nicolas-grekas commentedOct 18, 2023

7.1 indeed now

nicolas-grekas reviewed

src/Symfony/Component/HttpKernel/Tests/EventListener/ErrorListenerTest.php OutdatedShow resolvedHide resolved

src/Symfony/Component/HttpKernel/Tests/EventListener/ErrorListenerTest.phpShow resolvedHide resolved

nicolas-grekas changed the title~~[HttpKernel] Introduce ExceptionEvent::isKernelTerminated method to skip error rendering when kernel is terminated~~[HttpKernel] IntroduceExceptionEvent::isKernelTerminating() to skip error rendering when kernel is terminating

VincentLanglet force-pushed theterminated branch 2 times, most recently from76d7513 to5a1ecf8Compare

October 18, 2023 10:22

nicolas-grekas approved these changes

Nov 20, 2023

UPGRADE-7.1.md OutdatedShow resolvedHide resolved

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

Nov 20, 2023

fabpot approved these changes

[HttpKernel] IntroduceExceptionEvent::isKernelTerminating() to ski…

68673a3

…p error rendering when kernel is terminating

fabpot force-pushed theterminated branch fromec6eb2b to68673a3Compare

November 23, 2023 07:08

Copy link

Member

fabpot commentedNov 23, 2023

Thank you@VincentLanglet.

fabpot mentioned this pull request

[HttpKernel] IntroduceExceptionEvent::isKernelTerminating() to skip …symfony/symfony-docs#19174

Closed

fabpot merged commit638f7d0 intosymfony:7.1

OskarStark reviewed

UPGRADE-7.1.md

		HttpKernel
		----------

		*`ExceptionEvent` now takes an optional`$isKernelTerminating` parameter

Copy link

Contributor

OskarStarkNov 23, 2023

Choose a reason for hiding this comment

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

Shouldn't we add this to the CHANGELOG instead to the upgrade guide, as this is an optional one?

xabbuh mentioned this pull request