-
Notifications
You must be signed in to change notification settings - Fork 54
fix: remove over-eager debug logging filtering from connectionManager #1594
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: release53
Are you sure you want to change the base?
Conversation
WalkthroughThe TSR handler was modified to simplify error object generation when messages are missing, returning only a generated message with device name and stringified error. Additionally, debug logging for connection events is now controlled solely by the global logDebug flag, removing per-device debugLogging checks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
About the Contributor
This pull request is posted on behalf of the NRK.
Type of Contribution
This is a:
Bug fix
Current Behavior
The event handler for
connectionEvent:debuginside playout gateway'stsrHandler.tsis a bit too nosy, checking the device config, before forwarding the logging to the logger. If for whatever reason, the config states between the two sides gets mismatched, the logging will not be forwarded.New Behavior
Since device-level logging filtering is already handled by TSR, there's no point in checking this twice. Checking if top-level debug logging is enabled at this point should be enough.
Additionally, handle a case if a non-Error value is emitted (such as undefined) in
fixError.Testing
Affected areas
This PR fixes a potential issue in the logging output of Playout Gateway.
Time Frame
Not urgent, but we would like to get this merged into the in-development release.
Other Information
Status
Summary
This PR removes redundant device-level debug logging filtering from the Playout Gateway's
tsrHandler.tsfile, and improves error handling for non-Error values.Changes
Debug Logging Gate Simplification (lines 374-387)
connectionEvent:debughandler now gates debug output solely by the globallogDebugflag, removing a device-leveldebugLoggingcheck that was previously appliedError Handling Robustness (lines 261-276)
fixErrorhelper function now explicitly handles non-Error values (e.g.,undefined, plain objects)messageproperty, the function returns a simplified error object with only a generated message (including device name and stringified error representation)Impact