Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 24, 2025

Functions that post to Slack webhooks were raising obscure KeyError exceptions when required environment variables were missing. This change adds explicit validation and clear error messages.

Error handling improvements:

  • post_to_slack: Now raises RuntimeError with clear message when SLACK_WEBHOOK_URL is missing instead of generic KeyError
  • post_to_manager_slack: Returns gracefully with logged warning when MANAGER_SLACK_WEBHOOK_URL is missing, since manager notifications are optional

Code quality improvements:

  • Extract magic numbers (3, 5) to named constants RETRY_COUNT and RETRY_SLEEP_SECONDS
  • Add docstring to with_retries decorator documenting retry behavior
# Before: Raises KeyError: 'SLACK_WEBHOOK_URL'
url = os.environ["SLACK_WEBHOOK_URL"]

# After: Raises RuntimeError with clear message
url = os.environ.get("SLACK_WEBHOOK_URL")
if not url:
    logging.error("SLACK_WEBHOOK_URL environment variable is not set or empty.")
    raise RuntimeError("Missing SLACK_WEBHOOK_URL environment variable.")
Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The function logs errors but doesn't handle the case where SLACK_WEBHOOK_URL environment variable is missing, which would cause a KeyError with an unclear error message.","fixFiles":[{"filePath":"jobs.py","diff":"diff --git a/jobs.py b/jobs.py\n--- a/jobs.py\n+++ b/jobs.py\n@@ -37,7 +37,10 @@\n \n def post_to_slack(markdown: str):\n     \"\"\"Send a message to Slack and raise or log on failure.\"\"\"\n-    url = os.environ[\"SLACK_WEBHOOK_URL\"]\n+    url = os.environ.get(\"SLACK_WEBHOOK_URL\")\n+    if not url:\n+        logging.error(\"SLACK_WEBHOOK_URL environment variable is not set or empty.\")\n+        raise RuntimeError(\"Missing SLACK_WEBHOOK_URL environment variable.\")\n     response = requests.post(url, json={\"text\": markdown})\n     if response.status_code != 200:\n         logging.error(\"Slack API returned %s: %s\", response.status_code, response.text)\n"}]},{"message":"The function doesn't handle the case where MANAGER_SLACK_WEBHOOK_URL environment variable is missing, which would cause a KeyError with an unclear error message.","fixFiles":[{"filePath":"jobs.py","diff":"diff --git a/jobs.py b/jobs.py\n--- a/jobs.py\n+++ b/jobs.py\n@@ -46,7 +46,13 @@\n \n def post_to_manager_slack(markdown: str):\n     \"\"\"Send a message to the manager Slack webhook and raise or log on failure.\"\"\"\n-    url = os.environ[\"MANAGER_SLACK_WEBHOOK_URL\"]\n+    url = os.environ.get(\"MANAGER_SLACK_WEBHOOK_URL\")\n+    if not url:\n+        logging.error(\n+            \"MANAGER_SLACK_WEBHOOK_URL environment variable is not set; \"\n+            \"unable to send manager Slack message.\"\n+        )\n+        return\n     response = requests.post(url, json={\"text\": markdown})\n     if response.status_code != 200:\n         logging.error(\n"}]},{"message":"The with_retries decorator lacks documentation explaining its retry behavior, sleep duration, and exception handling strategy.","fixFiles":[{"filePath":"jobs.py","diff":"diff --git a/jobs.py b/jobs.py\n--- a/jobs.py\n+++ b/jobs.py\n@@ -74,6 +74,11 @@\n \n \n def with_retries(func):\n+    \"\"\"Decorator that retries the wrapped function up to 3 times.\n+\n+    On each failure, it logs the exception, waits 5 seconds, and retries.\n+    After the third failed attempt, the last exception is re-raised.\n+    \"\"\"\n     @wraps(func)\n     def wrapper(*args, **kwargs):\n         for attempt in range(3):\n"}]},{"message":"The retry count (3) and sleep duration (5) are magic numbers that should be configurable parameters or named constants.","fixFiles":[{"filePath":"jobs.py","diff":"diff --git a/jobs.py b/jobs.py\n--- a/jobs.py\n+++ b/jobs.py\n@@ -34,7 +34,11 @@\n \n load_dotenv()\n \n+# Retry configuration for the with_retries decorator.\n+RETRY_COUNT = 3\n+RETRY_SLEEP_SECONDS = 5\n \n+\n def post_to_slack(markdown: str):\n     \"\"\"Send a message to Slack and raise or log on failure.\"\"\"\n     url = os.environ[\"SLACK_WEBHOOK_URL\"]\n@@ -76,14 +79,14 @@\n def with_retries(func):\n     @wraps(func)\n     def wrapper(*args, **kwargs):\n-        for attempt in range(3):\n+        for attempt in range(RETRY_COUNT):\n             try:\n                 return func(*args, **kwargs)\n             except Exception as e:\n                 logging.error(f\"Function {func.__name__} failed: {e}\")\n-                if attempt == 2:\n+                if attempt == RETRY_COUNT - 1:\n                     raise\n-                time.sleep(5)\n+                time.sleep(RETRY_SLEEP_SECONDS)\n \n     return wrapper\n \n"}]},{"message":"The sleep duration (5 seconds) is a magic number that should be a configurable parameter or named constant.","fixFiles":[{"filePath":"jobs.py","diff":"diff --git a/jobs.py b/jobs.py\n--- a/jobs.py\n+++ b/jobs.py\n@@ -34,7 +34,10 @@\n \n load_dotenv()\n \n+# Number of seconds to wait between retry attempts in the with_retries decorator.\n+RETRY_SLEEP_SECONDS = 5\n \n+\n def post_to_slack(markdown: str):\n     \"\"\"Send a message to Slack and raise or log on failure.\"\"\"\n     url = os.environ[\"SLACK_WEBHOOK_URL\"]\n@@ -83,7 +85,7 @@\n                 logging.error(f\"Function {func.__name__} failed: {e}\")\n                 if attempt == 2:\n                     raise\n-                time.sleep(5)\n+                time.sleep(RETRY_SLEEP_SECONDS)\n \n     return wrapper\n \n"}]}]

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

