Fix slack alert retries and text length limits #1353
Conversation
|
WalkthroughThe changes involve modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- apps/webapp/app/models/orgIntegration.server.ts (1 hunks)
- apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (7 hunks)
🔇 Additional comments not posted (5)
apps/webapp/app/models/orgIntegration.server.ts (1)
72-80: LGTM! Consider verifying retry configuration and addressing other objectives.The changes to the
WebClientconfiguration align with the PR objective of addressing the retry policy issue. The new configuration limits retries and sets reasonable timeouts, which should help prevent excessive retries.Please consider the following points:
Verify that the retry configuration values (2 retries, 5s max timeout, 10s max retry time) are optimal for your specific use case.
The PR objectives also mention issues with text length limits and final errors handling. These are not addressed in this file. Ensure that these objectives are covered in other parts of the PR.
To help verify the changes, you can run the following script to check for any other usages of
WebClientthat might need similar configuration:This will help ensure consistency across the codebase in handling Slack client configurations.
✅ Verification successful
To continue the verification, please run the following script to search for other
WebClientinstantiations in TypeScript files:
Verified no additional WebClient instantiations.
There are no other instances of
WebClientbeing instantiated outside ofapps/webapp/app/models/orgIntegration.server.ts. This ensures that the retry configuration changes are consistently applied.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other WebClient instantiations rg --type typescript 'new WebClient\(' -g '!**/orgIntegration.server.ts'Length of output: 109
Script:
#!/bin/bash # Search for other WebClient instantiations excluding orgIntegration.server.ts rg 'new WebClient\(' -g '*.ts' -g '*.tsx' -g '!**/orgIntegration.server.ts'Length of output: 77
apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (4)
634-636: Ensure error messages are properly formatted and truncated.The use of
this.#wrapInCodeBlockenhances error message formatting and ensures messages adhere to Slack's length limitations. This helps prevent errors due to message length exceeding Slack's limits.
746-748: Consistent formatting of error messages in Slack alerts.Applying
this.#wrapInCodeBlockto the error messages improves readability and maintains consistency across different alert types sent to Slack.
846-848: Appropriate handling of deployment error messages.Wrapping the deployment error messages with
this.#wrapInCodeBlockensures that large error messages are properly formatted and truncated, preventing potential issues with Slack's message length restrictions.
1027-1033: Properly handle Slack 'invalid_blocks' errors to prevent retries.By checking for the
"invalid_blocks"error and throwing aSkipRetryError, the code prevents unnecessary retries for errors that won't succeed on subsequent attempts. This enhances the efficiency of the alert delivery system.
A few things were causing issues with Slack alerts:
This has all been fixed.
Summary by CodeRabbit
New Features
Bug Fixes