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

feat: Add support timeouts in milliseconds for HTTP client #1783

Conversation

nikolicaleksa
Copy link
Contributor

Adds support for having timeout and connection timeout in milliseconds for HTTP client

@stayallive stayallive requested a review from cleptric October 27, 2024 15:34
@cleptric
Copy link
Member

Why do you see the need to reduce the timeout below 1s?

@nikolicaleksa
Copy link
Contributor Author

Why do you see the need to reduce the timeout below 1s?

TLDR: We would like to reduce it to 250ms, risking dropping some exceptions but not blocking PHP-FPM workers for a second or more.

Detailed:
We encountered the following situation

  • We were getting a lot of requests to the application where some of them were failing and logging the exception to Sentry
  • We are using Sentry Relay, but for some reason it was unavailable
  • Because of that, our PHP-FPM workers were blocked for the full duration of a configured timeout which resulted in a lot of workers being blocked and therefor we were returning 504 errors

We now solved the first issue about many requests by adding the rate limiter, however as an additional precaution we also want to reduce the Sentry timeout to 250ms because we would rather drop some exceptions than block workers for at least a second in case of unavailability.

@cleptric cleptric merged commit af7955a into getsentry:master Oct 28, 2024
33 checks passed
@mfb
Copy link
Contributor

mfb commented Oct 28, 2024

So is CURLOPT_TIMEOUT only an integer? Meaning CURLOPT_TIMEOUT_MS would also be needed to configure 1.5 seconds?

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.

3 participants