-
-
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 parameterRefactor to use subscription IDs for handler managementStore browsing context IDs and subscribe per callbackAdd unsubscribe method and return subscription IDsUpdate subscribe/unsubscribe to return/accept subscription IDsReplace callbacks dict with Registration struct arrayStore callbacks by ID in hash instead of array list2 files
Update test to use returned intercept ID directlyUpdate all tests to check registrations instead of callbacks7 files
Update type signatures for callback methodsUpdate subscribe_log_entry return type to stringAdd browsing_context_ids instance variable typeUpdate add_intercept and on method return typesUpdate subscribe return type to stringUpdate Registration struct and registrations array typesUpdate callback method signatures and add clear_callbacks