Skip to content

logger: retry transient Slack webhook failures#4957

Open
chrismaree wants to merge 1 commit into
masterfrom
chrismaree/logger-slack-retry
Open

logger: retry transient Slack webhook failures#4957
chrismaree wants to merge 1 commit into
masterfrom
chrismaree/logger-slack-retry

Conversation

@chrismaree

Copy link
Copy Markdown
Member

What Changed

  • Added bounded retry handling around Slack webhook posts in packages/logger/src/logger/SlackTransport.ts.
  • Retries Slack 429 responses and common transient 5xx statuses before surfacing a TransportError.
  • Honors Slack Retry-After headers when present and caps the wait at 5 seconds to avoid long logger stalls.
  • Added focused unit coverage for retry success, retry exhaustion, non-retryable failures, and Retry-After capping.

Why

  • The PagerDuty incident was caused by Slack returning HTTP 429 to a normal monitor notification.
  • Before this change, any Slack post failure immediately became a TransportError, which can be routed into PagerDuty when logTransportErrors is enabled.
  • Retrying transient Slack delivery failures gives rate limits and short Slack-side outages a chance to clear before the logger emits a transport failure.
  • Non-transient webhook errors, such as malformed payloads or revoked/bad webhook responses, still fail fast so real delivery misconfiguration remains visible.

Impact

  • Reduces noisy PagerDuty incidents caused by transient Slack webhook rate limiting.
  • Keeps existing Slack routing behavior and payload formatting unchanged.
  • Keeps the final failure mode unchanged after retry exhaustion: the logger still returns a Slack TransportError to the existing transport-error path.
  • Does not include deployment env values or webhook URLs.

High risk Sections to review with detail

  • postWithRetry in SlackTransport.ts: retry count, retryable status list, and delay cap are the main behavior changes.
  • Split-message Slack payload flow: each chunk now uses the same retry helper, so long messages may retry per chunk.
  • getSlackPostRetryDelaySeconds: validates both numeric and date-form Retry-After headers while bounding delay time.

Validation

  • Ran git diff --check and git diff --cached --check successfully.
  • Did not run builds or tests, per repo instructions not to run them unless explicitly requested.
  • Added tests in packages/logger/test/logger/SlackTransport.retries.js covering the intended retry behavior.

Docs

  • No repo docs update was needed; this package does not have checked-in logger operational docs for Slack webhook retry policy or the managed OO monitor deployment env.

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.

1 participant