NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[HttpClient] Fix handling thrown \Exception in \Generator in MockResponse#44438

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

Jump to bottom

Merged

nicolas-grekas merged 1 commit intosymfony:4.4fromfancyweb:http-client/fix-mock-response-throw-exception

Dec 11, 2021

Merged

[HttpClient] Fix handling thrown \Exception in \Generator in MockResponse#44438

nicolas-grekas merged 1 commit intosymfony:4.4fromfancyweb:http-client/fix-mock-response-throw-exception

Dec 11, 2021

Conversation

Copy link

Contributor

fancyweb commentedDec 3, 2021

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

carsonbot added Status: Needs Review Bug HttpClient labels

Dec 3, 2021

carsonbot added this to the4.4 milestone

Dec 3, 2021

fancyweb force-pushed thehttp-client/fix-mock-response-throw-exception branch from810e8a6 to6ba7548Compare

December 3, 2021 11:28

Copy link

ContributorAuthor

fancyweb commentedDec 3, 2021

Failure is related, I need to investigate.

chalasr added Status: Needs Work and removed Status: Needs Review labels

Dec 3, 2021

Copy link

ContributorAuthor

@nicolas-grekas The failure is related to the PHP version. It seems that the destructor ofErrorChunk is not triggered in the same way on 7.2 and above, I don't really know what should I do here. Keep the current behavior (direct throw) on 7.2? Modify the test so that it doesn't rely on the destructor? Something else?

fancyweb force-pushed thehttp-client/fix-mock-response-throw-exception branch 4 times, most recently from3beaf09 tof2867f4Compare

December 11, 2021 10:34

[HttpClient] Fix handling thrown \Exception in \Generator in MockResp…

c71265b

…onse

fancyweb force-pushed thehttp-client/fix-mock-response-throw-exception branch fromf2867f4 toc71265bCompare

December 11, 2021 11:03

fancyweb commented

Dec 11, 2021

View reviewed changes

src/Symfony/Component/HttpClient/Tests/MockHttpClientTest.php

		$responses[] =newMockResponse($body, ['response_headers' =>$headers]);
		$responses[] =newMockResponse($body, ['response_headers' =>$headers]);
		$responses[] =newMockResponse((function () {thrownew \Exception('Fake connection timeout');yield''; })(), ['response_headers' =>$headers]);
		$responses[] =newMockResponse((function () {yield''; })(), ['response_headers' =>$headers]);