Skip to content
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

[FIX] Add http timeouts even if no http_proxy is used #1034

Merged
merged 5 commits into from
Jul 3, 2020

Conversation

etlam
Copy link
Contributor

@etlam etlam commented Jun 29, 2020

Fixes #1033

etlam added 3 commits June 29, 2020 10:26
ERROR: UndefinedClass - src/HttpClient/HttpClientFactory.php:117:21
UndefinedClass - src/HttpClient/HttpClientFactory.php:118:21
@etlam
Copy link
Contributor Author

etlam commented Jun 29, 2020

I don't know how to suppress the UndefinedClass error in psalm, maybe someone can help

@Jean85 Jean85 requested a review from ste93cry June 29, 2020 08:50
@Jean85 Jean85 added this to the 2.4 milestone Jun 29, 2020
Copy link
Collaborator

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Code looks fine, apart from the static analysis failures.

IMHO we should use stubs or install the optional dependencies while running those; I would prefer the second, since it would always work, in local too.

@ste93cry
Copy link
Collaborator

I don't know how to suppress the UndefinedClass error in psalm

Just move the annotation before the declaration of the $guzzleConfig variable and drop all the remaining unnecessary annotations

I would prefer the second, since it would always work, in local too

We could install those dependencies as part of require-dev, but then it would be harder to test the SDK with another HTTP client as the order in which the clients are discovered is not something we can decide. Even moving the installation of suchs deps to the CI means that we still have to suppress the errors locally

ERROR: UndefinedClass - src/HttpClient/HttpClientFactory.php:118:21 - Class or interface GuzzleHttp\RequestOptions does not exist (see https://psalm.dev/019)
ERROR: UndefinedClass - src/HttpClient/HttpClientFactory.php:120:21 - Class or interface GuzzleHttp\RequestOptions does not exist (see https://psalm.dev/019)
@etlam
Copy link
Contributor Author

etlam commented Jul 3, 2020

Just move the annotation before the declaration of the $guzzleConfig variable and drop all the remaining unnecessary annotations

Thanks!

How can I help to get the patch into the next release?

@ste93cry
Copy link
Collaborator

ste93cry commented Jul 3, 2020

Would you mind adding also an entry to the CHANGELOG? I always forget about asking this 🤦

How can I help to get the patch into the next release?

As soon as we merge it it will be already part of the next patch release which I would like to release shortly after

@ste93cry ste93cry merged commit c6acde7 into getsentry:master Jul 3, 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.

http_timeout and connection_timeout only working with a http proxy
3 participants