Skip to content

fix(bot): drain webhooks and close the Discord session on SIGTERM - #202

Merged
dimoschi merged 2 commits into
mainfrom
fix/graceful-sigterm
Oct 6, 2026
Merged

dimoschi merged 2 commits into
mainfrom
fix/graceful-sigterm

Conversation

@dimoschi

@dimoschi dimoschi commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Types of changes

  • Bugfix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality not to work as expected).
  • Documentation Update (if none of the other choices applies).

Proposed changes

On SIGTERM the bot kept its Discord session open until SIGKILL. hypercorn's worker_serve installs its own SIGTERM/SIGINT handlers when shutdown_trigger is None, and they replace the ones py-cord's Client.run sets. So SIGTERM only stopped the webhook server. With one pod per bot token, an eviction meant two sessions on the same token for the whole grace period.

Now there is one shutdown sequence:

  1. SIGTERM or SIGINT sets a shutdown event (src/webhooks/server.py).
  2. hypercorn uses that event as its shutdown_trigger. It stops accepting requests and lets in-flight webhooks finish, up to graceful_timeout = 50. The bot stays connected meanwhile, because the handlers need it.
  3. The bot closes its Discord session, bot.run returns, and the process exits.

The 50s is an upper bound. An idle server stops at once, so the bot stays connected only as long as the slowest in-flight webhook. The pod needs a grace period above 50s: hackthebox/kubernetes-deploy#5511 sets 60s.

Also:

  • bot.run(...) is called directly. The old bot.loop.create_task(bot.run(...)) ran bot.run first and then called create_task(None) on a closed loop, so the process exited 1 with "Event loop is closed".
  • The bot token is no longer logged at DEBUG.

Checklist

  • I have read and followed the CONTRIBUTING.md
    doc.
  • Lint and unit tests pass locally with my changes.
  • I have added the necessary documentation (if appropriate).

Additional context

Tests:

  • tests/src/webhooks/test_server_serve.py: SIGTERM and SIGINT each make the server drain and then close the bot, in that order. The drain timeout is 50s.
  • tests/src/test_no_token_in_logs.py: fails if any log call in src/ reads a TOKEN attribute.

End-to-end check with py-cord 2.7.1's Client.run, real hypercorn, a stubbed gateway and a real SIGTERM sent during a 2s webhook request:

  1.02s webhook: request started (takes 2s)
  1.50s sending SIGTERM
  3.02s webhook: request finished
  3.02s client: got HTTP 200
  3.02s webhook server: drained
  3.02s bot: close() called, gateway session closed
  3.02s process: bot.run returned, exiting      (exit 0)

hypercorn drain time, measured with the 50s timeout: 0.00s with an idle keep-alive connection, 1.8s with a 2s request in flight.

In the container, SIGTERM reaches python -m src through uv (PID 1), task and sh.

Hypercorn installed its own SIGTERM/SIGINT handlers when the webhook server started, replacing the ones py-cord sets in Client.run. On SIGTERM only the webhook server stopped and the bot kept its gateway session until SIGKILL. Passing a shutdown trigger keeps py-cord's handlers, so the bot closes and the process exits right away. Also call bot.run directly instead of wrapping its result in create_task.
@dimoschi
dimoschi requested a review from makelarisjr as a code owner October 5, 2026 14:45
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.12%. Comparing base (99fe0e8) to head (5c61a57).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   67.20%   67.12%   -0.08%     
==========================================
  Files          74       76       +2     
  Lines        3436     3501      +65     
==========================================
+ Hits         2309     2350      +41     
- Misses       1127     1151      +24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ToxicBiohazard ToxicBiohazard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is a clear write-up and the diagnosis holds up. I checked it against the versions we pin:

  • Client.run in py-cord 2.7.1 sets SIGINT/SIGTERM to call loop.stop. When the loop stops, its cleanup cancels the remaining tasks, and the runner's finally calls close().
  • worker_serve in hypercorn 0.17 calls loop.add_signal_handler for SIGINT/SIGTERM/SIGBREAK when shutdown_trigger is None. Its task first runs after run_forever() starts, so these handlers replace py-cord's.
  • The old bot.loop.create_task(bot.run(...)) evaluated bot.run(...) first, which blocks until the loop is closed. It then passed None to create_task on a closed loop, which explains the Event loop is closed error and exit code 1.

The problem is what this fix does to webhooks during shutdown. Hypercorn never gets to drain, so on SIGTERM any in-flight webhook request is cancelled along with everything else. The webhook source doesn't retry, so those events are lost, or applied only partway. Details and a suggested approach are inline. Everything else is minor.

Comment thread src/__main__.py Outdated
Comment thread src/webhooks/server.py Outdated
SIGTERM and SIGINT now set one shutdown event. hypercorn uses it as its shutdown trigger: it stops accepting requests and lets in-flight webhooks finish (up to 50s), then the bot closes its Discord session. Before, py-cord's handler stopped the loop and cancelled in-flight webhooks.

The 50s timeout is an upper bound: an idle server stops at once, so the bot stays connected only as long as the slowest in-flight webhook.

Also stops logging the bot token at DEBUG, with a test that fails if a log call includes a token.
@dimoschi dimoschi changed the title fix(bot): close the Discord session on SIGTERM fix(bot): drain webhooks and close the Discord session on SIGTERM Oct 6, 2026

@ToxicBiohazard ToxicBiohazard left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thanks!

@dimoschi
dimoschi merged commit f4a4dc3 into main Oct 6, 2026
4 checks passed
@dimoschi
dimoschi deleted the fix/graceful-sigterm branch October 6, 2026 12:12
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.

2 participants