-
Notifications
You must be signed in to change notification settings - Fork 15
Fix DevTools not logging outgoing messages #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
## Problem DevTools was only displaying incoming messages but not outgoing messages, making it difficult to debug bot conversations. ## Root Cause The event routing logic in AppEvents.cs used .Equals() to skip notifying plugins of their own events. However, this comparison had issues that caused DevToolsPlugin to be incorrectly skipped even though it should always be notified as an observer plugin. ## Solution Replaced .Equals() with ReferenceEquals() in three event handlers: - OnErrorEvent - OnActivitySentEvent - OnActivityResponseEvent ReferenceEquals() explicitly checks if two references point to the same object instance, ensuring only the actual sender plugin is skipped, not observer plugins like DevToolsPlugin. ## Testing - All 559 existing tests pass - Build succeeds with no errors - Added debug logging to help diagnose similar issues in the future Fixes #154 Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this 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 fixes issue #154 where DevTools was not displaying outgoing messages from bots, only incoming messages. The root cause was incorrect use of .Equals() for plugin identity checks, which caused DevToolsPlugin to be incorrectly skipped when broadcasting activity sent events.
Key Changes
- Replaced
.Equals()withReferenceEquals()in plugin skip logic to correctly identify the actual sender plugin - Added extensive debug logging to track event routing behavior (should be removed)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Libraries/Microsoft.Teams.Apps/AppEvents.cs | Fixed plugin comparison in three event handlers (OnErrorEvent, OnActivitySentEvent, OnActivityResponseEvent) by replacing .Equals() with ReferenceEquals(), plus added temporary debug logging |
| Libraries/Microsoft.Teams.Plugins/Microsoft.Teams.Plugins.AspNetCore.DevTools/DevToolsPlugin.cs | Added temporary debug logging to OnActivitySent to track when the method is called and how many WebSocket connections receive events |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Removed temporary debug logging from AppEvents.cs and DevToolsPlugin.cs - Refactored foreach loops to use .Where() for explicit filtering instead of continue - Cleaner, more idiomatic code following existing codebase patterns - All 559 tests still pass Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Addressed Review FeedbackThank you for the review. I have addressed all the feedback: Changes Made
Testing✅ All 559 tests pass The core fix (using |
There was a problem hiding this 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 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes issue #154 where DevTools was only displaying incoming messages but not outgoing messages, making it difficult to debug bot conversations during development.
Problem
The DevTools UI was correctly showing incoming messages but outgoing messages were not being captured or displayed. This significantly impacted the developer experience when debugging bots.
Root Cause
The event routing logic in
AppEvents.csused.Equals()to determine which plugins to skip when broadcasting events. The intention was to skip the plugin that sent the message (to avoid notifying it of its own action), but this comparison was incorrectly causing DevToolsPlugin to be skipped as well.Changes Made
Core Fix
Replaced
.Equals()withReferenceEquals()in three event handlers inLibraries/Microsoft.Teams.Apps/AppEvents.cs:OnErrorEvent(line 32)OnActivitySentEvent(line 55)OnActivityResponseEvent(line 72)ReferenceEquals()explicitly checks if two references point to the exact same object instance, ensuring only the actual sender plugin is skipped, not observer plugins like DevToolsPlugin.Debugging Enhancements
Added detailed debug logging to help diagnose similar issues in the future:
AppEvents.cs: Added logging to track sender, plugin iterations, equality checks, and skip decisionsDevToolsPlugin.cs: Added logging to confirm when OnActivitySent is called and how many WebSocket connections receive the eventTesting
✅ All 559 existing tests pass
✅ Build succeeds with no errors or warnings
✅ No breaking changes to public APIs
Verification Steps for Reviewers
Closes #154
🤖 Generated with Claude Code