Skip to content

[FIX] Add timeouts on outbound HTTP calls outside mod_ci - #1220

Open
devnishantt wants to merge 1 commit into
CCExtractor:masterfrom
devnishantt:fix/http-timeouts-1192
Open

devnishantt wants to merge 1 commit into
CCExtractor:masterfrom
devnishantt:fix/http-timeouts-1192

Conversation

@devnishantt

Copy link
Copy Markdown

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):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have used the project briefly.

Fixes #1192

#943 added timeouts inside mod_ci/controllers.py only. Five outbound
requests calls outside that file still had no timeout, so a hung
GitHub/Mailgun connection could wedge a worker forever.

Changes

Add timeout=(3.05, 10) (connect, read) at the five sites from the issue:

  1. utility.py — GitHub /meta (webhook IP allowlist)
  2. mod_auth/controllers.py — OAuth token exchange
  3. mod_auth/controllers.py — token validity check
  4. mailer.py — Mailgun send
  5. mod_upload/controllers.py — create GitHub issue

HTTP_TIMEOUT lives in utility.py for the Flask-using modules.
mailer.py uses the same tuple inline so it does not import Flask.

No extra exception handlers. mod_ci is unchanged.

Tests

  • tests/test_utility.py
  • tests/test_auth/test_controllers.py
  • tests/test_mailer.py (2 passed locally)
  • tests/test_upload/test_controllers.py

Screenshots / video

Not applicable. This is HTTP timeout kwargs, not UI.

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>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Missing timeouts on 5 outbound HTTP calls outside mod_ci (follow-up to #943)

1 participant