… documentation

- Add RETRY_COUNT and RETRY_SLEEP_SECONDS constants to avoid magic numbers
- Fix post_to_slack to handle missing SLACK_WEBHOOK_URL with clear error message
- Fix post_to_manager_slack to handle missing MANAGER_SLACK_WEBHOOK_URL gracefully
- Add comprehensive documentation to with_retries decorator
- Update with_retries to use named constants instead of hardcoded values

Co-authored-by: redreceipt <[email protected]>
@redreceipt redreceipt temporarily deployed to bug-board-copilot-fix-s-wmsl47 December 24, 2025 06:21 Inactive
- Update with_retries docstring to reference RETRY_COUNT and RETRY_SLEEP_SECONDS constants
- Update post_to_manager_slack docstring to reflect graceful degradation behavior

Co-authored-by: redreceipt <[email protected]>
@redreceipt redreceipt temporarily deployed to bug-board-copilot-fix-s-wmsl47 December 24, 2025 06:22 Inactive
@redreceipt redreceipt temporarily deployed to bug-board-copilot-fix-s-wmsl47 December 24, 2025 06:23 Inactive
Copilot AI changed the title [WIP] Handle missing SLACK_WEBHOOK_URL in error logging Add error handling for missing Slack webhook environment variables Dec 24, 2025
Copilot AI requested a review from redreceipt December 24, 2025 06:25
@redreceipt redreceipt marked this pull request as ready for review December 24, 2025 06:30
@redreceipt redreceipt merged commit 3a2328b into main Dec 24, 2025
7 checks passed
@redreceipt redreceipt deleted the copilot/fix-slack-webhook-url-error branch December 24, 2025 06:31
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