Skip to content

fix(slack): Handle archived channels gracefully instead of logging errors#256

Merged
rgibert merged 5 commits into
mainfrom
rgibert/fix-archived-slack-channels
Jul 3, 2026
Merged

fix(slack): Handle archived channels gracefully instead of logging errors#256
rgibert merged 5 commits into
mainfrom
rgibert/fix-archived-slack-channels

Conversation

@rgibert

@rgibert rgibert commented Jun 16, 2026

Copy link
Copy Markdown
Member

Catch the Slack API 'is_archived' error in all channel-mutating methods (join, post, pin, bookmark, invite, rename, set topic) and log a warning instead of an error. Also add an upfront archived-channel check in the backfill handler to skip setup entirely and avoid sending a misleading DM about inviting the bot.

Fixes FIRETOWER-BACKEND-4D
Co-Authored-By: Claude noreply@anthropic.com

Agent transcript: https://claudescope.sentry.dev/share/iSJ8GLrggJu2yvWAMY0Qg8VvHJ9AgfdC3lL8L3cly6A

…rors

Catch the Slack API 'is_archived' error in all channel-mutating methods
(join, post, pin, bookmark, invite, rename, set topic) and log a warning
instead of an error. Also add an upfront archived-channel check in the
backfill handler to skip setup entirely and avoid sending a misleading
DM about inviting the bot.

Include is_archived in get_channel_info return value and reuse the
existing get_channel_info call in backfill setup to avoid a duplicate
API request.

Fixes FIRETOWER-BACKEND-4D
Co-Authored-By: Claude <noreply@anthropic.com>

Agent transcript: https://claudescope.sentry.dev/share/iSJ8GLrggJu2yvWAMY0Qg8VvHJ9AgfdC3lL8L3cly6A
@rgibert rgibert self-assigned this Jun 16, 2026
@rgibert rgibert requested a review from a team as a code owner June 16, 2026 16:01
Comment thread src/firetower/slack_app/handlers/backfill_incident.py
Comment thread src/firetower/integrations/services/slack.py
rgibert added 3 commits June 16, 2026 13:01
…etry

When post_message or add_bookmark hit a not_in_channel error and the
subsequent join_channel call fails, the failure was silently swallowed.
Add explicit error logging for these cases.

Co-Authored-By: Claude <noreply@anthropic.com>

Agent transcript: https://claudescope.sentry.dev/share/WUxpRlQ-Dvgz-R6V962vnuF0JTfCMODq8t1uPCJ65HY
When the initial get_channel_info call returns None (transient API
error) but join_channel succeeds, the rename step was silently skipped
because it short-circuited on the None channel_info. Re-fetch channel
info after a successful join when the initial lookup failed so the
rename proceeds correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Agent transcript: https://claudescope.sentry.dev/share/RgcHeFRWgaj2R6OxQjKoWYR9gJhMlO3Hj9fat4C-AmA
@rgibert rgibert enabled auto-merge (squash) July 3, 2026 14:22
Comment thread src/firetower/slack_app/tests/handlers/test_backfill_incident.py
The side_effect list had only 2 entries: one consumed by
handle_backfill_submission and one by _setup_channel_for_incident's
initial get_channel_info call. Because the second call returned a
valid dict, the `if not channel_info` retry branch was never reached.

Add a third side_effect entry so setup's initial call also returns
None, forcing the retry path after join. Update expected call_count
from 2 to 3 accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Agent transcript: https://claudescope.sentry.dev/share/SZpW242OEy3gh6Vzune4dgeQjtHQ3ESeFLcWvlszNfo

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 950413f. Configure here.

channel_info = _slack_service.get_channel_info(channel_id)

expected_name = build_channel_name(incident)
channel_info = _slack_service.get_channel_info(channel_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ignores archived flag from submission

Medium Severity

During backfill submission, get_channel_info runs before _setup_channel_for_incident, but setup fetches channel info again and only uses that second result for the archived check. If the first call returned is_archived and the second returns None, setup skips the early exit, join_channel fails, and the user still gets the invite-the-bot DM the change was meant to avoid.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 950413f. Configure here.

Comment on lines +53 to +57
logger.warning(
"Skipping setup for archived channel %s on incident %s",
channel_id,
incident.id,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incident created silently with no user notification when channel is archived during backfill submission

When handle_backfill_submission is called for an archived channel, the incident is persisted to the DB but _setup_channel_for_incident returns early at line 57 without sending any DM, leaving the user with no confirmation the incident was created or how to find it.

Evidence
  • handle_backfill_submission calls serializer.save() to create the incident unconditionally, then calls _setup_channel_for_incident.
  • The new early-return block (lines 53–57) logs a warning and returns with no call to client.chat_postMessage.
  • The only user-visible completion message is sent at the bottom of _setup_channel_for_incident after all setup steps — it is never reached for archived channels.
  • The test test_skips_setup_for_archived_channel explicitly asserts client.chat_postMessage.assert_not_called(), confirming this silent behavior is the current design.
  • Before this change, join_channel returned False for archived channels and a DM was sent telling the user to invite the bot — imperfect but at least actionable feedback was delivered.

Identified by Warden find-bugs · EZM-YQ3

@rgibert rgibert merged commit 2993b59 into main Jul 3, 2026
33 checks passed
@rgibert rgibert deleted the rgibert/fix-archived-slack-channels branch July 3, 2026 17:09
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