Repository navigation
[FIX] Add timeouts on outbound HTTP calls outside mod_ci - #1220
Open
devnishantt wants to merge 2 commits into
Open
devnishantt wants to merge 2 commits into
devnishantt wants to merge 2 commits into
Conversation
devnishantt
requested review from
canihavesomecoffee and
thealphadollar
as code owners
September 29, 2026 05:58
|
| data['from'] = self.sender | ||
| try: | ||
| return requests.post(f"{self.api_url}/messages", auth=self.auth, data=data) | ||
| return requests.post(f"{self.api_url}/messages", auth=self.auth, data=data, |
Member
There was a problem hiding this comment.
Why not use the HTTP_TIMEOUT?
canihavesomecoffee
left a comment
canihavesomecoffee
left a comment
Member
There was a problem hiding this comment.
Can you also rebase?
requests has no default timeout, so GitHub meta, OAuth, token checks, Mailgun, and issue creation could hang a worker forever. Use the same (3.05, 10) connect/read tuple already used in fetch_username_from_token. Fixes CCExtractor#1192 Signed-off-by: Nishant <nishanttgithub@gmail.com>
Addresses review feedback: reuse utility.HTTP_TIMEOUT instead of a hardcoded (3.05, 10) tuple. Signed-off-by: Nishant <nishanttgithub@gmail.com>
devnishantt
force-pushed
the
fix/http-timeouts-1192
branch
from
November 12, 2026 09:30
32046b1 to
460615f
Compare
Author
|
Thanks for the review. Rebased onto latest |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please prefix your pull request with one of the following: [FEATURE] [FIX] [IMPROVEMENT]. [FIX]
In raising this pull request, I confirm the following (please check boxes):
My familiarity with the project is as follows (check one):
Fixes #1192
#943 added timeouts inside
mod_ci/controllers.pyonly. Five outboundrequestscalls outside that file still had no timeout, so a hungGitHub/Mailgun connection could wedge a worker forever.
Changes
Add
timeout=(3.05, 10)(connect, read) at the five sites from the issue:utility.py— GitHub/meta(webhook IP allowlist)mod_auth/controllers.py— OAuth token exchangemod_auth/controllers.py— token validity checkmailer.py— Mailgun sendmod_upload/controllers.py— create GitHub issueHTTP_TIMEOUTlives inutility.pyfor the Flask-using modules.mailer.pyuses the same tuple inline so it does not import Flask.No extra exception handlers.
mod_ciis unchanged.Tests
tests/test_utility.pytests/test_auth/test_controllers.pytests/test_mailer.py(2 passedlocally)tests/test_upload/test_controllers.pyScreenshots / video
Not applicable. This is HTTP timeout kwargs, not UI.