Skip to content

Conversation

@shaperman
Copy link
Contributor

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues

@terabytesoftw
Copy link
Member

terabytesoftw commented Aug 15, 2025

Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
@samdark
Copy link
Member

samdark commented Aug 18, 2025

That solves the issue but, I think, we have more places like that. I don't particularly like how it was solved in Yii2 in the first place. For Yii3, we did it like the following: https://github.com/yiisoft/files/blob/master/src/FileHelper.php#L71

@samdark samdark requested a review from a team August 18, 2025 15:25
Co-authored-by: Alexander Makarov <sam@rmcreative.ru>
@terabytesoftw
Copy link
Member

That solves the issue but, I think, we have more places like that. I don't particularly like how it was solved in Yii2 in the first place. For Yii3, we did it like the following: https://github.com/yiisoft/files/blob/master/src/FileHelper.php#L71

We should then adapt the best solution, and identify where to apply it to do so.

@samdark
Copy link
Member

samdark commented Aug 19, 2025

Current implementation is missing closing of the steam in catch block as well.

@samdark
Copy link
Member

samdark commented Aug 21, 2025

@terabytesoftw would you please check why tests fail in master?

@terabytesoftw
Copy link
Member

1) yiiunit\extensions\httpclient\StreamTransportTest::testSendError
Undefined variable: stream

/home/runner/work/yii2-httpclient/yii2-httpclient/src/StreamTransport.php:71
/home/runner/work/yii2-httpclient/yii2-httpclient/src/Client.php:233
/home/runner/work/yii2-httpclient/yii2-httpclient/src/Request.php:451
/home/runner/work/yii2-httpclient/yii2-httpclient/tests/TransportTestCase.php:141
phpvfscomposer:///home/runner/work/yii2-httpclient/yii2-httpclient/vendor/phpunit/phpunit/phpunit:51
/home/runner/work/yii2-httpclient/yii2-httpclient/vendor/bin/phpunit:118

2) yiiunit\extensions\httpclient\StreamTransportTest::testInvalidUrl
Undefined variable: stream

/home/runner/work/yii2-httpclient/yii2-httpclient/src/StreamTransport.php:71
/home/runner/work/yii2-httpclient/yii2-httpclient/src/Client.php:233
/home/runner/work/yii2-httpclient/yii2-httpclient/src/Request.php:451
/home/runner/work/yii2-httpclient/yii2-httpclient/tests/TransportTestCase.php:254
phpvfscomposer:///home/runner/work/yii2-httpclient/yii2-httpclient/vendor/phpunit/phpunit/phpunit:51
/home/runner/work/yii2-httpclient/yii2-httpclient/vendor/bin/phpunit:118

@samdark samdark merged commit f6a1930 into yiisoft:master Aug 26, 2025
10 checks passed
@samdark
Copy link
Member

samdark commented Aug 26, 2025

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants