Skip to content

fix: make polling startup cancellation-safe - #959

Closed
roshnicdave wants to merge 4 commits into
grammyjs:mainfrom
roshnicdave:fix/cancel-polling-startup
Closed

roshnicdave wants to merge 4 commits into
grammyjs:mainfrom
roshnicdave:fix/cancel-polling-startup

Conversation

@roshnicdave

@roshnicdave roshnicdave commented Aug 17, 2026 •

Copy link
Copy Markdown

Fixes cancellation races between start, stop, and shared bot initialization

-Scope polling controllers and stop confirmation to each run
-Cancel pending setup/retry work and ignore stale polling results after a restart

  • Preserve setup failures and shared initialization behavior for concurrent callers

@codecov

codecov Bot commented Aug 17, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.65537% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.69%. Comparing base (b04bfee) to head (a4eb241).
⚠️ Report is 142 commits behind head on main.

Files with missing lines Patch % Lines
src/bot.ts 92.65% 9 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #959      +/-   ##
==========================================
- Coverage   45.52%   42.69%   -2.84%     
==========================================
  Files          19       19              
  Lines        5520     7383    +1863     
  Branches      360      497     +137     
==========================================
+ Hits         2513     3152     +639     
- Misses       3003     4146    +1143     
- Partials        4       85      +81     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread test/bot.test.ts Outdated
ok: false,
error_code: 429,
description: "Too Many Requests",
parameters: { retry_after: 0 },

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.

How is this test meaningfully different from the test immediately above it?

If the previous test has established that retry occurs - even in the absence of parameters.retry_after for HTTP 429 - what will we be trying to establish with this test with a parameters.retry_after value of 0?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sorry, forgot to look over tests! can you review again 🙏

@KnorpelSenf

Copy link
Copy Markdown
Member

I have no idea which issue you ran into with your bot, and this seems to be just a slop contribution. I don't see why I should spend my free time on a review if you don't spend yours on the quality of your contributions. Closing.

Feel free to join the chat and convince us otherwise if you actually care about this.

@KnorpelSenf KnorpelSenf added the slop disregard this if your time is valuable to you label Aug 23, 2026
@roshnicdave
roshnicdave deleted the fix/cancel-polling-startup branch August 23, 2026 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

slop disregard this if your time is valuable to you

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants