-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[rb] remove bidi callbacks from sockets when unsubscribing #16476
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: trunk
Are you sure you want to change the base?
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
PR Code Suggestions ✨Explore these optional code suggestions:
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Moving this to draft status until we decide on #16487 |
User description
🔗 Related Issues
Right now clearing handlers between tests does not work because the callbacks are still executing on the socket.
💥 What does this PR do?
🔧 Implementation Notes
I considered a number of data structures first, but being able to store both the event and the subscription id with the same object was by far the easiest way to do the deletions
💡 Additional Considerations
🔄 Types of changes
PR Type
Bug fix, Enhancement
Description
Refactor callback management to use subscription IDs instead of object IDs
Enable proper cleanup of BiDi event handlers when unsubscribing
Replace callbacks dictionary with Registration struct for tracking subscriptions
Update WebSocket connection to store callbacks by ID in hash instead of array
Diagram Walkthrough
File Walkthrough
7 files
Update add_callback signature to accept ID parameter
Refactor to use subscription IDs for handler management
Store browsing context IDs and subscribe per callback
Add unsubscribe method and return subscription IDs
Update subscribe/unsubscribe to return/accept subscription IDs
Replace callbacks dict with Registration struct array
Store callbacks by ID in hash instead of array list
2 files
Update test to use returned intercept ID directly
Update all tests to check registrations instead of callbacks
7 files
Update type signatures for callback methods
Update subscribe_log_entry return type to string
Add browsing_context_ids instance variable type
Update add_intercept and on method return types
Update subscribe return type to string
Update Registration struct and registrations array types
Update callback method signatures and add clear_callbacks