Skip to content

fix: do not await slack when handling function success#5175

Merged
TBonnin merged 1 commit intomasterfrom
tbonnin/nan-4182/slack-void
Dec 19, 2025
Merged

fix: do not await slack when handling function success#5175
TBonnin merged 1 commit intomasterfrom
tbonnin/nan-4182/slack-void

Conversation

@TBonnin
Copy link
Collaborator

@TBonnin TBonnin commented Dec 18, 2025

Some calls to slackService were still being awaited. We don't want to await SlackService function to prevent delaying the functions result processing when Slack is slow/ratelimiting/unresponsive. Other call to SlackService in the same files were already not awaited


Avoid awaiting Slack cleanup during success handlers

Updates handleActionSuccess and the sync success flow to fire-and-forget slackService.removeFailingConnection so Slack latency does not delay job completion. Aligns these success paths with other non-blocking Slack invocations already present in the codebase.

Key Changes

• Switched slackService.removeFailingConnection to run via void in packages/jobs/lib/execution/action.ts success handler
• Applied the same non-blocking pattern in packages/jobs/lib/execution/sync.ts success handler

Affected Areas

• packages/jobs/lib/execution/action.ts
• packages/jobs/lib/execution/sync.ts


This summary was automatically generated by @propel-code-bot

Some calls to slackService were still being awaited.
We don't want to await SlackService function to prevent delaying the functions result
processing when Slack is slow/ratelimiting/unresponsive. Other call to
SlackService in the same files were already not awaited
@TBonnin TBonnin requested a review from a team December 18, 2025 21:14
@linear
Copy link

linear bot commented Dec 18, 2025

@my-senior-dev-pr-review
Copy link

🤖 My Senior Dev

2 files reviewed • 1 need attention

⚠️ Needs Attention:

  • packages/jobs/lib/execution/action.ts — The change affects error handling behavior critically by ignoring promise rejections, which could lead to silent failures.

View full analysis →


💬 Try: @my-senior-dev explain this change or summon @chaos-monkey 🐵 @security-auditor 🔒 @optimizer ⚡ for different perspectives

📖 All commands & personas

You can interact with me by mentioning @my-senior-dev in any comment:

In PR comments or on any line of code:

  • Ask questions about the code or PR
  • Request explanations of specific changes
  • Get suggestions for improvements

Slash commands:

  • /help — Show all available commands
  • /archeology — See the history and evolution of changed files
  • /profile — Performance analysis and suggestions
  • /expertise — Find who knows this code best
  • /personas — List all available AI personas

AI Personas (mention to get their perspective):

Persona Focus
@chaos-monkey 🐵 Edge cases & failure scenarios
@skeptic 🤨 Challenge assumptions
@optimizer Performance & efficiency
@security-auditor 🔒 Security vulnerabilities
@accessibility-advocate Inclusive design
@junior-dev 🌱 Simple explanations
@tech-debt-collector 💳 Code quality & shortcuts
@ux-champion 🎨 User experience
@devops-engineer 🚀 Deployment & scaling
@documentation-nazi 📚 Documentation gaps
@legacy-whisperer 🏛️ Working with existing code
@test-driven-purist Testing & TDD

For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews.

@TBonnin TBonnin added this pull request to the merge queue Dec 19, 2025
Merged via the queue into master with commit 77def9a Dec 19, 2025
24 of 26 checks passed
@TBonnin TBonnin deleted the tbonnin/nan-4182/slack-void branch December 19, 2025 14:01
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