Skip to content

[HttpClient] add TimeoutExceptionInterface #36766

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
merged 1 commit into from
May 10, 2020

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 8, 2020

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

This is fixing a design issue: unlike any other TransportExceptionInterface, timeouts are not fatal transport errors. For this reason, we must allow one to identify them.

This PR also marks exception classes as @internal, to encourage ppl to catch them using their interface and not their class name. this would revert #30549

@@ -23,7 +23,7 @@
"require": {
"php": "^7.2.5",
"psr/log": "^1.0",
"symfony/http-client-contracts": "^1.1.8|^2",
"symfony/http-client-contracts": "^2.1.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

tag to be added on symfony/contracts and symfony/http-client-contracts after merge

@nicolas-grekas nicolas-grekas force-pushed the hc-timeout-if branch 4 times, most recently from e7a45e8 to ab236d6 Compare May 9, 2020 13:11
@nicolas-grekas nicolas-grekas force-pushed the hc-timeout-if branch 10 times, most recently from d80448b to e89a845 Compare May 9, 2020 17:44
@fabpot
Copy link
Member

fabpot commented May 10, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 3acc28f into symfony:master May 10, 2020
@fabpot fabpot mentioned this pull request May 16, 2020
@nicolas-grekas nicolas-grekas deleted the hc-timeout-if branch May 26, 2020 06:59
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.

5 participants