Skip to content

[HttpClient][Psr18Client] Remove Psr18ExceptionTrait #35025

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

fancyweb
Copy link
Contributor

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

Interesting case. Declared trait are instantly loaded by PHP. When ClassExistenceResource fails to load this class because of a thrown \LogicException (when a dependency is missing, cf the top of this class) the trait is loaded. Then, to display the exception, this class is reaccessed (I guess by the duplicated request 🤔) and it results in a fatal error "Cannot redeclare trait...". Basically ClassExistenceResource does not support this kind of structure (thrown exception + trait). Let's do the easy fix first, and then revert if someone finds a fix for the root problem?

@fancyweb
Copy link
Contributor Author

Or we can if (true)? As you want.

@nicolas-grekas nicolas-grekas added this to the 4.3 milestone Dec 18, 2019
@stof
Copy link
Member

stof commented Dec 18, 2019

Another solution could be to move these internal classes to separate files, to let the autoloader load them (which should prevent the issue, as each file will not declare other things that what the autoloader expects to be there)

@fancyweb
Copy link
Contributor Author

Moving the trait works but I think Nicolas did this to keep everything inside one file.

@nicolas-grekas
Copy link
Member

Yes, I don't want each adapter to spread many files around.

@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

nicolas-grekas added a commit that referenced this pull request Dec 18, 2019
…yweb)

This PR was merged into the 4.3 branch.

Discussion
----------

[HttpClient][Psr18Client] Remove Psr18ExceptionTrait

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

Interesting case. Declared trait are instantly loaded by PHP. When ClassExistenceResource fails to load this class because of a thrown \LogicException (when a dependency is missing, cf the top of this class) the trait is loaded. Then, to display the exception, this class is reaccessed (I guess by the duplicated request 🤔) and it results in a fatal error "Cannot redeclare trait...". Basically ClassExistenceResource does not support this kind of structure (thrown exception + trait). Let's do the easy fix first, and then revert if someone finds a fix for the root problem?

Commits
-------

c1746d8 [HttpClient][Psr18Client] Remove Psr18ExceptionTrait
@nicolas-grekas nicolas-grekas merged commit c1746d8 into symfony:4.3 Dec 18, 2019
@fancyweb fancyweb deleted the http-client-psr18-client-remove-trait branch December 19, 2019 08:09
This was referenced Dec 19, 2019
@fabpot fabpot mentioned this pull request Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants