Skip to content

feat: data change notify error handling#11

Merged
deleteLater merged 2 commits intomainfrom
fix/data-change-notify
Nov 28, 2025
Merged

feat: data change notify error handling#11
deleteLater merged 2 commits intomainfrom
fix/data-change-notify

Conversation

@deleteLater
Copy link
Contributor

@deleteLater deleteLater commented Nov 28, 2025

added error handling for data update notification

related to featbit/featbit#815

@deleteLater deleteLater changed the title feat: error handling and background processing fix: error handling and background processing Nov 28, 2025
@deleteLater deleteLater changed the title fix: error handling and background processing fix: data change notify error handling Nov 28, 2025
@deleteLater deleteLater requested a review from Copilot November 28, 2025 10:07
@deleteLater deleteLater self-assigned this Nov 28, 2025
@deleteLater deleteLater moved this to In Progress in FeatBit Nov 28, 2025
@deleteLater deleteLater added the bug Something isn't working label Nov 28, 2025
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 adds error handling for data change notifications when processing data synchronization messages. However, the implementation introduces a critical bug through the use of fire-and-forget pattern that can cause unobserved task exceptions.

Key Changes:

  • Added try-catch error handling in DataChangeNotifier.NotifyAsync to gracefully handle exceptions from individual message handlers
  • Changed NotifyItemsUpdated from awaited to fire-and-forget pattern in DataSyncMessageHandler (introduces critical bug)
  • Renamed method from OnItemsUpdated to NotifyItemsUpdated to better reflect its purpose

Reviewed changes

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

File Description
src/Api/Messaging/DataChangeNotifier.cs Added try-catch block around handler.HandleAsync with proper error logging; improved log message from "Handled" to "Notified"
src/Api/DataSynchronizer/DataSyncMessageHandler.cs Renamed method to NotifyItemsUpdated and converted to fire-and-forget pattern with discard operator (critical: creates unobserved task exception risk)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deleteLater deleteLater requested a review from Copilot November 28, 2025 11:01
@deleteLater deleteLater changed the title fix: data change notify error handling feat: data change notify error handling Nov 28, 2025
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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@deleteLater deleteLater merged commit 7544725 into main Nov 28, 2025
6 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in FeatBit Nov 28, 2025
@deleteLater deleteLater deleted the fix/data-change-notify branch November 28, 2025 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants