Skip to content

Conversation

@zzstoatzz
Copy link
Collaborator

summary

  • fix pydantic validation error when github returns null label descriptions
  • remove ERROR_HANDLED completed state so flows properly fail

changes

github labels fix

  • GitHubLabel.description now accepts str | None
  • github api sometimes returns null for label descriptions
  • fixes validation error: Input should be a valid string [type=string_type, input_value=None]

error handling fix

  • removed Completed(name="ERROR_HANDLED") pattern
  • errors are now re-raised after sending user-friendly slack message
  • maintains good UX while allowing proper flow failure monitoring

why

proper failure states are important for:

  • alerting/monitoring
  • understanding when the bot actually fails vs succeeds
  • debugging issues in production

🤖 Generated with Claude Code

**github labels fix:**
- allow None for label description field (github api returns null sometimes)
- fixes pydantic validation error when fetching issues with labels

**error handling fix:**
- remove ERROR_HANDLED completed state
- errors now properly fail the flow after sending user-friendly message
- maintains user experience while allowing proper monitoring/alerting

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@zzstoatzz zzstoatzz marked this pull request as ready for review October 16, 2025 17:32
Copilot AI review requested due to automatic review settings October 16, 2025 17:32
@zzstoatzz zzstoatzz merged commit 9f0ad64 into main Oct 16, 2025
3 checks passed
@zzstoatzz zzstoatzz deleted the fix-github-labels-and-error-handling branch October 16, 2025 17:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures the Slackbot handles GitHub’s nullable label descriptions and improves error handling to allow flows to properly fail for monitoring.

  • Allow GitHub label descriptions to be None to prevent Pydantic validation errors.
  • Replace swallowing errors with a direct re-raise after notifying the user, removing the ERROR_HANDLED Completed state.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
examples/slackbot/src/slackbot/github/models.py Model change: description now allows None to handle GitHub’s null label descriptions.
examples/slackbot/src/slackbot/api.py Error handling updated to re-raise exceptions rather than returning a Completed(ERROR_HANDLED).

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

name: str = Field(default="")
color: str = Field(default="")
description: str = Field(default="")
description: str | None = Field(default="")
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

[nitpick] The field type allows None but the default remains an empty string. This mixes two different sentinel values for 'no description' and can confuse downstream consumers; consider setting the default to None to align with the Optional type.

Suggested change
description: str | None = Field(default="")
description: str | None = Field(default=None)

Copilot uses AI. Check for mistakes.
Comment on lines +282 to 285
raise
finally:
try:
await mark_thread_completed(db, message_ts)
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

Because mark_thread_completed is in a finally block, it will run even when an exception is re-raised, marking the thread as completed on failure. If the intent is to signal a failed flow, move this call to the success path (outside finally) or add a separate failure path (e.g., mark_thread_failed) for the except branch.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants