Sentry-python does not set connection timeouts #4247
Labels
Comments
|
Hey @aivarsk, thanks for taking the time to open this -- this would definitely be good to add. I'll take a look. |
sentrivana
added a commit
that referenced
this issue
Apr 10, 2025
For some reason, we don't define any timeouts in our default transport(s). With this change: - We add a 30s total timeout for the whole connect+read cycle in the default HTTP transport - In the experimental HTTP/2 httpcore-based transport there is no way to set a single timeout, so we set 15s each for getting a connection from the pool, connecting, writing, and reading Backend SDKs in general set wildly different timeouts, from 30s in Go to <5s in Ruby or PHP. I went for the higher end of the range here since this is mainly meant to prevent the SDK preventing process shutdown like described in #4247 -- we don't want to cut off legitimate requests that are just taking a long time. (I was considering going even higher, maybe to 60s -- but I think 30s is a good first shot at this and we can always change it later.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
How do you use Sentry?
Sentry Saas (sentry.io)
Version
2.25.1
Steps to Reproduce
When sentry-python tries to report errors during network failures, it may hang up forever while trying to connect.
Expected Result
Inside
transport.pyboth_get_pool_optionsit should set timeouts to something reasonable like:That will allow process to terminate during network failures.
Actual Result
When the PoolManager is created, it gets the default timeout values (https://urllib3.readthedocs.io/en/stable/reference/urllib3.util.html#urllib3.util.Timeout)
Timeout(connect=<_TYPE_DEFAULT.token: -1>, read=<_TYPE_DEFAULT.token: -1>, total=None)This leads to
connectcall being made in blocking mode and it might block forever preventing the process to terminate and restart.The text was updated successfully, but these errors were encountered: